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

From: SZEDER Gábor <szeder@ira.uka.de>
Date: Feb 24, 2010, 01:04 UTC
Message-ID: <20100224010459.GP4431@neumann>
In-Reply-To: <7v3a0rd2lz.fsf@alter.siamese.dyndns.org>

Hi,

On Tue, Feb 23, 2010 at 02:11:20PM -0800, Junio C Hamano wrote:

Show 17 quoted lines

> SZEDER Gábor <szeder@ira.uka.de> 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)