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

From: Stefan Beller <sbeller@google.com>
Date: Apr 28, 2017, 17:34 UTC
Message-ID: <CAGZ79kbUqVfz+6Y0XkTL7FCZfaD+2YRMZ_v0vP8-DOFhWc+ELw@mail.gmail.com>
In-Reply-To: <20170427205037.1787-1-marcnarc@xiplink.com>

On Thu, Apr 27, 2017 at 1:50 PM, Marc Branchaud <marcnarc@xiplink.com> wrote:

Show 28 quoted lines

> 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 <sbeller@google.com>
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 <sbeller@google.com>
---

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;