Re: [PATCH v1 0/3] Introduce config variable "diff.primer"

From: Jeff King <peff@peff.net>
Date: Jan 25, 2009, 22:07 UTC
Message-ID: <20090125220756.GA18855@coredump.intra.peff.net>
In-Reply-To: <alpine.GSO.2.00.0901251239000.12651@kiwi.cs.ucla.edu>

On Sun, Jan 25, 2009 at 12:41:21PM -0800, Keith Cascio wrote:

Show 9 quoted lines

> > I am puzzled.
> > 
> > The gitattributes mechanism is about per-path settings, but I do not
> > think a mnemonicprefix that is per-path makes much sense.
> 
> That was just an example (perhaps poorly chosen).  What I meant to
> suggest is making gitattributes consistent with gitconfig WRT at least
> the [diff] section.  But maybe that's not appropriate.  Thanks for the
> insight.

I don't think you want it entirely consistent. What would diff.renamelimit mean in the context of a gitattribute? But I do think it makes sense for some (like specific diff options such as whitespace handling).

Also, if you're going to have options that apply to gitattributes diff drivers _and_ as a general fallback, I think we need to define when the fallback kicks in. That is, let's say I have a gitattributes file like this:

   *.c diff=c

and my config says:

  [diff]
    opt1 = val1_default
    opt2 = val2_default

  [diff "c"]
    opt1 = val1_c

Now obviously if I want to use opt1 for my C files, it should be val1_c. But if I want to use opt2, what should it use? There are two reasonable choices, I think:

  1. You use val2_default. The rationale is that the "c" diff driver did
     not define an opt2, so you fall back to the global default.

  2. It is unset. The rationale is that you are using the "c" diff
     driver, and it has left the value unset. The default then means "if
     you have no diff driver setup".

I suspect "1" is what people would want most of the time, but "2" is actually more flexible (since there is otherwise no way to say "I explicitly left diff.c.opt2 unset").

If (2) is desired, I think it makes more sense to put such "default" options into their own diff driver section. Like:

  [diff "default"]
    opt2 = whatever

And then it is more clear that once you have selected the "c" diff driver, the values in the other "default" are not relevant.

I don't think this is a huge issue overall, but it occurs to me that we have just added diff.wordRegex and diff.*.wordRegex. So it makes sense to think for a minute which behavior we want before it ships and we are stuck with backwards compatibility forever.

-Peff