From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Fri, 30 Nov 2007 23:38:57 GMT
Subject: Re: [PATCH] Move all dashed form git commands to libexecdir
Message-ID: <Pine.LNX.4.64.0711302338350.27959@racer.site>
In-Reply-To: <20071130232127.GA3169@sigill.intra.peff.net>

Hi,

On Fri, 30 Nov 2007, Jeff King wrote:

> On Fri, Nov 30, 2007 at 11:05:50PM +0000, Johannes Schindelin wrote:
> 
> > > > >  	alias_command = (*argv)[0];
> > > > >  	git_config(git_alias_config);
> > > > > +	if (!alias_string)
> > > > > +		alias_string = builtin_alias(alias_command);
> > > > >  	if (alias_string) {
> > > > >  		if (alias_string[0] == '!') {
> > > > >  			if (*argcp > 1) {
> > > > 
> > > > Didn't you mean to put this _before_ the git_config() call?  As you wrote 
> > > > it, the "soft" alias overrides the user-specified one.
> > > 
> > > No. The "if (!alias_string)" means we only do the lookup if no user
> > > alias was found. Try it.
> > 
> > Ah.  To me, that was rather easy to miss, though...
> 
> I don't particularly care if it is re-written as:
> 
>   alias_string = builtin_alias(alias_command);
>   git_config(git_alias_config);
> 
> which should be equivalent.  I wrote it the original way to avoid doing
> the O(n) search through builtin aliases when it was unnecessary, but
> obviously this isn't a performance critical code path.

Actually, I felt/feel quite dumb missing it.

Ciao,
Dscho