Re: [PATCH v2 10/10] git.c: ignore pager.* when launching builtin as dashed external

From: Martin Ågren <martin.agren@gmail.com>
Date: Jul 31, 2017, 17:42 UTC
Message-ID: <CAN0heSr0RFAuRhw9UQaewWWtHv7=EMXqHyMBOD7ARrpM=P1-Qw@mail.gmail.com>
In-Reply-To: <20170731034507.2fdjnvgzzt4rgvas@sigill.intra.peff.net>

On 31 July 2017 at 05:45, Jeff King <peff@peff.net> wrote:

Show 13 quoted lines

> On Mon, Jul 17, 2017 at 10:10:52PM +0200, Martin Ågren wrote:
>
>> One could address this in run_argv(), by making the second call to
>> execv_dashed_external() conditional on "!is_builtin()" whereas a builtin
>> would be started as "git foo". (Possibly after unrolling and cleaning up
>> the "while (1)"-loop.) That seems like the wrong fix for this particular
>> issue, but might be a wanted change on its own -- or maybe not --, since
>> it would mean one could relay, e.g., "-c baz" to "git -c baz foo" (but
>> only for builtins...).
>
> We shouldn't need to relay them. They get added to the environment by
> the initial "git" invocation, and then are available everywhere (in
> fact, it would be wrong to relay them for multi-valued config).

Thanks for explaining. I did some very sloppy reading of the comment in git.c that we "cannot take flags in between the 'git' and the 'xxxx'" which I somehow misunderstood completely as "we cannot pass that sort of information to git-xxxx". Silly. Thanks for taking the time to explain what I should have found out myself...

So yeah, I meant the above and not this:

Show 13 quoted lines

> Or did
> you mean that we could potentially allow:
>
>   [alias]
>   foo = "-c baz some-builtin"
>
> That's interesting, but I think the fact that it only works with
> builtins makes it a bad idea. And you can always do:
>
>   [alias]
>   foo = "!git -c baz some-builtin"
>
> which is equivalent.