| Message ID | 8c902c53-815d-43c2-8ba9-8144d8884804@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | completion for git-reflog show | expand |
Rubén Justo <rjusto@gmail.com> writes: > +# Completion for subcommands in commands that follow the syntax: > +# > +# git <command> <subcommand> > +# > +# 1: List of possible completion words. > +# Returns false if the current word is not a possible subcommand > +# (possitioned after the command), or if no option is found in > +# the list provided. > +__gitcomp_subcommand () > +{ > + local subcommands="$1" > + > + if [ $cword -eq $(($__git_cmd_idx + 1)) ]; then > + __gitcomp "$subcommands" > + > + test -n "$COMPREPLY" > + else > + false > + fi > +} I am not at all familiar with the code in this file, so treat this as a question from somebody who do not know the calling convention used around here. This helper function clobbers what was in COMPREPLY[@] before calling it, from a caller's point of view. Is it a pattern that potential callers in this file should already find familiar, and they do not have to be reminded that they cannot rely on what they prepared in COMPREPLY to be preserved across a call into this function? Otherwise I would suggest mentioning it in the helpful comment before the function, but I cannot tell if such a comment is even needed by the intended audience, so...
On 26-ene-2024 09:26:44, Junio C Hamano wrote: > Rubén Justo <rjusto@gmail.com> writes: > > > +# Completion for subcommands in commands that follow the syntax: > > +# > > +# git <command> <subcommand> > > +# > > +# 1: List of possible completion words. > > +# Returns false if the current word is not a possible subcommand > > +# (possitioned after the command), or if no option is found in > > +# the list provided. > > +__gitcomp_subcommand () > > +{ > > + local subcommands="$1" > > + > > + if [ $cword -eq $(($__git_cmd_idx + 1)) ]; then > > + __gitcomp "$subcommands" > > + > > + test -n "$COMPREPLY" > > + else > > + false > > + fi > > +} > > > I am not at all familiar with the code in this file, so treat this > as a question from somebody who do not know the calling convention > used around here. > > This helper function clobbers what was in COMPREPLY[@] before > calling it, from a caller's point of view. Is it a pattern that > potential callers in this file should already find familiar, and > they do not have to be reminded that they cannot rely on what they > prepared in COMPREPLY to be preserved across a call into this > function? Otherwise I would suggest mentioning it in the helpful > comment before the function, but I cannot tell if such a comment is > even needed by the intended audience, so... Maybe adding such a comment might suggest at first glance that we're working different here than in the rest of the __gitcomp_* family of functions, which is not the intention ... I dunno.
Rubén Justo <rjusto@gmail.com> writes: > On 26-ene-2024 09:26:44, Junio C Hamano wrote: >> Rubén Justo <rjusto@gmail.com> writes: >> >> > +# Completion for subcommands in commands that follow the syntax: >> > +# >> > +# git <command> <subcommand> >> > +# >> > +# 1: List of possible completion words. >> > +# Returns false if the current word is not a possible subcommand >> > +# (possitioned after the command), or if no option is found in >> > +# the list provided. >> > +__gitcomp_subcommand () >> > +{ >> > + local subcommands="$1" >> > + >> > + if [ $cword -eq $(($__git_cmd_idx + 1)) ]; then >> > + __gitcomp "$subcommands" >> > + >> > + test -n "$COMPREPLY" >> > + else >> > + false >> > + fi >> > +} >> >> >> I am not at all familiar with the code in this file, so treat this >> as a question from somebody who do not know the calling convention >> used around here. >> >> This helper function clobbers what was in COMPREPLY[@] before >> calling it, from a caller's point of view. Is it a pattern that >> potential callers in this file should already find familiar, and >> they do not have to be reminded that they cannot rely on what they >> prepared in COMPREPLY to be preserved across a call into this >> function? Otherwise I would suggest mentioning it in the helpful >> comment before the function, but I cannot tell if such a comment is >> even needed by the intended audience, so... > > Maybe adding such a comment might suggest at first glance that we're > working different here than in the rest of the __gitcomp_* family of > functions, which is not the intention ... I dunno. Exactly. That is why I asked. If it is a norm for all these helper functions to stomp on COMPREPLY and if it is accepted as a common pattern by developers who touch this file, then I agree it would be misleading to have such a comment only to this function. THanks.
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 8c40ade494..916e137021 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -554,6 +554,27 @@ __gitcomp_file () true } +# Completion for subcommands in commands that follow the syntax: +# +# git <command> <subcommand> +# +# 1: List of possible completion words. +# Returns false if the current word is not a possible subcommand +# (possitioned after the command), or if no option is found in +# the list provided. +__gitcomp_subcommand () +{ + local subcommands="$1" + + if [ $cword -eq $(($__git_cmd_idx + 1)) ]; then + __gitcomp "$subcommands" + + test -n "$COMPREPLY" + else + false + fi +} + # Execute 'git ls-files', unless the --committable option is specified, in # which case it runs 'git diff-index' to find out the files that can be # committed. It return paths relative to the directory specified in the first
Let's have a function to complete "subcommands" only in the correct position (right after the command), in commands that follow the syntax: git <command> <subcommand> Signed-off-by: Rubén Justo <rjusto@gmail.com> --- contrib/completion/git-completion.bash | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)