Re: [PATCH 1/3] setup: Provide GIT_PREFIX to built-ins

From: David Aguilar <davvid@gmail.com>
Date: May 25, 2011, 04:19 UTC
Message-ID: <20110525041926.GB21810@gmail.com>
In-Reply-To: <BANLkTi=ssDA=y1CnMAZtvk6dTyMmd4LjrQ@mail.gmail.com>

On Mon, May 23, 2011 at 02:09:40PM +0200, Ævar Arnfjörð Bjarmason wrote:

Show 6 quoted lines

> On Sun, May 22, 2011 at 11:57, David Aguilar <davvid@gmail.com> wrote:
> > +       printf "#!/bin/sh\n" >diff &&
> > +       printf "printf \"\$GIT_PREFIX\"\n" >>diff &&
> 
> If you're going to use /bin/sh (which might be a non-POSIX shell) it's
> probably better to use echo than rely on printf understanding \n.

I'll reroll a v2 of these patches using echo instead of printf. The mergetool--lib patch will make the test -z "$GIT_PREFIX" check happen unconditionally as you suggested, Junio.

Another thought was that I could have implemented the mergetool--lib patch without $GIT_PREFIX at all and just called rev-parse --show-prefix explicitly. The change has merits, though, and I'll consider mergetool--lib not using rev-parse as an optimization. Afterall, fork+exec is expensive on Windows so doing without an additional call is nicer for our msysgit brothers.

Thank you both.

-- 
					David