Gábor Szeder <szeder@xxxxxxxxxx> writes:
> words[] is just fine, we never modify it after it is filled by
> _get_comp_words_by_ref() at the very beginning.
Hmph. I would have understood if the latter were "we never look at
it (to decide what to do)". "we never modify it" does not sound
like an enough justification behind "words[] is just fine"---maybe I
am not reading you correctly.
> The root of the problem is that the expected position of the name
> of the git command in __git_complete_remote_or_refspec() is
> hardcoded as words[1], but that is not the case when:
>
> 1) it's an alias, as in Felipe's example: git p ori<TAB>,
> because while the index is ok, the content is not.
>
> 2) in presence of options of the main git command: git -c
> foo=bar push ori<TAB>, because the index is off.
>
> 3) the command is a shell alias for which the user explicitly
> set the completion function with __git_complete() (at his own
> risk): alias gp="git push"; __git_complete gp _git_push; gp
> ori<TAB> Neither the index nor the content are ok.
>
> Fixing the hard-coded indexing would only solve 2) but not 1) and
> 3), as it obviously couldn't turn the git or shell alias into a
> git command on its own.
>
> Felipe's patch only deals with 1), as it only kicks in in case of
> a git alias.
Yeah, do completions for commands (not just for the ones that use
remote-or-refspec Felipe's patch addresses) have trouble with the
latter two in general? If that is the case,...
> Communicating the name of the git command to
> __git_complete_remote_or_refspec() by its callers via a new
> variable as suggested by Junio, or perhaps by an additional
> parameter to the function is IMHO the right thing to do, because,
> unless I'm missing something, it would make all three cases work.
... while the above analysis may be correct, taking Felipe's patch
to address only (1) and leaving a solution to the more general
words[1] problem for other patches on top might not be too bad an
approach.
Unless
(A) remote-or-refspec thing is the primary offender, and other
commands do not suffer from the words[1] problem, in which case
I tend to agree that an additional parameter would be the way
to go (there are only a few callers of the function); or
(B) even if words[1] problem is more widespread, such a more
general solution to all three issues can be coded cleanly and
quickly, without having to have Felipe's patch as a stop-gap
measure.
that is.
I'll keep Felipe's patch in the meantime to 'pu'.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html