All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Andreas Heiduk <asheiduk@gmail.com>
Cc: Git Mailing List <git@vger.kernel.org>
Subject: Re: [PATCH] doc: add missing "none" value for diff.wsErrorHighlight
Date: Tue, 25 Jul 2017 12:05:28 -0700	[thread overview]
Message-ID: <xmqqa83sbamf.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20170724223021.9124-1-asheiduk@gmail.com> (Andreas Heiduk's message of "Tue, 25 Jul 2017 00:30:21 +0200")

Andreas Heiduk <asheiduk@gmail.com> writes:

> The value has not eluded documentation so far.

I am not sure what "has not eluded" means in this context (did you
mean "has eluded"?).  

The patch text itself is not wrong per-se, but if we are to add
documentation for 'none', diff-options.txt must also document that
it clears the default and previously given values, unlike new, old
and context that are cumulative.  For that matter, we do not list
'default' and 'all' (which also clears the previous ones before
setting their own) in that three-item list, either.

I think we need to either 

 - make it to a six-item list and then describe that 'none', 'all'
   and 'default' clear the slate before taking any effect, or 

 - keep it three-item list of cumulative things, and then in the
   sentence that talks about `all` in Documentation/diff-options.txt
   to also explain what 'default' and 'none' do.

Thanks.

> Signed-off-by: Andreas Heiduk <asheiduk@gmail.com>
> ---
>  Documentation/diff-config.txt  | 2 +-
>  Documentation/diff-options.txt | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt
> index cbce8ec63..c84ced8f6 100644
> --- a/Documentation/diff-config.txt
> +++ b/Documentation/diff-config.txt
> @@ -200,7 +200,7 @@ diff.algorithm::
>  +
>  
>  diff.wsErrorHighlight::
> -	A comma separated list of `old`, `new`, `context`, that
> +	A comma separated list of `old`, `new`, `context` and `none`, that
>  	specifies how whitespace errors on lines are highlighted
>  	with `color.diff.whitespace`.  Can be overridden by the
>  	command line option `--ws-error-highlight=<kind>`
> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
> index 89cc0f48d..903d68eb7 100644
> --- a/Documentation/diff-options.txt
> +++ b/Documentation/diff-options.txt
> @@ -302,7 +302,7 @@ ifndef::git-format-patch[]
>  --ws-error-highlight=<kind>::
>  	Highlight whitespace errors on lines specified by <kind>
>  	in the color specified by `color.diff.whitespace`.  <kind>
> -	is a comma separated list of `old`, `new`, `context`.  When
> +	is a comma separated list of `old`, `new`, `context` and `none`.  When
>  	this option is not given, only whitespace errors in `new`
>  	lines are highlighted.  E.g. `--ws-error-highlight=new,old`
>  	highlights whitespace errors on both deleted and added lines.

  reply	other threads:[~2017-07-25 19:05 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-24 22:30 [PATCH] doc: add missing "none" value for diff.wsErrorHighlight Andreas Heiduk
2017-07-25 19:05 ` Junio C Hamano [this message]
2017-07-25 20:53   ` [PATCH v2] doc: add missing values "none" and "default" " Andreas Heiduk
2017-07-25 21:31     ` 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

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

  git send-email \
    --in-reply-to=xmqqa83sbamf.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=asheiduk@gmail.com \
    --cc=git@vger.kernel.org \
    /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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.