Add support for git flags in gh repo fork#2282
Conversation
pkg/cmd/repo/fork/fork.go
Outdated
| With no argument, creates a fork of the current repository. Otherwise, forks the specified repository.`, | ||
| With no argument, creates a fork of the current repository. Otherwise, forks the specified repository. | ||
|
|
||
| If '--clone' is specified, you can pass additional 'git clone' flags by listing them after '--'.`, |
There was a problem hiding this comment.
"If the '--clone' flag is specified, additional 'git clone' flags can be passed in by listing them after '--'."
I think this wording is a bit more clear.
cc @ampinsk
There was a problem hiding this comment.
If --clone wasn't specified, a prompt will be shown and the user will still have an opportunity to clone. Therefore, gitflags might take effect regardless of whether --clone was explicitly used or not.
samcoe
left a comment
There was a problem hiding this comment.
👍 Thanks for contributing. The code looks good to me, I just made one comment regarding the language.
pkg/cmd/repo/fork/fork.go
Outdated
| cmd := &cobra.Command{ | ||
| Use: "fork [<repository>]", | ||
| Args: cobra.MaximumNArgs(1), | ||
| Use: "fork [<repository>] [-- <gitflags>...]", |
There was a problem hiding this comment.
Unlike the repo clone command, the <repository> argument here is optional, and so are the gitflags. Parsing flags is therefore tricky, because if you pass any gitflags while omitting the <repository> argument, the first gitflag (e.g. --depth) will get interpreted as the repository name.
The solution would have to be explicitly detecting what is left and what is right of the -- flag separator, but I don't think the pflag parser gives us that information. What the command sees is the list of all arguments joined together in a []string slice with the -- separator removed.
There was a problem hiding this comment.
I've tried a few possible cases:
gh repo fork -- --depth 1
argument error: expected the "[HOST/]OWNER/REPO" format, got "--depth"
-----------------------------------------------------------------------------
gh repo fork --depth 1
unknown flag: --depth
Separate git clone flags with '--'.
Usage: gh repo fork [<repository>] [-- <gitflags>...]
Flags:
--clone Clone the fork {true|false}
--remote Add remote for fork {true|false}In the former case, ghrepo.FromFullName catches it and shows the expected format.
In the second case, any flag that's not '--clone' or '--remote' fires the custom err msg. Do you think it should show something else here?
There was a problem hiding this comment.
nvm, I added a custom validator to warn the user if they is passing git flags without specifying the <repository> argument.
ArgsLenAtDash allows you to check the length of args before the -- separator.
| return forkRun(opts) | ||
| }, | ||
| } | ||
| cmd.SetFlagErrorFunc(func(cmd *cobra.Command, err error) error { |
pkg/cmd/repo/fork/fork.go
Outdated
| With no argument, creates a fork of the current repository. Otherwise, forks the specified repository.`, | ||
| With no argument, creates a fork of the current repository. Otherwise, forks the specified repository. | ||
|
|
||
| If '--clone' is specified, you can pass additional 'git clone' flags by listing them after '--'.`, |
There was a problem hiding this comment.
If --clone wasn't specified, a prompt will be shown and the user will still have an opportunity to clone. Therefore, gitflags might take effect regardless of whether --clone was explicitly used or not.
Fix #2236
I added the same custom error msg from #1475 for
gh repo forkwhen trying to use unknown git flags.