Re: [PATCH] pretty: allow to override the built-in formats

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 9, 2020, 18:27 UTC
Message-ID: <xmqqtuw6vm8w.fsf@gitster.c.googlers.com>
In-Reply-To: <20200909090842.GA2496536@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 20 quoted lines

> On Tue, Sep 08, 2020 at 11:12:11AM -0700, Junio C Hamano wrote:
>
>> > Here's a slightly different proposal. I'm not sure if I like it or not,
>> > but just thinking out loud for a moment. The issue is that we're worried
>> > the consumer of the output may be surprised by a user-configured pretty
>> > format. Can we give them a way to say "I don't care about the exact
>> > output; pick what the user configured for this name, or some sane
>> > default". I.e., something like:
>> >
>> >   git log --format=loose:reference
>> 
>> Yeah, that, or with s/loose/user/ or something.
>
> Heh, I actually called it "user:" initially but wasn't sure if that was
> sufficiently descriptive, so I groped around for another word. But if
> both of us thought of "user", maybe it's better.
>
> At any rate, this was mostly just thinking out loud, and isn't something
> I'm planning to follow up on with a patch. But maybe it inspires
> somebody to run with it.

Of course, we could go the other way and follow the same approach as the "--literal-pathspecs" feature (and what bash does with the alias and uses "command" keyword to work around the confusion it causes).

IOW, we could force those scripts that want to be strict to pay the price and be explicit (e.g. "--format=builtin:<name>") and allow others that want to be affected by end user customization can keep saying "--format=<name>". It unfortunately breaks our long standing stance against backward compatibility breaking changes, so I would say it is not likely to happen. "--format=loose:reference" does not share the problem, and it is much safer.

In any case, I do not think "pretty.override<word>" configuration variable, which warns with the 'override' and makes those who tweak builtin format think twice, is a good idea. Those who add the custom configuration are not in the position to decide if a particular use of --format=<word> in a script (like gitk[*1*]) should or should not be affected by customization. It is up to the script writers [*2*].

[Footnote]

*1* We've been using gitk as an example but it is not the best one,
    since it was made crystal clear that gitk will not be accepting a
    single liner patch that uses --pretty=reference anyway, it is a
    moot point.

    cf. https://lore.kernel.org/git/20191211215826.GA31614@blackberry/

*2* In general, a script wants strict/builtin output if it captures
    and parses, and customizable output if it just lets the git
    command it calls directly talk to the end user.