[v3,10/11] add -p: prefer color.diff.context over color.diff.plain

Commit Message

Johannes Schindelin Nov. 16, 2020, 4:08 p.m. UTC

From: Johannes Schindelin <[e-mail]>

Git's diff machinery allows users to override the colors to use in
diffs, even the plain-colored context lines. As of 8dbf3eb6850 (diff.h:
rename DIFF_PLAIN color slot to DIFF_CONTEXT, 2015-05-27), the preferred
name of the config setting is `color.diff.context`, although Git still
allows `color.diff.plain`.

In the context of `git add -p`, this logic is a bit hard to replicate:
`git_diff_basic_config()` reads all config values sequentially and if it
sees _any_ `color.diff.context` or `color.diff.plain`, it accepts the
new color. The Perl version of `git add -p` needs to go through `git
config --get-color`, though, which allows only one key to be specified.
The same goes for the built-in version of `git add -p`, which has to go
through `repo_config_get_value()`.

The best we can do here is to look for `.context` and if none is found,
fall back to looking for `.plain`, and if still not found, fall back to
the hard-coded default (which in this case is simply the empty string,
as context lines are typically rendered without colored).

This still leads to inconsistencies when both config names are used: the
initial diff will be colored by the diff machinery. Once edited by a
user, a hunk has to be re-colored by `git add -p`, though, which would
then use the other setting to color the context lines.

In practice, this is not _all_ that bad. The `git config` manual says
this in the `color.diff.<slot>`:

	`context` (context text - `plain` is a historical synonym)

We should therefore assume that users use either one or the other, but
not both names. Besides, it is relatively uncommon to look at a hunk
after editing it because it is immediately staged by default.

Signed-off-by: Johannes Schindelin <[e-mail]>
---
 add-interactive.c         | 6 ++++--
 git-add--interactive.perl | 6 +++---
 2 files changed, 7 insertions(+), 5 deletions(-)

Patch

diff --git a/add-interactive.c b/add-interactive.c
index c298a8b80f..54dfdc56f5 100644
--- a/add-interactive.c
+++ b/add-interactive.c
@@ -49,8 +49,10 @@  void init_add_i_state(struct add_i_state *s, struct repository *r)
 
 	init_color(r, s, "diff.frag", s->fraginfo_color,
 		   diff_get_color(s->use_color, DIFF_FRAGINFO));
-	init_color(r, s, "diff.context", s->context_color,
-		diff_get_color(s->use_color, DIFF_CONTEXT));
+	init_color(r, s, "diff.context", s->context_color, "fall back");
+	if (!strcmp(s->context_color, "fall back"))
+		init_color(r, s, "diff.plain", s->context_color,
+			   diff_get_color(s->use_color, DIFF_CONTEXT));
 	init_color(r, s, "diff.old", s->file_old_color,
 		diff_get_color(s->use_color, DIFF_FILE_OLD));
 	init_color(r, s, "diff.new", s->file_new_color,
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index adbac2bc6d..bc3a1e8eff 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -30,9 +30,9 @@ 
 	$diff_use_color ? (
 		$repo->get_color('color.diff.frag', 'cyan'),
 	) : ();
-my ($diff_plain_color) =
+my ($diff_context_color) =
 	$diff_use_color ? (
-		$repo->get_color('color.diff.plain', ''),
+		$repo->get_color($repo->config('color.diff.context') ? 'color.diff.context' : 'color.diff.plain', ''),
 	) : ();
 my ($diff_old_color) =
 	$diff_use_color ? (
@@ -1046,7 +1046,7 @@  sub color_diff {
 		colored((/^@/  ? $fraginfo_color :
 			 /^\+/ ? $diff_new_color :
 			 /^-/  ? $diff_old_color :
-			 $diff_plain_color),
+			 $diff_context_color),
 			$_);
 	} @_;
 }