Junio C Hamano <gitster@xxxxxxxxx> writes:
> - 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html