Subject: Re: [PATCH v3 0/4] showing existing ws breakage

From: Jeff King <[email-masked]>
Date: Wed, 27 May 2015 03:22:19 -0400

On Tue, May 26, 2015 at 11:30:28PM -0700, Junio C Hamano wrote:

> The fourth one in v2 used a new option "--[no-]ws-check-deleted",
> but in this round a new option "--ws-error-highlight=<kinds>" is
> defined instead.  With that, you can say
> 
> 	diff --ws-error-highlight=new,old
> 
> to say "I want to see whitespace errors on new and old lines", and
> 
> 	diff --ws-error-highlight=new,old,context
> 	diff --ws-error-highlight=all
> 	
> can be used to also see whitespace errors on context lines.  Being
> able to see whitespace errors on context lines, i.e. the ones that
> were there in the original and you left intact, would help you see
> how prevalent whitespace errors are in the original and hopefully
> would make it easier for you to decide if a separate preliminary
> clean-up to only fix these whitespace errors is warranted.

That makes sense. Neat feature.

In color.diff.*, these are called "new", "old", and "plain". I am of the
opinion that "context" is a far better name than "plain", but perhaps we
should support both for consistency.

Here's a patch for the color.diff side, if we want to go that route.

-- >8 --
Subject: diff: accept color.diff.context as a synonym for "plain"

The term "plain" is a bit ambiguous; let's allow the more
specific "context", but keep "plain" around for
compatibility.

Signed-off-by: Jeff King <[email-masked]>
---
I didn't bother mentioning the historical "plain" in the documentation.
I don't know if it's better to (for people who find it in the wild and
wonder what it means) or if it simply clutters the description. It may
also be that people don't find "plain" as meaningless as I do, and would
rather leave it as the primary in the documentation (and accepting
"context" is just a nicety).

I also resisted the urge to refactor every internal variable and enum
mentioning "plain" into "context". I guess whether that is a good idea
depends on how strongly you agree that "plain" is a bad name. :)

 Documentation/config.txt | 2 +-
 diff.c                   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 5f76e8c..34ef9fe 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -914,7 +914,7 @@ command line with the `--color[=<when>]` option.
 color.diff.<slot>::
 	Use customized color for diff colorization.  `<slot>` specifies
 	which part of the patch to use the specified color, and is one
-	of `plain` (context text), `meta` (metainformation), `frag`
+	of `context` (context text), `meta` (metainformation), `frag`
 	(hunk header), 'func' (function in hunk header), `old` (removed lines),
 	`new` (added lines), `commit` (commit headers), or `whitespace`
 	(highlighting whitespace errors).
diff --git a/diff.c b/diff.c
index 7500c55..27bd371 100644
--- a/diff.c
+++ b/diff.c
@@ -54,7 +54,7 @@ static char diff_colors[][COLOR_MAXLEN] = {
 
 static int parse_diff_color_slot(const char *var)
 {
-	if (!strcasecmp(var, "plain"))
+	if (!strcasecmp(var, "context") || !strcasecmp(var, "plain"))
 		return DIFF_PLAIN;
 	if (!strcasecmp(var, "meta"))
 		return DIFF_METAINFO;
-- 
2.4.1.552.g6de66a4