Re: [PATCH 2/3] diff: introduce diff.submodule configuration variable

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 13, 2012, 16:27 UTC
Message-ID: <7vlie5foyr.fsf@alter.siamese.dyndns.org>
In-Reply-To: <CALkWK0nJwznx36yoAUKXRnyA+32143tBVJHnnrosz-Ht7VhwHw@mail.gmail.com>

Ramkumar Ramachandra <artagnon@gmail.com> writes:

Show 15 quoted lines

> Jeff King wrote:
>> On Sun, Nov 11, 2012 at 10:29:05PM +0530, Ramkumar Ramachandra wrote:
>>> @@ -223,6 +238,15 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)
>>>               return 0;
>>>       }
>>>
>>> +     if (!strcmp(var, "diff.submodule")) {
>>
>> Shouldn't this be in git_diff_ui_config so it does not affect scripts
>> calling plumbing?
>
> I honestly didn't understand the difference between
> git_diff_basic_config and git_diff_ui_config.  The latter function
> calls the former function at the end of its body.  Why are they two
> separate functions in the first place?

In case you meant s/didn't/don't/, git_diff_ui_config() should be called only by human-facing Porcelain commands where their behaviours can and should be affected by end user configuration variables.

When a configuration variable should not affect output from plumbing commands like diff-files, diff-index, and diff-tree, it must not be read in git_diff_basic_config(), but in git_diff_ui_config().

The output from "git format-patch" is consumed by "git apply" that expects "Subproject commit %s\n" with fully spelled object name, so your configuration must not affect the output of format-patch, either.