git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: git@vger.kernel.org
Cc: Christian Brabandt <cblists@256bit.org>
Subject: [PATCH v2 0/5] showing existing ws breakage
Date: Tue, 26 May 2015 12:46:20 -0700	[thread overview]
Message-ID: <1432669584-342-1-git-send-email-gitster@pobox.com> (raw)
In-Reply-To: <xmqq1ti3kz5v.fsf@gitster.dls.corp.google.com>

We paint whitespace breakages in new (i.e. added or updated) lines
when showing the "git diff" output to help people avoid introducing
them with their changes.  The basic premise is that people would
want to avoid touching existing lines only to fix whitespace errors
in a patch that does other changes of substance, and that is why we
traditionally did not paint whitespace breakages in existing
(i.e. deleted or context) lines.

However, some people would want to keep existing breakages when they
are doing other changes of substance; "new" lines in such a patch
would show existing whitespace breakages painted, and it is not
apparent if the breakages were inherited from the original or newly
introduced.

Christian Brabandt had an interesting idea to help users in this
situation; why not give them a mode to paint whitespace breakages
in "old" (i.e. deleted or was replaced) lines, too?

http://thread.gmane.org/gmane.comp.version-control.git/269912/focus=269956

This series builds on that idea but with a different implementation
(Christian's original painted trailing whitespaces only).

The first three patches are preliminary cleanups.  The last one is
the interesting bit.

Having done this series, I am starting to suspect that painting ws
breakages only in deleted lines may not be such a useful thing to
do.  In order to decide if fixing ws breakages "while at it" is more
appropriate, you would need to know if such breakages are prevalent
in the original.  After all, the line you are updating might be one
of only few lines that the original had breakages, in which case you
may want to go back to your editor and fix them all while you are at
it, or fix only these few ws breakages as a preliminary step.  In
order to help users do that, the new option may be better not to
limit itself to "deleted" lines, but "context and deleted",
i.e. "preimage" lines.

Junio C Hamano (4):
  t4015: modernise style
  t4015: separate common setup and per-test expectation
  diff.c: add emit_del_line() and update callers of emit_line_0()
  diff.c: --ws-check-deleted option

 Documentation/diff-options.txt |   7 +
 diff.c                         |  58 +++--
 diff.h                         |   1 +
 t/t4015-diff-whitespace.sh     | 474 ++++++++++++++++++++---------------------
 4 files changed, 290 insertions(+), 250 deletions(-)

-- 
2.4.1-511-gc1146d5

  parent reply	other threads:[~2015-05-26 19:46 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               ` Junio C Hamano [this message]
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

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=1432669584-342-1-git-send-email-gitster@pobox.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).