-
Notifications
You must be signed in to change notification settings - Fork 583
fix: skip flag-completion registration outside completion path #598
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| // Copyright (c) 2026 Lark Technologies Pte. Ltd. | ||
| // SPDX-License-Identifier: MIT | ||
|
|
||
| package cmdutil | ||
|
|
||
| import ( | ||
| "sync/atomic" | ||
|
|
||
| "github.com/spf13/cobra" | ||
| ) | ||
|
|
||
| // Cobra keeps completion callbacks in a package-global map keyed by | ||
| // *pflag.Flag with no removal path, so registrations made for a *cobra.Command | ||
| // outlive the command itself. Skip registration when the current invocation | ||
| // will not serve a completion request. | ||
| var flagCompletionsDisabled atomic.Bool | ||
|
|
||
| // SetFlagCompletionsDisabled switches RegisterFlagCompletion between | ||
| // registering and no-op. Typically set once at process start. | ||
| func SetFlagCompletionsDisabled(disabled bool) { | ||
| flagCompletionsDisabled.Store(disabled) | ||
| } | ||
|
|
||
| // FlagCompletionsDisabled reports the current switch state. | ||
| func FlagCompletionsDisabled() bool { | ||
| return flagCompletionsDisabled.Load() | ||
| } | ||
|
|
||
| // RegisterFlagCompletion wraps (*cobra.Command).RegisterFlagCompletionFunc | ||
| // and honors the package switch. The underlying error is swallowed to match | ||
| // the `_ = cmd.RegisterFlagCompletionFunc(...)` style already used here. | ||
| func RegisterFlagCompletion(cmd *cobra.Command, flagName string, fn cobra.CompletionFunc) { | ||
| if flagCompletionsDisabled.Load() { | ||
| return | ||
| } | ||
| _ = cmd.RegisterFlagCompletionFunc(flagName, fn) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| // Copyright (c) 2026 Lark Technologies Pte. Ltd. | ||
| // SPDX-License-Identifier: MIT | ||
|
|
||
| package cmdutil | ||
|
|
||
| import ( | ||
| "runtime" | ||
| "sync/atomic" | ||
| "testing" | ||
| "time" | ||
|
|
||
| "github.com/spf13/cobra" | ||
| ) | ||
|
|
||
| func TestSetFlagCompletionsDisabled_RoundTrip(t *testing.T) { | ||
| t.Cleanup(func() { SetFlagCompletionsDisabled(false) }) | ||
|
|
||
| if FlagCompletionsDisabled() { | ||
| t.Fatal("expected default false") | ||
| } | ||
| SetFlagCompletionsDisabled(true) | ||
| if !FlagCompletionsDisabled() { | ||
| t.Fatal("expected true after Set(true)") | ||
| } | ||
| SetFlagCompletionsDisabled(false) | ||
| if FlagCompletionsDisabled() { | ||
| t.Fatal("expected false after Set(false)") | ||
| } | ||
| } | ||
|
|
||
| // When disabled, a *cobra.Command must be collectable after the caller drops | ||
| // its reference — i.e. the wrapper did not touch cobra's global map. | ||
| func TestRegisterFlagCompletion_Disabled_DoesNotRetainCommand(t *testing.T) { | ||
| SetFlagCompletionsDisabled(true) | ||
| t.Cleanup(func() { SetFlagCompletionsDisabled(false) }) | ||
|
|
||
| const N = 5 | ||
| var collected atomic.Int32 | ||
| func() { | ||
| for range N { | ||
| cmd := &cobra.Command{Use: "x"} | ||
| cmd.Flags().String("foo", "", "") | ||
| RegisterFlagCompletion(cmd, "foo", func(_ *cobra.Command, _ []string, _ string) ([]cobra.Completion, cobra.ShellCompDirective) { | ||
| return nil, cobra.ShellCompDirectiveNoFileComp | ||
| }) | ||
| runtime.SetFinalizer(cmd, func(_ *cobra.Command) { collected.Add(1) }) | ||
| } | ||
| }() | ||
| // Finalizers run on a dedicated goroutine after GC; loop to give it time. | ||
| for range 30 { | ||
| runtime.GC() | ||
| time.Sleep(20 * time.Millisecond) | ||
| } | ||
| if got := collected.Load(); int(got) != N { | ||
| t.Fatalf("expected %d *cobra.Command finalizers to fire when completions disabled, got %d", N, got) | ||
| } | ||
| } | ||
|
|
||
| // When enabled, the registered completion must be reachable via cobra. | ||
| func TestRegisterFlagCompletion_Enabled_DoesRegister(t *testing.T) { | ||
| SetFlagCompletionsDisabled(false) | ||
|
|
||
| cmd := &cobra.Command{Use: "x"} | ||
| cmd.Flags().String("foo", "", "") | ||
| want := []cobra.Completion{"a", "b"} | ||
| RegisterFlagCompletion(cmd, "foo", func(_ *cobra.Command, _ []string, _ string) ([]cobra.Completion, cobra.ShellCompDirective) { | ||
| return want, cobra.ShellCompDirectiveNoFileComp | ||
| }) | ||
|
|
||
| fn, ok := cmd.GetFlagCompletionFunc("foo") | ||
| if !ok { | ||
| t.Fatal("expected completion func to be registered") | ||
| } | ||
| got, _ := fn(cmd, nil, "") | ||
| if len(got) != 2 || got[0] != "a" || got[1] != "b" { | ||
| t.Fatalf("unexpected completion result: %v", got) | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,98 @@ | ||
| // Copyright (c) 2026 Lark Technologies Pte. Ltd. | ||
| // SPDX-License-Identifier: MIT | ||
|
|
||
| package common | ||
|
|
||
| import ( | ||
| "context" | ||
| "testing" | ||
|
|
||
| "github.com/larksuite/cli/internal/cmdutil" | ||
| "github.com/spf13/cobra" | ||
| ) | ||
|
|
||
| // TestShortcutMount_FlagCompletionsRegistered exercises the two | ||
| // cmdutil.RegisterFlagCompletion call sites in registerShortcutFlagsWithContext: | ||
| // the per-flag enum completion (runner.go:879) and the auto-injected --format | ||
| // completion (runner.go:895). | ||
| func TestShortcutMount_FlagCompletionsRegistered(t *testing.T) { | ||
| t.Cleanup(func() { cmdutil.SetFlagCompletionsDisabled(false) }) | ||
| cmdutil.SetFlagCompletionsDisabled(false) | ||
|
|
||
| f, _, _, _ := cmdutil.TestFactory(t, nil) | ||
| parent := &cobra.Command{Use: "root"} | ||
| shortcut := Shortcut{ | ||
| Service: "docs", | ||
| Command: "+fetch", | ||
| Description: "fetch doc", | ||
| HasFormat: true, | ||
| Flags: []Flag{ | ||
| {Name: "sort-by", Desc: "sort", Enum: []string{"asc", "desc"}}, | ||
| }, | ||
| Execute: func(context.Context, *RuntimeContext) error { return nil }, | ||
| } | ||
| shortcut.Mount(parent, f) | ||
|
|
||
| cmd, _, err := parent.Find([]string{"+fetch"}) | ||
| if err != nil { | ||
| t.Fatalf("Find() error = %v", err) | ||
| } | ||
|
|
||
| // Enum flag completion. | ||
| fn, ok := cmd.GetFlagCompletionFunc("sort-by") | ||
| if !ok { | ||
| t.Fatal("expected completion func for --sort-by") | ||
| } | ||
| got, _ := fn(cmd, nil, "") | ||
| if len(got) != 2 || got[0] != "asc" || got[1] != "desc" { | ||
| t.Fatalf("sort-by completion = %v, want [asc desc]", got) | ||
| } | ||
|
|
||
| // HasFormat-injected --format completion. | ||
| fn, ok = cmd.GetFlagCompletionFunc("format") | ||
| if !ok { | ||
| t.Fatal("expected completion func for --format") | ||
| } | ||
| got, _ = fn(cmd, nil, "") | ||
| want := []string{"json", "pretty", "table", "ndjson", "csv"} | ||
| if len(got) != len(want) { | ||
| t.Fatalf("format completion = %v, want %v", got, want) | ||
| } | ||
| for i, v := range want { | ||
| if got[i] != v { | ||
| t.Fatalf("format completion[%d] = %q, want %q", i, got[i], v) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // TestShortcutMount_FlagCompletionsDisabled verifies the switch actually | ||
| // prevents the two registrations from landing in cobra's global map. | ||
| func TestShortcutMount_FlagCompletionsDisabled(t *testing.T) { | ||
| t.Cleanup(func() { cmdutil.SetFlagCompletionsDisabled(false) }) | ||
| cmdutil.SetFlagCompletionsDisabled(true) | ||
|
|
||
| f, _, _, _ := cmdutil.TestFactory(t, nil) | ||
| parent := &cobra.Command{Use: "root"} | ||
| shortcut := Shortcut{ | ||
| Service: "docs", | ||
| Command: "+fetch", | ||
| Description: "fetch doc", | ||
| HasFormat: true, | ||
| Flags: []Flag{ | ||
| {Name: "sort-by", Desc: "sort", Enum: []string{"asc", "desc"}}, | ||
| }, | ||
| Execute: func(context.Context, *RuntimeContext) error { return nil }, | ||
| } | ||
| shortcut.Mount(parent, f) | ||
|
|
||
| cmd, _, err := parent.Find([]string{"+fetch"}) | ||
| if err != nil { | ||
| t.Fatalf("Find() error = %v", err) | ||
| } | ||
| if _, ok := cmd.GetFlagCompletionFunc("sort-by"); ok { | ||
| t.Fatal("did not expect completion func for --sort-by when disabled") | ||
| } | ||
| if _, ok := cmd.GetFlagCompletionFunc("format"); ok { | ||
| t.Fatal("did not expect completion func for --format when disabled") | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.