Re: [RFH] Mention of 1.7.0 transition plans in Release Notes to 1.6.6

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 29, 2009, 07:24 UTC
Message-ID: <7vmy25dc9h.fsf@alter.siamese.dyndns.org>
In-Reply-To: <7vfx7yfetb.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

Show 6 quoted lines

>  - I do not think of a sane way to cover "diff -b/-w" changes, as this is
>    a "bugfix --- but there may be some scripts that have been relying on
>    the bug", and a configuration option that retains the buggy behaviour
>    does not make much sense.  But I may be mistaken and somebody can come
>    up with an easy patch to allow both behaviour, in which case we should
>    add similar anti-procrastination measure to this change.

The patch to enable/disable this feature would look like the attached, which doesn't look too bad. This apply to a merge between two topics:

    jc/1.7.0-diff-whitespace-only-status (97bf2a0)
    gb/1.7.0-diff-whitespace-only-output (3e97c7c)

An anti-procrastination measure must be done in at least two, but probably in three steps on top of this patch:

 #1. Flip the default in this patch back to 1, as the traditional
     behaviour is to treat ignore-whitespace options as purely affecting
     output of patch text, not affecting status nor the header;

 #2. Add extra logic to detect if a particular invocation may trigger a
     different behaviour once the default is changed to 0, and issue an
     warning if there is no configuration explicitly set;

 #3. Flip the default to 0, perhaps still keeping the warning when there
     is no configuration.

Make a major release after the second step is done, and wait for at least one but preferrably two releases, and then ship the result of the third step in the "flag day" release. In a much later version we would remove the "no configuration warning".

Do we need an anti-procrastination measure for this? If so, I think we probably need to postpone the "diff" semantics changes after 1.7.0, as I am not very confident that we can solidly finish the second step before the 1.6.6 final.

Also, I am not very keen on having this configuration, as its only purpose is to ask for a broken semantics from "diff", even though it has been the traditional behaviour for the past four years.

Comments?

---
 diff.c |   15 +++++++++++----
 1 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/diff.c b/diff.c
index da90e6e..6e7c47c 100644
--- a/diff.c
+++ b/diff.c
@@ -29,6 +29,7 @@ static const char *diff_word_regex_cfg;
 static const char *external_diff_cmd_cfg;
 int diff_auto_refresh_index = 1;
 static int diff_mnemonic_prefix;
+static int diff_b_w_output_only;
 
 static char diff_colors[][COLOR_MAXLEN] = {
 	GIT_COLOR_RESET,
@@ -133,6 +134,11 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)
 		return 0;
 	}
 
+	if (!strcmp(var, "diff.bwoutputonly")) {
+		diff_b_w_output_only = git_config_bool(var, value);
+		return 0;
+	}
+
 	return git_color_default_config(var, value, cb);
 }
 
@@ -1668,7 +1674,7 @@ static void builtin_diff(const char *name_a,
 		struct emit_callback ecbdata;
 		const struct userdiff_funcname *pe;
 
-		if (!DIFF_XDL_TST(o, WHITESPACE_FLAGS)) {
+		if (!DIFF_XDL_TST(o, WHITESPACE_FLAGS) || diff_b_w_output_only) {
 			fprintf(o->file, "%s", header.buf);
 			strbuf_reset(&header);
 		}
@@ -2537,9 +2543,10 @@ int diff_setup_done(struct diff_options *options)
 	 * inside contents.
 	 */
 
-	if (DIFF_XDL_TST(options, IGNORE_WHITESPACE) ||
-	    DIFF_XDL_TST(options, IGNORE_WHITESPACE_CHANGE) ||
-	    DIFF_XDL_TST(options, IGNORE_WHITESPACE_AT_EOL))
+	if (!diff_b_w_output_only &&
+	    (DIFF_XDL_TST(options, IGNORE_WHITESPACE) ||
+	     DIFF_XDL_TST(options, IGNORE_WHITESPACE_CHANGE) ||
+	     DIFF_XDL_TST(options, IGNORE_WHITESPACE_AT_EOL)))
 		DIFF_OPT_SET(options, DIFF_FROM_CONTENTS);
 	else
 		DIFF_OPT_CLR(options, DIFF_FROM_CONTENTS);

-- 
1.6.6.rc0.61.g41d5b.dirty