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

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

quoted

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