diff --git a/internal/cli/fieldproject.go b/internal/cli/fieldproject.go index 47245d0..bc1fdee 100644 --- a/internal/cli/fieldproject.go +++ b/internal/cli/fieldproject.go @@ -139,8 +139,18 @@ func declareOutputNarrowing(cmd *cobra.Command, projectionFlag, rowsFlag string) // declaredNarrowing returns the flag cmd declared under annotation, or "" when // it declared none. A declaration whose flag the command no longer carries (a // renamed flag) is dropped, so the note can never name a flag that is not there. +// +// The projection side needs no declaration on a command that has no --fields of +// its own: the global --fields (outputfields.go) projects every structured +// record that command prints, so the command is narrowed by it without saying +// so. A command whose own --fields shadows the global one is not: that flag +// means whatever the command defines, and only an explicit declaration says it +// narrows output. func declaredNarrowing(cmd *cobra.Command, annotation string) string { name := cmd.Annotations[annotation] + if name == "" && annotation == narrowsByProjection && usesGlobalFields(cmd) { + return globalFieldsFlag + } if name == "" || cmd.Flags().Lookup(name) == nil { return "" } diff --git a/internal/cli/fieldproject_test.go b/internal/cli/fieldproject_test.go index 3607ea0..f570a54 100644 --- a/internal/cli/fieldproject_test.go +++ b/internal/cli/fieldproject_test.go @@ -1402,9 +1402,10 @@ func TestReductionAdviceNamesOnlyDeclaredFlags(t *testing.T) { } }) - // channel silence-rule-list is a generated verb that declares no narrowing - // flag: the note must not invent one. - t.Run("no narrowing flag", func(t *testing.T) { + // channel silence-rule-list is a generated verb: the global --fields narrows + // it, and it has no rows flag, so the note offers the first and invents no + // second. + t.Run("global fields only", func(t *testing.T) { saveAndResetGlobals(t) flagOutputFormat = "json" stub := newGFStub(t) @@ -1425,13 +1426,14 @@ func TestReductionAdviceNamesOnlyDeclaredFlags(t *testing.T) { if !strings.Contains(stderrText, "note: emitted") { t.Fatalf("over-budget page should announce the reduction, got:\n%s", stderrText) } - if !strings.Contains(stderrText, "declares no flag that narrows the output") { - t.Fatalf("advice should say no flag narrows this command, got:\n%s", stderrText) + if !strings.Contains(stderrText, "narrow --fields to fit more rows per page") { + t.Fatalf("advice should offer the global --fields, got:\n%s", stderrText) } - for _, flag := range []string{"--fields", "--limit"} { - if strings.Contains(stderrText, flag) { - t.Fatalf("advice must not name %s on a verb that does not narrow with it, got:\n%s", flag, stderrText) - } + if strings.Contains(stderrText, "declares no flag") { + t.Fatalf("advice must not claim nothing narrows this command, got:\n%s", stderrText) + } + if strings.Contains(stderrText, "--limit") { + t.Fatalf("advice must not name --limit on a verb that has none, got:\n%s", stderrText) } }) } @@ -1484,8 +1486,9 @@ func TestShorteningAdviceOmitsRowsFlag(t *testing.T) { // declare as narrowing. func TestOverflowErrorNamesOnlyDeclaredFlags(t *testing.T) { // A row whose bulk is a non-string field cannot be shortened, so the command - // fails. channel silence-rule-list declares no narrowing flag. - t.Run("no narrowing flag", func(t *testing.T) { + // fails. channel silence-rule-list is generated: only the global --fields + // narrows it. + t.Run("global fields only", func(t *testing.T) { saveAndResetGlobals(t) flagOutputFormat = "json" stub := newGFStub(t) @@ -1502,13 +1505,11 @@ func TestOverflowErrorNamesOnlyDeclaredFlags(t *testing.T) { if !strings.Contains(err.Error(), "largest fields:") { t.Fatalf("error should name the largest fields, got: %v", err) } - if !strings.Contains(err.Error(), "declares no flag that narrows the output") { - t.Fatalf("error should state no flag narrows this command, got: %v", err) + if !strings.Contains(err.Error(), "narrow --fields") { + t.Fatalf("error should offer the global --fields, got: %v", err) } - for _, flag := range []string{"--fields", "--limit"} { - if strings.Contains(err.Error(), flag) { - t.Fatalf("error must not name %s on a verb that does not narrow with it, got: %v", flag, err) - } + if strings.Contains(err.Error(), "declares no flag") || strings.Contains(err.Error(), "--limit") { + t.Fatalf("error must neither claim nothing narrows this command nor name --limit, got: %v", err) } }) @@ -1643,3 +1644,58 @@ func TestBoundProjectedListIdentifierOnlyOverflowReducesPage(t *testing.T) { t.Fatalf("bounded output is %d bytes, want <512", len(encoded)+1) } } + +// TestDeclaredNarrowingGlobalFieldsByFlagIdentity pins how the projection flag is +// derived for a command that declares nothing: it is the global --fields only +// while the command's --fields is that very flag. A command defining its own +// --fields (a write selector, say) shadows it and gets no advice unless it +// declares one; a command with no --fields at all gets none either. +func TestDeclaredNarrowingGlobalFieldsByFlagIdentity(t *testing.T) { + newTree := func(child *cobra.Command) *cobra.Command { + root := &cobra.Command{Use: "root"} + root.PersistentFlags().String(globalFieldsFlag, "", "") + root.AddCommand(child) + // Merging the persistent flags into the child's set is what a real + // execution does before the command runs. + child.Flags().AddFlagSet(root.PersistentFlags()) + return child + } + + t.Run("inherits global", func(t *testing.T) { + cmd := newTree(&cobra.Command{Use: "list"}) + if got := declaredNarrowing(cmd, narrowsByProjection); got != "fields" { + t.Errorf("projection flag = %q, want fields", got) + } + if got := declaredNarrowing(cmd, narrowsByRows); got != "" { + t.Errorf("rows flag = %q, want none: nothing declared one", got) + } + }) + + t.Run("own fields shadows global", func(t *testing.T) { + child := &cobra.Command{Use: "update"} + child.Flags().String(globalFieldsFlag, "", "fields to write") + cmd := newTree(child) + if got := declaredNarrowing(cmd, narrowsByProjection); got != "" { + t.Errorf("projection flag = %q, want none: --fields here is the command's own", got) + } + }) + + t.Run("own fields declared", func(t *testing.T) { + child := &cobra.Command{Use: "list"} + child.Flags().String(globalFieldsFlag, "", "fields to project") + declareOutputNarrowing(child, "fields", "") + cmd := newTree(child) + if got := declaredNarrowing(cmd, narrowsByProjection); got != "fields" { + t.Errorf("projection flag = %q, want the declared fields", got) + } + }) + + t.Run("no fields flag", func(t *testing.T) { + root := &cobra.Command{Use: "root"} + child := &cobra.Command{Use: "list"} + root.AddCommand(child) + if got := declaredNarrowing(child, narrowsByProjection); got != "" { + t.Errorf("projection flag = %q, want none: the command has no --fields", got) + } + }) +} diff --git a/internal/cli/gen_support_test.go b/internal/cli/gen_support_test.go index 42df45f..8757b2a 100644 --- a/internal/cli/gen_support_test.go +++ b/internal/cli/gen_support_test.go @@ -76,14 +76,15 @@ func TestPrintGenericResultBoundsListEnvelope(t *testing.T) { if !strings.Contains(stderrText, "note: emitted") { t.Errorf("reduced %s page should announce itself on stderr, got:\n%s", format, stderrText) } - // insight incident-list is a generated verb: it declares no narrowing - // flag, so even though it carries a --limit, the note names none. - if !strings.Contains(stderrText, "declares no flag that narrows the output") { - t.Errorf("a generated verb should get the flagless note, got:\n%s", stderrText) + // insight incident-list is a generated verb: the global --fields + // narrows it, but its --limit has no declared effect on the rows, so + // the note offers the first and never the second. + if !strings.Contains(stderrText, "narrow --fields to fit more rows per page") { + t.Errorf("a generated verb should be offered the global --fields, got:\n%s", stderrText) } - for _, flag := range []string{"--limit", "--fields"} { - if strings.Contains(stderrText, flag) { - t.Errorf("advice must not name %s on a verb that declares no narrowing flag, got:\n%s", flag, stderrText) + for _, bad := range []string{"--limit", "declares no flag"} { + if strings.Contains(stderrText, bad) { + t.Errorf("advice must not contain %q for a generated verb, got:\n%s", bad, stderrText) } } // The first row survives intact; the last was dropped by the @@ -165,13 +166,13 @@ func TestPrintGenericResultBoundsTopLevelArray(t *testing.T) { t.Errorf("reduced page should announce itself on stderr, got:\n%s", stderrText) } // monit rule-list-basic is a generated verb whose --limit sizes the response - // only alongside --include-descendants: it declares no narrowing flag, so the - // note must not offer --limit. - if !strings.Contains(stderrText, "declares no flag that narrows the output") { - t.Errorf("a generated verb should get the flagless note, got:\n%s", stderrText) + // only alongside --include-descendants: the note offers the global --fields + // and must not offer --limit. + if !strings.Contains(stderrText, "narrow --fields to fit more rows per page") { + t.Errorf("a generated verb should be offered the global --fields, got:\n%s", stderrText) } - if strings.Contains(stderrText, "--limit") { - t.Errorf("advice must not name --limit; this verb declares no narrowing flag, got:\n%s", stderrText) + if strings.Contains(stderrText, "--limit") || strings.Contains(stderrText, "declares no flag") { + t.Errorf("advice must not name --limit or claim nothing narrows, got:\n%s", stderrText) } var decoded []map[string]any if err := json.Unmarshal([]byte(strings.TrimSpace(out)), &decoded); err != nil { diff --git a/internal/cli/outputfields.go b/internal/cli/outputfields.go index c6efe8d..a69f9cd 100644 --- a/internal/cli/outputfields.go +++ b/internal/cli/outputfields.go @@ -18,6 +18,19 @@ import ( // monit rule-update-fields) shadows the global flag, so flagFields stays empty // for it and this projection never runs on top of the command's own. +// globalFieldsFlag is the name the root command registers the global +// projection flag under. +const globalFieldsFlag = "fields" + +// usesGlobalFields reports whether cmd's --fields is the root's global flag +// rather than a flag cmd defines itself under the same name. Flag identity, not +// the name, tells them apart: a command's own --fields is a different flag +// object that shadows the global one. +func usesGlobalFields(cmd *cobra.Command) bool { + own := cmd.Flags().Lookup(globalFieldsFlag) + return own != nil && own == cmd.Root().PersistentFlags().Lookup(globalFieldsFlag) +} + // projection is the result of projectOutput: the projected value, the // requested fields that matched no key, and every key the unprojected rows // carried (for the note that names what does exist). diff --git a/internal/cli/root.go b/internal/cli/root.go index b72528d..7e72ea0 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -108,7 +108,7 @@ func init() { rootCmd.PersistentFlags().BoolVar(&flagNoTrunc, "no-trunc", false, "Do not truncate table output") rootCmd.PersistentFlags().StringVar(&flagAppKey, "app-key", "", "Override app key") rootCmd.PersistentFlags().StringVar(&flagBaseURL, "base-url", "", "Override base URL") - rootCmd.PersistentFlags().StringVar(&flagFields, "fields", "", "Comma-separated top-level fields to keep in json/toon output (per row of a list, or of a single record); ignored in table mode") + rootCmd.PersistentFlags().StringVar(&flagFields, globalFieldsFlag, "", "Comma-separated top-level fields to keep in json/toon output (per row of a list, or of a single record); ignored in table mode") _ = rootCmd.PersistentFlags().MarkHidden("app-key") registerEnumFlag(rootCmd, "output-format", "table", "json", "toon") rootCmd.SetFlagErrorFunc(flagErrorWithCandidates) diff --git a/internal/cmd/cligen/main.go b/internal/cmd/cligen/main.go index bb55684..8af72d9 100644 --- a/internal/cmd/cligen/main.go +++ b/internal/cmd/cligen/main.go @@ -1336,9 +1336,11 @@ func emitCmd(fn string, s service, o specOp, mi methodInfo) string { // structured contract for whether a --limit bounds the emitted rows (the // knowledge file-list documents its --limit as ignored; rule-list-basic // honors its own only alongside --include-descendants), and a flag's name is - // not evidence of its effect. With no declaration the projection note names - // no flag, which is the honest default; a hand-written command that knows - // what its flags do declares them itself. + // not evidence of its effect, so the projection note never offers one. The + // global --fields needs no declaration: declaredNarrowing derives it for a + // command that does not define its own --fields (monit rule-update-fields + // does, as a write selector, and is left alone). A hand-written command that + // knows what its flags do declares them itself. switch { case mi.HasData: fmt.Fprintf(&b, "\t\t\t\tout, _, err := %s\n", call)