Subject: Re: [PATCH 1/4] bash: improve aliased command recognition

From: SZEDER Gábor <[email-masked]>
Date: Wed, 24 Feb 2010 02:04:59 +0100

Hi,

On Tue, Feb 23, 2010 at 02:11:20PM -0800, Junio C Hamano wrote:
> SZEDER Gábor <[email-masked]> writes:
> 
> > [alias]
> >     lgm = "!sh -c 'GIT_NOTES_REF=refs/notes/amlog git log \"$@\" || :' -"
> >
> > The full parsing of a shell command alias like that in the completion
> > code is clearly unfeasible.  However, we can easily improve on aliased
> > command recognition by eleminating stuff that is definitely not a git
> > command: shell commands (anything starting with '!'), command line
> > options (anything starting with '-'), environment variables (anything
> > with a '=' in it), and git itself.  This way the above alias would be
> > handled correctly, and the completion script would correctly recognize
> > "log" as the aliased git command.
> 
> I personally do not think such a heuristic is worth the trouble (both for
> writing and maintaining the completion code nor runtime overhead to
> iterate over words on the expansion).

Well, the code is already written, so ;)

Runtime overhead seems not to be an issue:

$ git config alias.shortalias log
$ git config alias.longalias \!"sh -c '$(for i in $(seq 1 100) ;do echo -n "A=$i " ;done) git log -1'"
$ time __git_aliased_command shortalias
log

real    0m0.008s
user    0m0.004s
sys     0m0.004s
$ time __git_aliased_command longalias
log

real    0m0.017s
user    0m0.012s
sys     0m0.004s

That is, although it takes around twice as long in case of a 100+ word
long alias than with a trivial one, it is still in the hundredth of a
second range.  And I doubt that many people have that long aliases.

Maintenance might be an issue, or, erm...  well, it already is.  An
alias like "!sh -c 'git log -1'" does not work, because "'git" does
not match any of the patterns, therefore it is returned as the aliased
command.

> I vaguely recall somebody floated an idea to tell completion code that
> "you may not know what lgm is, but it takes the same set of options as
> log" (either via config or a shell function---I don't recall the details).

Shawn proposed the approach via config variables and patches 2/4 and
3/4 actually implement the shell function approach.

Personally, I prefer the shell function approach.  It needs less 'git
config' queries; actually, in this respect it is better than current
master, because there is no 'git config' query at all for the git
command case.  Furthermore, I don't really like the idea of putting
completion related stuff into git configuration files, but this is, of
course, subjective.

> I think that would be a lot more robust, efficient and easy to explain
> solution to the same problem.

I agree that this heuristic is not 100% robust.  Efficiency is not an
issue, as shown above.  But I think we should also look at efficiency
and overhead at the user, too.  That is, this heuristic will work
without any action required from the user, while the other two
approaches require the user to explicitly specify the completion rules
for his non-trivial aliases.  Finally, I think it is not all that
difficult to explain:

  "The bash completion script uses some heuristics to find out the git
  command invoked by aliases.  If you have an alias for which the
  completion script does nothing or outputs garbage, then you should
  write a one-liner shell function, which ..." and here comes what you
  would need to explain anyway.

Note, that patches 1/4 and 4/4 are independent from the changes in 2/4
and 3/4, so if you are not satisfied with these heuristic changes, you
can still drop only those but not the custom completion patches.

Best,
Gábor

(and it's already tomorrow here, so the thoughts about completion
namespaces will have to wait)