| Message ID | pull.1585.v2.git.1694538135853.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 4333267995662ccc9f4db3b628eebb78599e0025 |
| Headers | show |
| Series | [v2] completion: improve doc for complex aliases | expand |
"Philippe Blain via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Philippe Blain <levraiphilippeblain@gmail.com> > > The completion code can be told to use a particular completion for > aliases that shell out by using ': git <cmd> ;' as the first command of > the alias. This only works if <cmd> and the semicolon are separated by a > space, since if the space is missing __git_aliased_command returns (for > example) 'checkout;' instead of just 'checkout', and then > __git_complete_command fails to find a completion for 'checkout;'. > > The examples have that space but it's not clear if it's just for > style or if it's mandatory. Explicitly mention it. > > Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com> > --- Thanks. I scanned the case statement in the loop in the function and thought "hmph, everybody says ': git <cmd> ;' but is 'git' really needed?" I had "git l3" alias that invokes "$HOM#/bin/git-l" command, like so: [alias] l3 = "!sh -c ': git log ; git l \"$@\"' -" but if I did 's/: git log/: log/' it still completes just fine. I wonder if this hack is worth adding, instead of (or in addition to) requiring the user to insert $IFS to please the "parser", we can honor the rather obvious wish of the user in a more direct way. contrib/completion/git-completion.bash | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git c/contrib/completion/git-completion.bash w/contrib/completion/git-completion.bash index 19139ac121..e31d71955f 100644 --- c/contrib/completion/git-completion.bash +++ w/contrib/completion/git-completion.bash @@ -1183,7 +1183,7 @@ __git_aliased_command () :) : skip null command ;; \'*) : skip opening quote after sh -c ;; *) - cur="$word" + cur="${word%;}" break esac done
"Philippe Blain via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Philippe Blain <levraiphilippeblain@gmail.com> > > The completion code can be told to use a particular completion for > aliases that shell out by using ': git <cmd> ;' as the first command of > the alias. This only works if <cmd> and the semicolon are separated by a > space, since if the space is missing __git_aliased_command returns (for > example) 'checkout;' instead of just 'checkout', and then > __git_complete_command fails to find a completion for 'checkout;'. > > The examples have that space but it's not clear if it's just for > style or if it's mandatory. Explicitly mention it. > > Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com> > --- > completion: improve doc for complex aliases > > Changes since v1: > > * fixed the typo pointed out by Eric > * added an explanation of why the space is mandatory, as suggested by > Linus > Thanks for the investigation. The commit message reads much better now. This LGTM, but I think Junio's review comments [1] are worth considering. I'll respond there also. [1] https://lore.kernel.org/git/xmqqo7i6khxv.fsf@gitster.g/#t
Junio C Hamano <gitster@pobox.com> writes: > "Philippe Blain via GitGitGadget" <gitgitgadget@gmail.com> writes: > >> From: Philippe Blain <levraiphilippeblain@gmail.com> >> >> The completion code can be told to use a particular completion for >> aliases that shell out by using ': git <cmd> ;' as the first command of >> the alias. This only works if <cmd> and the semicolon are separated by a >> space, since if the space is missing __git_aliased_command returns (for >> example) 'checkout;' instead of just 'checkout', and then >> __git_complete_command fails to find a completion for 'checkout;'. >> >> The examples have that space but it's not clear if it's just for >> style or if it's mandatory. Explicitly mention it. >> >> Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com> >> --- > > Thanks. I scanned the case statement in the loop in the function > and thought "hmph, everybody says ': git <cmd> ;' but is 'git' > really needed?" > > I had "git l3" alias that invokes "$HOM#/bin/git-l" command, like so: > > [alias] > l3 = "!sh -c ': git log ; git l \"$@\"' -" > > but if I did 's/: git log/: log/' it still completes just fine. Interesting! I searched for the 'git <cmd>' and got some hits in "t9902-completion.sh" when running "git grep -nE 'git <cmd>'": t/t9902-completion.sh:2432:test_expect_success "completion uses <cmd> completion for alias: !sh -c 'git <cmd> ...'" ' t/t9902-completion.sh:2441:test_expect_success 'completion uses <cmd> completion for alias: !f () { VAR=val git <cmd> ... }' ' t/t9902-completion.sh:2450:test_expect_success 'completion used <cmd> completion for alias: !f() { : git <cmd> ; ... }' ' When I did 's/: git log/: log/' in the test at line 2450, the test still passed. Perhaps we should add this "git"-less version as another test case? > I wonder if this hack is worth adding, instead of (or in addition > to) requiring the user to insert $IFS to please the "parser", we can > honor the rather obvious wish of the user in a more direct way. > > > > contrib/completion/git-completion.bash | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git c/contrib/completion/git-completion.bash w/contrib/completion/git-completion.bash > index 19139ac121..e31d71955f 100644 > --- c/contrib/completion/git-completion.bash > +++ w/contrib/completion/git-completion.bash > @@ -1183,7 +1183,7 @@ __git_aliased_command () > :) : skip null command ;; > \'*) : skip opening quote after sh -c ;; > *) > - cur="$word" > + cur="${word%;}" > break > esac > done I think this is a good defensive technique. This obviously changes the guidance that Phillipe gave in their patch (we no longer have to worry about adding a space or not between "word" and ";", so there's no need to mention this explicitly any more), but to me this seems like a better experience for our users because it's one less thing they have to worry about.
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index dc95c34cc85..659df570496 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -28,6 +28,7 @@ # completion style. For example '!f() { : git commit ; ... }; f' will # tell the completion to use commit completion. This also works with aliases # of form "!sh -c '...'". For example, "!sh -c ': git commit ; ... '". +# Be sure to add a space between the command name and the ';'. # # If you have a command that is not part of git, but you would still # like completion, you can use __git_complete: