bug: compactionheuristic config var case issue

bug: compactionheuristic config var case issue

[Brian Lalor]

I’m very happy to see the new compaction heuristic option; it’s the way I always thought diffs should read!  

The config option in the documentation references “diff.compactionHeuristic”, but diff.c does a case-sensitive comparison on “diff.compactionheuristic” (note the case of the “h” in “heuristic”) and `git diff` does not honor the config.  Confusingly, `git config diff.compactionheuristic` returns true when diff.compactionHeuristic is set in ~/.gitconfig.  When diff.compactionheuristic is set to true in ~/.gitconfig, the desired behavior is achieved.

Thank you all for Git: it’s hard to remember the terrible world we lived in before it existed. :-)

Re: bug: compactionheuristic config var case issue

[Duy Nguyen]

On Wed, Jun 15, 2016 at 5:39 PM, Brian Lalor <blalor@bravo5.org> wrote:
> I’m very happy to see the new compaction heuristic option; it’s the way I always thought diffs should read!
>
> The config option in the documentation references “diff.compactionHeuristic”, but diff.c does a case-sensitive comparison on “diff.compactionheuristic” (note the case of the “h” in “heuristic”)

I think this misled you. All configuration variable names are
lower-cased before they reach that strcmp() call, the whole picture is
more like strcmp(tolower(var), "diff.compactionheuristic"), which I
believe is correct.

> and `git diff` does not honor the config.  Confusingly, `git config diff.compactionheuristic` returns true when diff.compactionHeuristic is set in ~/.gitconfig.  When diff.compactionheuristic is set to true in ~/.gitconfig, the desired behavior is achieved.
>
> Thank you all for Git: it’s hard to remember the terrible world we lived in before it existed. :-)
-- 
Duy

Re: bug: compactionheuristic config var case issue

[Jeff King]

On Wed, Jun 15, 2016 at 07:33:31PM +0700, Duy Nguyen wrote:

> On Wed, Jun 15, 2016 at 5:39 PM, Brian Lalor <blalor@bravo5.org> wrote:
> > I’m very happy to see the new compaction heuristic option; it’s the way I always thought diffs should read!
> >
> > The config option in the documentation references
> > “diff.compactionHeuristic”, but diff.c does a case-sensitive
> > comparison on “diff.compactionheuristic” (note the case of the “h”
> > in “heuristic”)
> 
> I think this misled you. All configuration variable names are
> lower-cased before they reach that strcmp() call, the whole picture is
> more like strcmp(tolower(var), "diff.compactionheuristic"), which I
> believe is correct.

Yep, that is correct. Config keys are case-insensitive (except for the
middle portion "Y" of a key like "X.Y.Z"), and the downcasing happens
before they even hit the config callbacks.

> > and `git diff` does not honor the config.  Confusingly, `git config
> > diff.compactionheuristic` returns true when diff.compactionHeuristic
> > is set in ~/.gitconfig.  When diff.compactionheuristic is set to
> > true in ~/.gitconfig, the desired behavior is achieved.

Brian, do you have a case you can share where it is not working as
expected?

Config isn't enabled automatically for plumbing commands like diff-tree.
So some commands may not respect it, but "git diff" definitely should.

-Peff