Re: [PATCH] git-web--browse: invoke kfmclient directly

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 19, 2011, 20:42 UTC
Message-ID: <7v62ko9scw.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20110919182049.GA26115@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 16 quoted lines

> On Mon, Sep 19, 2011 at 10:57:37AM -0700, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>> 
>> > Hmm. Actually, the one for custom browser commands might need it,
>> > because that one is expected to be a shell snippet. I suspect the
>> > simplest thing is to do something like:
>> >
>> >   eval "$browser_cmd \"\$@\""
>> 
>> Yeah, I agree, and the dq around $browser_cmd is kind of important, too,
>> for that to work and be readable.
>
> Oops, good catch. Probably the most readable version would be:
>
>   eval "\"$browser_cmd\"" '"$@"'

Actually I didn't mean that double dq.

In fact, if browser_cmd is meant to be split as a shell snippet, I do not think you want the string seen by eval to have dq around the expanded version of $browser_cmd. And I tend to prefer feeding a single string to eval, so the version in your message I quoted originally looks good to me.

Unless I am missing something here...?