Re: [PATCH] range-diff: update stale summary of --no-dual-color

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Aug 23, 2018, 03:22 UTC
Message-ID: <20180823032238.GI92374@aiede.svl.corp.google.com>
In-Reply-To: <87wosh7pjj.fsf@kyleam.com>

Kyle Meyer wrote:

> Jonathan Nieder <jrnieder@gmail.com> writes:
>> Kyle Meyer wrote:

>>> -			    N_("color both diff and diff-between-diffs")),
>>> +			    N_("restrict coloring to outer diff markers")),

[...]

Show 12 quoted lines

>> What is an outer diff marker?
>
> The diff markers from the diff of patches as opposed to the ones from
> the original patches.  I took the term from git-range-diff.txt:
>
>     --no-dual-color::
>         When the commit diffs differ, `git range-diff` recreates the
>         original diffs' coloring, and adds outer -/+ diff markers [...]
>
>     Use `--no-dual-color` to revert to color all lines according to the
>     outer diff markers (and completely ignore the inner diff when it
>     comes to color).

Aha: I think you're missing a few words (e.g. "color only according to
outer diff markers").  Though based on the output, I'm not sure the
focus on diff markers captures the difference.  (After all, some lines
are multiple colors in --no-dual-color mode and have no diff markers.)

"Restrict coloring to outer -/+ diff markers" would mean that everything will be in plain text, except for the minus or plus sign at the beginning of each line. So you'd see a colorful strip on the left and everything else monochrome.

I think what you mean is something like "color only based on the diff-between-diffs". Or it might be simpler to do something like the following. What do you think?

diff --git i/builtin/range-diff.c w/builtin/range-diff.c
index f52d45d9d6..88c19f48d3 100644
--- i/builtin/range-diff.c
+++ w/builtin/range-diff.c
@@ -20,12 +20,12 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)
 {
 	int creation_factor = 60;
 	struct diff_options diffopt = { NULL };
-	int simple_color = -1;
+	int dual_color = -1;
 	struct option options[] = {
 		OPT_INTEGER(0, "creation-factor", &creation_factor,
 			    N_("Percentage by which creation is weighted")),
-		OPT_BOOL(0, "no-dual-color", &simple_color,
-			    N_("color both diff and diff-between-diffs")),
+		OPT_BOOL(0, "dual-color", &dual_color,
+			    N_("color both diff and diff-between-diffs (default)")),
 		OPT_END()
 	};
 	int i, j, res = 0;
@@ -63,8 +63,8 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)
 			     options + ARRAY_SIZE(options) - 1, /* OPT_END */
 			     builtin_range_diff_usage, 0);
 
-	if (simple_color < 1) {
-		if (!simple_color)
+	if (dual_color != 0) {
+		if (dual_color > 0)
 			/* force color when --dual-color was used */
 			diffopt.use_color = 1;
 		diffopt.flags.dual_color_diffed_diffs = 1;