Thread (30 messages) 30 messages, 5 authors, 2017-05-09

Re: [PATCH 0/2] Make diff plumbing commands respect the indentHeuristic.

From: Stefan Beller <hidden>
Date: 2017-04-28 17:34:22
Subsystem: the rest · Maintainer: Linus Torvalds

On Thu, Apr 27, 2017 at 1:50 PM, Marc Branchaud [off-list ref] wrote:
So here's my attempt at fixing this.

The thing I was missing is that init_revisions() calls diff_setup(), which
sets the xdl options.  It's therefore necessary to have the
diff_indent_heuristic flag set before calling init_revisions().

A naive way to get the indentHeuristic config option respected in the
diff-* plumbing commands is to make them use the git_diff_heuristic_config()
callback right at the start of their main cmd functions.

But I did not like that for two reasons:

* It would make these commands invoke git_config() twice.

* It doesn't avoid the problem if/when someone creates a new diff-something
  plumbing command, and forgets to set the diff_indent_heuristic flag before
  calling init_revisions().

So instead I chose to make the indentHeuristic option part of diff's basic
configuration, and in each of the diff plumbing commands I moved the call to
git_config() before the call to init_revisions().

This still doesn't really future-proof things for possible new diff plumbing
commands, because someone could still invoke init_revisions() before setting
up diff's basic configuration.  But I don't see an obvious way of ensuring
that the diff_indent_heuristic flag is respected regardless of when
diff_setup() is invoked.
The feature was included in v2.11 (released 2016-11-29) and we got no
negative feedback. Quite the opposite, all feedback we got, was positive.
This could be explained by having the feature as an experimental feature
and users who would be broken by it, did not test it yet or did not speak up.

So I'd propose to turn it on by default and anyone negatively impacted by that
could then use the config to turn it off for themselves (including plumbing).

Something like this, maybe?

---8<---
From 58d9a1ef135eff9f85b165986e4bc13479914f8e Mon Sep 17 00:00:00 2001
From: Stefan Beller <redacted>
Date: Thu, 27 Apr 2017 14:26:59 -0700
Subject: [PATCH] diff.c: enable indent heuristic by default

The feature was included in v2.11 (released 2016-11-29) and we got no
negative feedback. Quite the opposite, all feedback we got was positive.

Turn it on by default. users who dislike the feature can turn it off
using by setting diff.compactionHeuristic (which includes plumbing
commands, see prior patches)

Signed-off-by: Stefan Beller <redacted>
---
diff --git a/diff.c b/diff.c
index 11eef1c85d..7f301c1b62 100644
--- a/diff.c
+++ b/diff.c
@@ -27,7 +27,7 @@
 #endif

 static int diff_detect_rename_default;
-static int diff_indent_heuristic; /* experimental */
+static int diff_indent_heuristic = 1;
 static int diff_rename_limit_default = 400;
 static int diff_suppress_blank_empty;
 static int diff_use_color_default = -1;
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help