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: git@vger.kernel.org
Subject: Re: Mark trailing whitespace error in del lines of diff
Date: Tue, 26 May 2015 10:33:48 -0700	[thread overview]
Message-ID: <xmqqa8wrkzur.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <20150526163108.GB24439@256bit.org> (Christian Brabandt's message of "Tue, 26 May 2015 18:31:08 +0200")

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

>> 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".
>
> Oh well, too bad. It sounded like a good idea...

Oh, don't get me wrong.  I do not think it is not a good idea
(i.e. problem and general strategy to solve it).

Problem:

    Sometimes it is desirable to keep existing whitespace breakages
    while working on fixes and other changes of substance.  The user
    however gets whitespace errors painted only on new lines in the
    output from "git diff", and this makes it hard to tell if they
    were introduced by the user's change or came from the original.

Strategy:

    By painting whitespace breakages in the old lines, the user can
    eyeball and find the corresponding old lines when whitespace
    errors on new lines are painted.  If the corresponding old lines
    have the same errors, the user can see they were inherited from
    the original.

These may be a pair of reasonable problem to solve and a general
strategy to solve it.

What I said was *not* good was the particular execution, the
approach that the patch took.

      reply	other threads:[~2015-05-26 17:34 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 ` 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 [this message]

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=xmqqa8wrkzur.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --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).