Re: [PATCH 1/4] completion: introduce __gitcomp_subcommand

[Junio C Hamano <gitster@pobox.com>]

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.