Re: git diff-tree do not honor diff.orderfile config

From: Jeff King <peff@peff.net>
Date: Jul 6, 2024, 07:02 UTC
Message-ID: <20240706070201.GA702644@coredump.intra.peff.net>
In-Reply-To: <Zohf7LzaRHaG9vX6@nand.local>

On Fri, Jul 05, 2024 at 05:04:44PM -0400, Taylor Blau wrote:

Show 14 quoted lines

> On Fri, Jul 05, 2024 at 11:28:23AM +0100, Tiago de Bem Natel de Moura wrote:
> > What did you do before the bug happened? (Steps to reproduce your issue)
> > The config `diff.orderfile` is not being honored in the `git diff-tree` command
> > as stated by the documentation.
> 
> Makes sense... the diff.orderFile configuration is part of the "UI" set
> of diff.* configuration options, which are honored by porcelain commands
> like commit, diff, log, etc., but not by plumbing commands like
> diff-tree.
> 
> I think from 6d8940b562 (diff: add diff.orderfile configuration
> variable, 2013-12-18) adding it to the UI-only configuration set was
> intentional, but it is somewhat awkward that we respect -O but not the
> configuration it falls back on.

I don't think it's awkward at all. The plumbing commands should provide most every option that the porcelain ones support, but should do so based only on the options given by the caller. I.e., imagine you were writing "git diff" (or a similar command) as a script and you wanted to support something like diff.orderfile. You'd do it by parsing diff.orderfile yourself and passing "-O" where it made sense to do so.

> So I suppose the question is whether supporting -O from diff-tree is
> sensible. If it is, then reading the diff.orderFile configuration option
> is a no-brainer. But if it isn't, then we should probably not make a bad
> situation worse by adding support for it.

No, I don't think we ever want diff-tree (or other plumbing commands) to support diff.orderFile. It will cause unexpected results for consumers of the plumbing output. E.g., it would probably at least screw up patch-id.

So this all just looks like the usual "plumbing does not respect some config" situation.

-Peff