git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Christian Brabandt <cblists@256bit.org>
Cc: Christian Brabandt <cb@256bit.org>, git@vger.kernel.org
Subject: Re: Mark trailing whitespace error in del lines of diff
Date: Mon, 25 May 2015 17:24:12 -0700	[thread overview]
Message-ID: <xmqq617g9oer.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <9b8e349e223dc9cd871fc5f7915e590548322932.1432585659.git.cb@256bit.org> (Christian Brabandt's message of "Mon, 25 May 2015 23:11:34 +0200")

Christian Brabandt <cblists@256bit.org>, Christian Brabandt
<cb@256bit.org> writes:

> As far as I can see, this does not break any tests and also the 
> behaviour of git-diff --check does not change. 

Even if this change introduced a bug that changed the behaviour
(e.g. say, exited with failure status code when only preimage had
errors), I wouldn't be surprised if no existing test caught such a
breakage.  Because the existing tests were written with the
assumption that the code to check whitespace breakages would never
look at preimage, it is plausible that no preimage line used in the
test has any whitespace error in the first place.

In other words, you'd need to add new tests that change preimage
lines with various kinds of whitespace errors into postimage lines
with and without whitespace errors, and run "diff" with various
combinations of the existing set of core.whitespace values as well
as your new one.

But as I said in the other message, I think that the approach this
patch takes goes in a wrong direction.  Instead of adding a single
"check and highlight this and only kind of breakage on preimage"
option as a new kind to existing "what kind of use of whitespaces
are errors" set, it would be more sensible to add a single "check
and highlight breakages on preimage lines as well" option that is
orthogonal to the existing ones that specify "what kind of use of
whitespaces are errors".

  parent reply	other threads:[~2015-05-26  0:24 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                   ` [PATCH v3 0/4] showing existing ws breakage Jeff King
2015-05-27 18:57                     ` 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 ` Junio C Hamano [this message]
2015-05-26 16:31   ` Mark trailing whitespace error in del lines of diff 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=xmqq617g9oer.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=cb@256bit.org \
    --cc=cblists@256bit.org \
    --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.
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).