RE: [PATCH] parse-options: don't complete option aliases by default

[Felipe Contreras <felipe.contreras@gmail.com>]

Philippe Blain via GitGitGadget wrote:
> From: Philippe Blain <levraiphilippeblain@gmail.com>
> 
> Since 'OPT_ALIAS' was created in 5c387428f1 (parse-options: don't emit
> "ambiguous option" for aliases, 2019-04-29), 'git clone
> --git-completion-helper', which is used by the Bash completion script to
> list options accepted by clone (via '__gitcomp_builtin'), lists both
> '--recurse-submodules' and its alias '--recursive', which was not the
> case before since '--recursive' had the PARSE_OPT_HIDDEN flag set, and
> options with this flag are skipped by 'parse-options.c::show_gitcomp',
> which implements 'git <cmd> --git-completion-helper'.
> 
> At the point where 'show_gitcomp' is called in 'parse_options_step',
> 'preprocess_options' was already called in 'parse_options', so any
> aliases are now copies of the original options with a modified help text
> indicating they are aliases.
> 
> Helpfully, since 64cc539fd2 (parse-options: don't leak alias help
> messages, 2021-03-21) these copies have the PARSE_OPT_FROM_ALIAS flag
> set, so check that flag early in 'show_gitcomp' and do not print them,

Makes sense but I don't think what the patch is doing should be buried
here. I think a separate paragraph explaining what you are trying to do
will be better.

> unless the user explicitely requested that *all* completion be shown (by
> setting 'GIT_COMPLETION_SHOW_ALL'). After all, if we want to encourage
> the use of '--recurse-submodules' over '--recursive', we'd better just
> suggest the former.
> 
> The only other options alias is 'log' and friends' '--mailmap', which is
> an alias for '--use-mailmap', but the Bash completion helpers for these
> commands do not use '__gitcomp_builtin', and thus are unnaffected by
> this change.
> 
> Test the new behaviour in t9902-completion.sh. As a side effect, this
> also tests the correct behaviour of GIT_COMPLETION_SHOW_ALL, which was
> not tested before. Note that since '__gitcomp_builtin' caches the
> options it shows, we need to re-source the completion script to clear
> that cache for the second test.

I agree this is better, and the patch itself looks obviously correct.

Reviewed-by: Felipe Contreras <felipe.contreras@gmail.com>

-- 
Felipe Contreras