threads / bug / 45797

BUG: diff-{index,files,tree} (and git-gui) do not respect the diff.indentHeuristic config setting

Subject: BUG: diff-{index,files,tree} (and git-gui) do not respect the diff.indentHeuristic config setting

## tl;dr

2 messages between Apr 25, 2017 and Apr 25, 2017.

replies: 1people: 2as markdown or json

Marc Branchaud· Apr 25, 2017, 17:21 UTC · lore
So I have
	diff.indentHeuristic = true

and I noticed that git-gui was not using the heuristic. This is because git-gui uses diff-index, and that does not respect the config setting, even though it supports the --indent-heuristic option.

And it looks like diff-files and diff-tree also have the same problem.

I tried a couple of quick-n-dirty things to fix it in diff-index, without success, and I've run out of git-hacking tame, so all I can do for now is throw out a bug report.

diff-index.c explicitly says "no 'diff' UI options" since 83ad63cfeb ("diff: do not use configuration magic at the core-level", 2006-07-08), so maybe this needs to be fixed in git-gui (and maybe elsewhere), but to me it feels like the diff-foo commands should respect the setting.

(CC'ing Michael Haggerty, who added the heuristic.)
(This is git v2.12.2.)
		M.
Jeff King· Apr 25, 2017, 21:27 UTC · re: Marc Branchaud · lore

Re: BUG: diff-{index,files,tree} (and git-gui) do not respect the diff.indentHeuristic config setting

On Tue, Apr 25, 2017 at 01:21:09PM -0400, Marc Branchaud wrote:
Show 13 quoted lines
> So I have
> 
> 	diff.indentHeuristic = true
> 
> and I noticed that git-gui was not using the heuristic.  This is because
> git-gui uses diff-index, and that does not respect the config setting, even
> though it supports the --indent-heuristic option.
> 
> And it looks like diff-files and diff-tree also have the same problem.
> 
> I tried a couple of quick-n-dirty things to fix it in diff-index, without
> success, and I've run out of git-hacking tame, so all I can do for now is
> throw out a bug report.

Right, this is intentional. Those commands are scriptable plumbing, and we try to avoid surprising their callers with behavior-changing config.

I could see an argument that the behavior change for this particular option is subtle enough that it any script caller would not need to care. The resulting diff is syntactically identical, and it's not like Git makes promises about the exact diff algorithm it uses. We generally tend to err on the side of caution with plumbing, though (for instance, if you piped the result through patch-id, you'd probably get different results with and without the config option set).

> diff-index.c explicitly says "no 'diff' UI options" since 83ad63cfeb ("diff:
> do not use configuration magic at the core-level", 2006-07-08), so maybe
> this needs to be fixed in git-gui (and maybe elsewhere), but to me it feels
> like the diff-foo commands should respect the setting.

Yes, if git-gui wants to respect the option, it needs to read the config and turn on the command-line option. That's what add--interactive does, for example.

It's possible we could make this simpler with a catch-all "allow diff ui config" command-line option. The effect is the same, but it makes things simpler for the caller.

-Peff

← back to recent threads