git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v3 0/4] showing existing ws breakage
Date: Wed, 27 May 2015 03:22:19 -0400	[thread overview]
Message-ID: <20150527072218.GB6898@peff.net> (raw)
In-Reply-To: <1432708232-29892-1-git-send-email-gitster@pobox.com>

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 <peff@peff.net>
---
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

  parent reply	other threads:[~2015-05-27  7:22 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-25 21:11 Mark trailing whitespace error in del lines of diff Christian Brabandt, Christian Brabandt
2015-05-25 22:22 ` brian m. carlson
2015-05-25 23:27   ` Junio C Hamano
2015-05-25 23:52     ` brian m. carlson
2015-05-26 16:29     ` Christian Brabandt
2015-05-26 17:26       ` Junio C Hamano
2015-05-26 17:34         ` Junio C Hamano
2015-05-26 17:39           ` Christian Brabandt
2015-05-26 17:48             ` Junio C Hamano
2015-05-26 18:21               ` Christian Brabandt
2015-05-26 19:46               ` [PATCH v2 0/5] showing existing ws breakage Junio C Hamano
2015-05-26 19:46                 ` [PATCH v2 1/4] t4015: modernise style Junio C Hamano
2015-05-26 19:46                 ` [PATCH v2 2/4] t4015: separate common setup and per-test expectation Junio C Hamano
2015-05-26 19:46                 ` [PATCH v2 3/4] diff.c: add emit_del_line() and update callers of emit_line_0() Junio C Hamano
2015-05-26 19:46                 ` [PATCH v2 4/4] diff.c: --ws-check-deleted option Junio C Hamano
2015-05-27  6:30                 ` [PATCH v3 0/4] showing existing ws breakage Junio C Hamano
2015-05-27  6:30                   ` [PATCH v3 1/4] t4015: modernise style Junio C Hamano
2015-05-27  6:30                   ` [PATCH v3 2/4] t4015: separate common setup and per-test expectation Junio C Hamano
2015-05-27  6:30                   ` [PATCH v3 3/4] diff.c: add emit_del_line() and emit_context_line() Junio C Hamano
2015-05-27  6:30                   ` [PATCH v3 4/4] diff.c: --ws-error-highlight=<kind> option Junio C Hamano
2015-05-27  7:22                   ` Jeff King [this message]
2015-05-27 18:57                     ` [PATCH v3 0/4] showing existing ws breakage Junio C Hamano
2015-05-27 20:36                       ` Jeff King
2015-05-27 20:46                         ` Junio C Hamano
2015-05-27 20:48                           ` Jeff King
2015-05-27 20:53                             ` Junio C Hamano
2015-05-27 20:51                     ` Jeff King
2015-05-26  0:24 ` Mark trailing whitespace error in del lines of diff Junio C Hamano
2015-05-26 16:31   ` Christian Brabandt
2015-05-26 17:33     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150527072218.GB6898@peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).