Re: [PATCH 2/3] Documentation: Add diff.<driver>.* to config

[Junio C Hamano <gitster@pobox.com>]

Michael J Gruber <git@drmicha.warpmail.net> writes:

>> ...
>> +diff.<driver>.xfuncname::
>> +	Defines the regular expression that the custom diff driver
>> +	should use to recognize the hunk header.  A built-in pattern
>> +	may also be used.  See linkgit:gitattributes[5] for details.
>
> The regular...

I agree with your "Defines..." comment.

>> +diff.<driver>.binary::
>> +	Set this option to true to make the custom diff driver treat
>> +	files as binary.  See linkgit:gitattributes[5] for details.
>
> This has nothing to do with the diff driver's operation. It is about how
> git treats the result (the output from the diff driver):

In general, the use of "diff.<driver>.foo" is consistent with the current
terminology, which is coming from the description of <driver> in the
gitattributes documentation:

   `diff`
   ^^^^^^

   The attribute `diff` affects how 'git' generates diffs for particular
   files. It can tell git whether to generate a textual patch for the path
   or to treat the path as a binary file.  It can also affect...

   Set::
   ...
   String::

           Diff is shown using the specified diff driver.  Each driver may
           specify one or more options, as described in the following
           section. The options for the diff driver "foo" are defined
           by the configuration variables in the "diff.foo" section of the
           git config file.

When the 'diff' attribute was invented, its string value was meant to say
more than "do generate diff for this path" (set) vs "don't generate diff
for this path" (unset) by saying what _kind_ of file it is and switch the
backend driver to generate diff based on that _kind_.  We could call this
"kind" a file-type, but that is a bit too broad a word; we are not talking
about regular file vs symbolic link or executable vs non-executable.  This
is about letting us determine the type of the _content_ more explicitly
than the case where the attribute is Unspecified and we inspect the
content to determine the type.  Perhaps we could have called this
content-type but again that word has other established meaning.

In any case, as you point out, the description itself is not quite right
from the end-user's point of view, even though it is correct at the
implementation level.

This diff.<driver>.binary variable is a somewhat ugly workaround for
content types that want to be treated as if they are binary while "diff"
(usually you would say "-diff" in gitattributes file for such a path)
works, but still want non-diff users (e.g. "cat-file --textconv") of the
textconv filter to still apply the specified text conversion.  In order to
specify what text conversion to apply, you would need a content-type to do
so, but once you specify a content-type, "diff" will not treat them as
binary anymore, so you tell the "diff" machinery by setting this variable.

At the implementation level, you are telling the <driver>, which is
defined as an array of operations indexed by content-types, to respond to
requests to "diff" by producing "binary files differ", so in that sense,
it is telling "the custom diff driver" to "treat files as binary".

>> +diff.<driver>.textconv::
>> +	Defines the command that the custom diff driver should call to
>> +	generate the text-converted version of a binary file.  The
>> +	result of the conversion is used to generate a human-readable
>> +	diff.  See linkgit:gitattributes[5] for details.
>
> No, please! You don't need a custom diff driver for textconv.

Correct.  Probably we should abolish "custom" from the above description.
Also "binary" is unnecessary in "version of a binary file" above, as it is
perfectly sensible to run ps2ascii for a text postscript file.

>> +diff.<driver>.wordregex::
>> +	Defines the regular expression that the custom diff driver
>> +	should use to split words in a line.  See
>> +	linkgit:gitattributes[5] for details.
>> +
>> +diff.<driver>.cachetextconv::
>> +	Set this option to true to make the custom diff driver cache
>> +	the text conversion outputs.  See linkgit:gitattributes[5] for
>> +	details.
>
> Again, both are independent of a custom diff driver.

Just drop "custom".

> ... Maybe even <driver>
> is misleading here, I dunno.

Perhaps.  See the "file-type" and "content-type" discussion above.