Allow auto-completing the --repo values#3942
Conversation
|
Hi! Thanks for the pull request. Please ensure that this change is linked to an issue by mentioning an issue number in the description of the pull request. If this pull request would close the issue, please put the word 'Fixes' before the issue number somewhere in the pull request body. If this is a tiny change like fixing a typo, feel free to ignore this message. |
| @@ -19,6 +21,18 @@ func executeParentHooks(cmd *cobra.Command, args []string) error { | |||
|
|
|||
| func EnableRepoOverride(cmd *cobra.Command, f *Factory) { | |||
mislav
left a comment
There was a problem hiding this comment.
The functionality spiked out here looks good to me and works well in my testing!
As described in the PR, this seeds possible -R values from the list of git remotes. That solves your use case, but note that it's possible to use any GitHub repository as the value of -R, and that outside of the context of a local git repository, there will be nothing completed for -R as there will be no git remotes. I wonder should we try to provide some suggestions there, for example using the API to list your recently accessed repositories. Of course, this explodes the scope of this feature quite a bit, so I'd be fine even with just going the git remotes approach and expanding the quality of -R suggestions as follow-up.
What are your thoughts?
pkg/cmdutil/repo_override.go
Outdated
| _ = cmd.RegisterFlagCompletionFunc("repo", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { | ||
| var results []string | ||
| remotes, _ := f.Remotes() | ||
| for _, remote := range remotes.FilterByHosts([]string{"github.com"}) { |
There was a problem hiding this comment.
It's best not to hardcode github.com. In case GH_HOST is set via environment, you should get Config from f and call cfg.DefaultHost() and use that. It will be github.com by default.
There was a problem hiding this comment.
Okay, it took a bit of guessing because I am not fluent in Go, but I think I got it now.
pkg/cmdutil/repo_override.go
Outdated
| } | ||
| } | ||
| sort.Strings(results) | ||
| return results, cobra.ShellCompDirectiveNoSpace |
There was a problem hiding this comment.
ShellCompDirectiveNoSpace is only to be used when no space character should be added after completion. Here, it's better to use:
| return results, cobra.ShellCompDirectiveNoSpace | |
| return results, cobra.ShellCompDirectiveNoFileComp |
pkg/cmdutil/repo_override.go
Outdated
| cmd.PersistentFlags().StringP("repo", "R", "", "Select another repository using the `[HOST/]OWNER/REPO` format") | ||
| _ = cmd.RegisterFlagCompletionFunc("repo", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { | ||
| var results []string | ||
| remotes, _ := f.Remotes() |
There was a problem hiding this comment.
You should handle errors here. If there is an error, return early with cobra.ShellCompDirectiveError
Looking at the locally-registered remotes, we have a pretty good idea what `--repo` values are available. Let's complete them. Helped by Nate Smith and Mislav Marohnić. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
d2dddb1 to
b43f78b
Compare
I would really like to avoid extra network roundtrips if possible, read: I still like my approach, and would want to fall back to querying the REST API only if there is no completion for the current prefix. |
|
Thank you @mislav !!! |
I have a local worktree with 28 different remotes, and
gh pr createusually tries to pick one of the 27 I don't want to open a PR. So I thought it would be neat to have auto-completion for the--repooption. And here it is.Please let me know if I made any mistake; This is the first Go code I wrote in at least two years.