Re: [BUG] completion.commands does not remove multiple commands

["SZEDER Gábor" <szeder.dev@gmail.com>]

On Fri, Mar 01, 2019 at 09:40:12PM -0500, Todd Zullinger wrote:
> Hi,
> 
> Junio C Hamano wrote:
> > Todd Zullinger <tmz@pobox.com> writes:
> > 
> >> Hmm.  The comments in list_cmds_by_config() made me wonder
> >> if not using a local repo config was intentional:
> >>
> >>         /*
> >>          * There's no actual repository setup at this point (and even
> >>          * if there is, we don't really care; only global config
> >>          * matters). If we accidentally set up a repository, it's ok
> >>          * too since the caller (git --list-cmds=) should exit shortly
> >>          * anyway.
> >>          */
> > 
> > Doesn't the output from list-cmds-by-config get cached at the
> > completion script layer in $__git_blah variables, and the cached
> > values are not cleared when you chdir around?
> 
> In testing, I didn't find any evidence of caching.  Setting
> commands to be added and removed in the global and local
> configs worked reasonably.
> 
> Duy's reply suggests that was considered but not
> implemented.  I there were caching (and if it were tedious
> for the completion code to keep fresh between repos), then
> it would a bad plan to allow per-repo config.

The completion script used to cache the list of porcelain commands,
but then Duy came along and removed it in 3301d36b29 (completion: add
and use --list-cmds=alias, 2018-05-20).

We cached the commands, because it was a bit involved to get them:
first we run 'git help -a' to list all commands, then extracted the
command names from the help output with 'grep', and finally an
enormous case statement removed all plumbing commands.  Caching spared
us the overhead of these external processes and maybe 2-3 subshells.
Note that because of this caching if you dropped a third-party
'git-foo' command in your $PATH, it didn't show up in completion until
you re-sourced the completion script, which was the standard way to
invalidate/refresh the caches.

However, even when we cached porcelain commands, we didn't cache
aliases, even though getting them is a bit involved as well, requiring
running 'git config', two subshells and a shell loop.  I presume that
back then the reason for not caching aliases was that they could be
repo-specific.

Now, ever since Duy revamped commands completion, we only run a simple
$(git --list-cmds=...), i.e. a git command and a subshell takes care
of all that.  IMO this is the best we ever had, because it uses one
less subshell than before to list both commands and aliases, and it
lists everything afresh, including third-party 'git-foo' commands.

I don't see the benefit of bringing back caching.  Yes, on one hand we
could complete commands with a single variable expansion, which is
clearly faster than that $(git --list-cmds=...).  OTOH, that's only
commands, but what about aliases?  If we cache aliases, too, then that
could be considered a regression by those who do have repo-specific
aliases; if we don't, then we are back at running 'git config' and two
subshells, i.e. one subshell more than we currently have.

And if we won't cache commands, then we might as well modify
list_cmds_by_config() to read 'completion.commands' from the
repo-specific config, too.  I'm fairly neutral on this, because I
can only imagine rather convoluted scenarios where a repo-specific
command list would be useful, but at least it would make it consistent
with aliases, which we read from the current repo's config and we
always have.


Note, however, that for completeness sake, if we choose to update
list_cmds_by_config() to read the repo's config as well, then we
should also update the completion script to run $(__git
--list-cmds=...), note the two leading underscores, so in case of 'git
-C over/there <TAB>' it would read 'completion.commands' from the right
repository.