Skip to content

Commit 65c5fdd

Browse files
committed
fix: address review feedback from CodeRabbit
- Remove -h shorthand on host flag to avoid panic from Cobra's built-in help flag conflict (cli/example_test.go) - Handle error from defaults.Set instead of ignoring it (config.go) - Preserve PersistentPreRunE in addition to PersistentPreRun so Init doesn't silently drop user hooks (cli.go) - Add Unwrap() to statusWriter so http.Flusher/Hijacker pass through, fixing streaming and h2c support (requestlog.go)
1 parent 8e152b6 commit 65c5fdd

File tree

4 files changed

+22
-7
lines changed

4 files changed

+22
-7
lines changed

cli/cli.go

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,16 +53,23 @@ func Init(rootCmd *cobra.Command, opts ...Option) {
5353
rootCmd.SilenceUsage = true
5454

5555
// Inject shared output and prompter into command context.
56-
existing := rootCmd.PersistentPreRun
57-
rootCmd.PersistentPreRun = func(cmd *cobra.Command, args []string) {
56+
// Preserve any existing PersistentPreRun or PersistentPreRunE hook.
57+
existingRun := rootCmd.PersistentPreRun
58+
existingRunE := rootCmd.PersistentPreRunE
59+
rootCmd.PersistentPreRun = nil
60+
rootCmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error {
5861
ctx := context.WithValue(cmd.Context(), contextKey{}, &cliContext{
5962
output: printer.NewOutput(os.Stdout),
6063
prompter: prompt.New(),
6164
})
6265
cmd.SetContext(ctx)
63-
if existing != nil {
64-
existing(cmd, args)
66+
if existingRunE != nil {
67+
return existingRunE(cmd, args)
6568
}
69+
if existingRun != nil {
70+
existingRun(cmd, args)
71+
}
72+
return nil
6673
}
6774

6875
// Wire commander features.

cli/example_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ func ExampleInit() {
1111
Use: "frontier",
1212
Short: "identity management",
1313
}
14-
rootCmd.PersistentFlags().StringP("host", "h", "", "API server host")
14+
rootCmd.PersistentFlags().String("host", "", "API server host")
1515

1616
listCmd := &cobra.Command{
1717
Use: "list",

config/config.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,10 @@ func (l *Loader) Load(config any) error {
9696
return err
9797
}
9898

99-
// Apply default values before reading configuration
100-
defaults.Set(config)
99+
// Apply default values before reading configuration.
100+
if err := defaults.Set(config); err != nil {
101+
return fmt.Errorf("failed to set defaults: %w", err)
102+
}
101103

102104
// Bind flags dynamically using reflection on `cmdx` tags if a flag set is provided
103105
if l.flags != nil {

middleware/requestlog/requestlog.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,3 +112,9 @@ func (w *statusWriter) WriteHeader(code int) {
112112
w.status = code
113113
w.ResponseWriter.WriteHeader(code)
114114
}
115+
116+
// Unwrap returns the underlying ResponseWriter, allowing the http package
117+
// to access optional interfaces (Flusher, Hijacker, etc.) through it.
118+
func (w *statusWriter) Unwrap() http.ResponseWriter {
119+
return w.ResponseWriter
120+
}

0 commit comments

Comments
 (0)