From: Junio C Hamano <gitster@pobox.com>
To: Stefan Beller <sbeller@google.com>
Cc: git@vger.kernel.org
Subject: Re: [RFC/PATCH 0/2] Color moved code differently
Date: Sat, 03 Sep 2016 00:00:21 -0700 [thread overview]
Message-ID: <xmqqtwdxqxh6.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20160903033120.20511-1-sbeller@google.com> (Stefan Beller's message of "Fri, 2 Sep 2016 20:31:18 -0700")
Stefan Beller <sbeller@google.com> writes:
> A line is colored differently if that line and the surroundign 2 lines
> appear as-is in the opposite part of the diff.
>
> Example:
> http://i.imgur.com/ay84q0q.png
>
> Or apply these patches and
> git show e28eae3184b26d3cf3293e69403babb5c575342c
> git show bc9204d4ef6e0672389fdfb0d398fa9a39dba3d5
> git show 8465541e8ce8eaf16e66ab847086779768c18f2d
I like this as a concept. Two quick comments are
* On 1/2, you would also need to teach diff-color-slot the
correspondence between the name used by configuration and the
enum used as the index into the diff_colors[] array. I think
these are not "DUPLICATE", but "MOVE", so I'd suggest renaming
dup-new and dup-old to some words or phrases that have "MOVED"
and "TO" or "FROM" in them (e.g. "DIFF_MOVED_FROM",
"DIFF_MOVED_TO").
* On 2/2, doing it at xdiff.c level may be limiting this good idea
to flourish to its full potential, as the interface is fed only
one diff_filepair at a time. All the examples you pointed at
above have line movement within a single path because of this
design limitation. I do not think 2/2 would serve as a small but
good first step to build on top of to enhance the feature to
notice line movements across files and the design (not the
implementation) needs rethinking.
The idea has a potential to help reviewing inter-file movement of
lines in 3b0c4200 ("apply: move libified code from builtin/apply.c
to apply.{c,h}", 2016-08-08). You can see what was _changed_ in the
part that has been moved across files with "show -B -M", and
sometimes that is useful, but at the same time, you cannot see what
was moved without changing, which often is necessary to understand
the changes and notice things like "you moved this across files
without changing, but this and that you did not change need to be
adjusted".
The coloring of "these are moved verbatim" in the style this series
gives would be very helpful for reviewing such a change.
next prev parent reply other threads:[~2016-09-03 7:00 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-03 3:31 [RFC/PATCH 0/2] Color moved code differently Stefan Beller
2016-09-03 3:31 ` [PATCH 1/2] diff.c: emit duplicate lines with a different color Stefan Beller
2016-09-03 3:31 ` [RFC/PATCH 2/2] WIP xdiff: markup duplicates differently Stefan Beller
2016-09-03 12:25 ` Jakub Narębski
2016-09-04 5:31 ` Stefan Beller
2016-09-04 10:35 ` Jakub Narębski
2016-09-04 6:48 ` Junio C Hamano
2016-09-03 7:00 ` Junio C Hamano [this message]
2016-09-04 5:23 ` [RFC/PATCH 0/2] Color moved code differently Stefan Beller
2016-09-04 6:41 ` Junio C Hamano
2016-09-04 8:28 ` Stefan Beller
2016-09-04 22:19 ` Junio C Hamano
2016-09-04 9:57 ` Jacob Keller
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=xmqqtwdxqxh6.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=sbeller@google.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).