From: Junio C Hamano Date: Tue, 13 Nov 2012 16:27:08 GMT Subject: Re: [PATCH 2/3] diff: introduce diff.submodule configuration variable Message-ID: <7vlie5foyr.fsf@alter.siamese.dyndns.org> In-Reply-To: Ramkumar Ramachandra writes: > 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.