From a510c729106c006374ac80cdd127ed62693e1749 Mon Sep 17 00:00:00 2001 From: Rahul Bansal <42.rahulbansal@gmail.com> Date: Fri, 13 Feb 2026 19:06:12 +0530 Subject: [PATCH] fix: address completion follow-up issues (#35-#39, #41) (#46) * fix: address completion follow-up issues (#35-#39, #41) - Add doc comment explaining filterPrefix redundancy with Cobra (#35) - Extract magic numbers into named constants (completionPageLimit, completionPRPageLimit) (#36) - Add Username to member completion fallback chain (#37) - Add --branch flag completion to browse command (#38) - Document 'on hold' space issue in issue list completion (#39) - Add completions for repo clone/delete/view/fork/sync/setdefault (#41) - Add --status and --branch completions to pipeline list (#41) - Add --role completion to snippet list (#41) - Register global --repo persistent flag completion on root command (#41) * fix: address code review findings - Align pipeline list --status completion values with API state model (use IN_PROGRESS/SUCCESSFUL instead of BUILDING/ERROR) - Remove misleading --branch completion from repo clone (CompleteBranchNames resolves from CWD, not the target repo) - Add clarifying comment on root --repo persistent flag shadowing --------- Co-authored-by: Rahul Bansal --- internal/cmd/browse/browse.go | 1 + internal/cmd/issue/list.go | 4 ++++ internal/cmd/pipeline/list.go | 4 ++++ internal/cmd/repo/clone.go | 4 ++-- internal/cmd/repo/delete.go | 2 ++ internal/cmd/repo/fork.go | 1 + internal/cmd/repo/setdefault.go | 2 ++ internal/cmd/repo/sync.go | 2 ++ internal/cmd/repo/view.go | 2 ++ internal/cmd/root.go | 4 ++++ internal/cmd/snippet/list.go | 3 +++ internal/cmdutil/completion.go | 26 ++++++++++++++++++++------ 12 files changed, 47 insertions(+), 8 deletions(-) diff --git a/internal/cmd/browse/browse.go b/internal/cmd/browse/browse.go index 3db7f03..1ea00f6 100644 --- a/internal/cmd/browse/browse.go +++ b/internal/cmd/browse/browse.go @@ -145,6 +145,7 @@ Use flags to open specific sections like issues, pull requests, or settings.`, cmd.Flags().BoolVar(&downloads, "downloads", false, "Open downloads page") _ = cmd.RegisterFlagCompletionFunc("repo", cmdutil.CompleteRepoNames) + _ = cmd.RegisterFlagCompletionFunc("branch", cmdutil.CompleteBranchNames) return cmd } diff --git a/internal/cmd/issue/list.go b/internal/cmd/issue/list.go index f19c11d..179dd40 100644 --- a/internal/cmd/issue/list.go +++ b/internal/cmd/issue/list.go @@ -74,6 +74,10 @@ priority, or assignee.`, cmd.Flags().BoolVar(&opts.JSON, "json", false, "Output in JSON format") cmd.Flags().StringVar(&opts.Repo, "repo", "", "Repository in WORKSPACE/REPO format") + // NOTE: "on hold" contains a space, which is the canonical Bitbucket API value + // (confirmed in the OpenAPI spec enum). Cobra handles quoting automatically for + // zsh, fish, and PowerShell. Bash completion may require the user to quote the + // value (e.g. --state "on hold" or --state 'on hold'). _ = cmd.RegisterFlagCompletionFunc("state", cmdutil.StaticFlagCompletion([]string{ "new", "open", "resolved", "on hold", "invalid", "duplicate", "wontfix", "closed", })) diff --git a/internal/cmd/pipeline/list.go b/internal/cmd/pipeline/list.go index 5d9c6b2..3571ea6 100644 --- a/internal/cmd/pipeline/list.go +++ b/internal/cmd/pipeline/list.go @@ -65,6 +65,10 @@ by pipeline status (PENDING, IN_PROGRESS, COMPLETED, FAILED, etc.).`, cmd.Flags().BoolVar(&opts.JSON, "json", false, "Output in JSON format") cmd.Flags().StringVarP(&opts.Repo, "repo", "R", "", "Repository in WORKSPACE/REPO format") + _ = cmd.RegisterFlagCompletionFunc("status", cmdutil.StaticFlagCompletion([]string{ + "PENDING", "IN_PROGRESS", "COMPLETED", "FAILED", "STOPPED", "EXPIRED", "SUCCESSFUL", + })) + _ = cmd.RegisterFlagCompletionFunc("branch", cmdutil.CompleteBranchNames) _ = cmd.RegisterFlagCompletionFunc("repo", cmdutil.CompleteRepoNames) return cmd diff --git a/internal/cmd/repo/clone.go b/internal/cmd/repo/clone.go index ce2b9a8..974196c 100644 --- a/internal/cmd/repo/clone.go +++ b/internal/cmd/repo/clone.go @@ -69,6 +69,8 @@ to change this preference.`, cmd.Flags().IntVar(&opts.depth, "depth", 0, "Create a shallow clone with a limited number of commits") cmd.Flags().StringVarP(&opts.branch, "branch", "b", "", "Clone a specific branch") + cmd.ValidArgsFunction = cmdutil.CompleteRepoNames + return cmd } @@ -208,5 +210,3 @@ func extractRepoNameFromURL(url string) string { } return "" } - - diff --git a/internal/cmd/repo/delete.go b/internal/cmd/repo/delete.go index ed3b08e..75e1cdb 100644 --- a/internal/cmd/repo/delete.go +++ b/internal/cmd/repo/delete.go @@ -49,6 +49,8 @@ unless the --yes flag is provided.`, cmd.Flags().BoolVarP(&opts.yes, "yes", "y", false, "Skip confirmation prompt") + cmd.ValidArgsFunction = cmdutil.CompleteRepoNames + return cmd } diff --git a/internal/cmd/repo/fork.go b/internal/cmd/repo/fork.go index 77e9fa9..ee3e93c 100644 --- a/internal/cmd/repo/fork.go +++ b/internal/cmd/repo/fork.go @@ -75,6 +75,7 @@ as a new remote (default name: "fork").`, cmd.Flags().BoolVarP(&opts.clone, "clone", "c", false, "Clone the fork after creation") cmd.Flags().StringVar(&opts.remoteName, "remote-name", "fork", "Name for the new remote when in an existing clone") + cmd.ValidArgsFunction = cmdutil.CompleteRepoNames _ = cmd.RegisterFlagCompletionFunc("workspace", cmdutil.CompleteWorkspaceNames) return cmd diff --git a/internal/cmd/repo/setdefault.go b/internal/cmd/repo/setdefault.go index 37575f7..122f5fa 100644 --- a/internal/cmd/repo/setdefault.go +++ b/internal/cmd/repo/setdefault.go @@ -71,6 +71,8 @@ require a repository context.`, cmd.Flags().BoolVar(&opts.View, "view", false, "Show the current default repository") cmd.Flags().BoolVar(&opts.Unset, "unset", false, "Remove the default repository") + cmd.ValidArgsFunction = cmdutil.CompleteRepoNames + return cmd } diff --git a/internal/cmd/repo/sync.go b/internal/cmd/repo/sync.go index 3a14cf9..91ba65d 100644 --- a/internal/cmd/repo/sync.go +++ b/internal/cmd/repo/sync.go @@ -57,6 +57,8 @@ By default, the main branch is synced. Use --branch to specify a different branc cmd.Flags().StringVarP(&opts.branch, "branch", "b", "", "Branch to sync (default: main branch)") cmd.Flags().BoolVarP(&opts.force, "force", "f", false, "Force update (reset to upstream, discarding local changes)") + _ = cmd.RegisterFlagCompletionFunc("branch", cmdutil.CompleteBranchNames) + return cmd } diff --git a/internal/cmd/repo/view.go b/internal/cmd/repo/view.go index c09378e..8ce9f61 100644 --- a/internal/cmd/repo/view.go +++ b/internal/cmd/repo/view.go @@ -63,6 +63,8 @@ You can specify a repository using the workspace/repo format.`, cmd.Flags().BoolVarP(&opts.web, "web", "w", false, "Open the repository in a web browser") cmd.Flags().BoolVar(&opts.jsonOut, "json", false, "Output in JSON format") + cmd.ValidArgsFunction = cmdutil.CompleteRepoNames + return cmd } diff --git a/internal/cmd/root.go b/internal/cmd/root.go index 918b653..b5a9c97 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -18,6 +18,7 @@ import ( "github.com/rbansal42/bitbucket-cli/internal/cmd/repo" "github.com/rbansal42/bitbucket-cli/internal/cmd/snippet" "github.com/rbansal42/bitbucket-cli/internal/cmd/workspace" + "github.com/rbansal42/bitbucket-cli/internal/cmdutil" "github.com/rbansal42/bitbucket-cli/internal/iostreams" ) @@ -66,6 +67,9 @@ func Execute() error { func init() { // Global flags rootCmd.PersistentFlags().StringP("repo", "R", "", "Select a repository using the WORKSPACE/REPO format") + // Register completion for the persistent --repo flag. Subcommands that define + // their own local --repo flag will shadow this with their own registration. + _ = rootCmd.RegisterFlagCompletionFunc("repo", cmdutil.CompleteRepoNames) // Version command rootCmd.AddCommand(&cobra.Command{ diff --git a/internal/cmd/snippet/list.go b/internal/cmd/snippet/list.go index 7ebe8ab..2b972a8 100644 --- a/internal/cmd/snippet/list.go +++ b/internal/cmd/snippet/list.go @@ -58,6 +58,9 @@ Snippets are workspace-scoped and can be filtered by your role.`, cmd.Flags().BoolVar(&opts.JSON, "json", false, "Output in JSON format") _ = cmd.RegisterFlagCompletionFunc("workspace", cmdutil.CompleteWorkspaceNames) + _ = cmd.RegisterFlagCompletionFunc("role", cmdutil.StaticFlagCompletion([]string{ + "owner", "contributor", "member", + })) return cmd } diff --git a/internal/cmdutil/completion.go b/internal/cmdutil/completion.go index cd4c470..436b982 100644 --- a/internal/cmdutil/completion.go +++ b/internal/cmdutil/completion.go @@ -15,6 +15,13 @@ import ( // completionTimeout is the maximum time allowed for completion API calls. const completionTimeout = 5 * time.Second +// completionListPageSize is the page size for list-type API calls during completion. +const completionListPageSize = 50 + +// completionDetailPageSize is the page size for PR/issue API calls during completion. +// Smaller because these include more data per item. +const completionDetailPageSize = 30 + // StaticFlagCompletion returns a completion function compatible with // cobra.RegisterFlagCompletionFunc. It filters values by the toComplete // prefix (case-insensitive) and always returns ShellCompDirectiveNoFileComp. @@ -91,7 +98,7 @@ func CompleteWorkspaceNames(cmd *cobra.Command, args []string, toComplete string ctx, cancel := completionCtx() defer cancel() - result, err := client.ListWorkspaces(ctx, &api.WorkspaceListOptions{Limit: 50}) + result, err := client.ListWorkspaces(ctx, &api.WorkspaceListOptions{Limit: completionListPageSize}) if err != nil { return nil, cobra.ShellCompDirectiveNoFileComp } @@ -121,7 +128,7 @@ func CompleteRepoNames(cmd *cobra.Command, args []string, toComplete string) ([] ctx, cancel := completionCtx() defer cancel() - result, err := client.ListRepositories(ctx, ws, &api.RepositoryListOptions{Limit: 50}) + result, err := client.ListRepositories(ctx, ws, &api.RepositoryListOptions{Limit: completionListPageSize}) if err != nil { return nil, cobra.ShellCompDirectiveNoFileComp } @@ -149,7 +156,7 @@ func CompleteBranchNames(cmd *cobra.Command, args []string, toComplete string) ( ctx, cancel := completionCtx() defer cancel() - result, err := client.ListBranches(ctx, ws, slug, &api.BranchListOptions{Limit: 50}) + result, err := client.ListBranches(ctx, ws, slug, &api.BranchListOptions{Limit: completionListPageSize}) if err != nil { return nil, cobra.ShellCompDirectiveNoFileComp } @@ -177,7 +184,7 @@ func CompletePRNumbers(cmd *cobra.Command, args []string, toComplete string) ([] ctx, cancel := completionCtx() defer cancel() - result, err := client.ListPullRequests(ctx, ws, slug, &api.PRListOptions{State: api.PRStateOpen, Limit: 30}) + result, err := client.ListPullRequests(ctx, ws, slug, &api.PRListOptions{State: api.PRStateOpen, Limit: completionDetailPageSize}) if err != nil { return nil, cobra.ShellCompDirectiveNoFileComp } @@ -205,7 +212,7 @@ func CompleteIssueIDs(cmd *cobra.Command, args []string, toComplete string) ([]s ctx, cancel := completionCtx() defer cancel() - result, err := client.ListIssues(ctx, ws, slug, &api.IssueListOptions{Limit: 30}) + result, err := client.ListIssues(ctx, ws, slug, &api.IssueListOptions{Limit: completionDetailPageSize}) if err != nil { return nil, cobra.ShellCompDirectiveNoFileComp } @@ -233,7 +240,7 @@ func CompleteWorkspaceMembers(cmd *cobra.Command, args []string, toComplete stri ctx, cancel := completionCtx() defer cancel() - result, err := client.ListWorkspaceMembers(ctx, ws, &api.WorkspaceMemberListOptions{Limit: 50}) + result, err := client.ListWorkspaceMembers(ctx, ws, &api.WorkspaceMemberListOptions{Limit: completionListPageSize}) if err != nil { return nil, cobra.ShellCompDirectiveNoFileComp } @@ -242,6 +249,9 @@ func CompleteWorkspaceMembers(cmd *cobra.Command, args []string, toComplete stri for _, m := range result.Values { if m.User != nil { name := m.User.Nickname + if name == "" { + name = m.User.Username + } if name == "" { name = m.User.DisplayName } @@ -256,6 +266,10 @@ func CompleteWorkspaceMembers(cmd *cobra.Command, args []string, toComplete stri // filterPrefix filters values by the toComplete prefix (case-insensitive). // For tab-separated values ("id\tdescription"), only the part before the tab is matched. +// +// NOTE: Cobra's completion framework also performs its own prefix filtering. This +// function provides a safety net to reduce API response data before passing to Cobra, +// and handles the tab-separated value format correctly. The redundancy is intentional. func filterPrefix(values []string, toComplete string) []string { if toComplete == "" { return values