From: Stefan Beller <sbeller@google.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git <git@vger.kernel.org>
Subject: Re: [RFC PATCH 7/7] diff/am: enhance diff format to use */~ for moved lines
Date: Sun, 5 Aug 2018 23:07:25 -0700 [thread overview]
Message-ID: <CAGZ79kYA4BNe1N+_S=vqwsn2wNH6qQz=kcgwc9Ufr8nfQT85rA@mail.gmail.com> (raw)
In-Reply-To: <xmqqtvoarr3d.fsf@gitster-ct.c.googlers.com>
On Sat, Aug 4, 2018 at 10:15 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Stefan Beller <sbeller@google.com> writes:
>
> > Try it out via
> > ./git-format-patch --mark-moved 15ef69314d^..15ef69314d
> > to see if you like it.
> >
> > This separates the coloring decision from the detection of moved lines.
> > When giving --mark-moved, move detection is still performed and the output
> > markers are adjusted to */~ for new and old code.
> >
> > git-apply and git-am will also accept these patches by rewriting those
> > signs back to +/-.
> >
> > Signed-off-by: Stefan Beller <sbeller@google.com>
> > ---
>
> This does not have anything to do with the range-diff topic, but
> would stand on its own merit.
Yes. I should have emphasized this more in the cover letter.
This is more a "while at it" thing, that is easy to do due to the
refactoring in previous patches.
> I have a mixed feeling about this.
Me, too.
> If you need to convince "GNU patch" maintainers to accept these two
> signs, then probably it is not worth the battle of incompatiblity.
> If it is truly a worthy innovation, they would follow suit, which is
> how they learned to take our renaming diffs without us prodding
> them. I just do not get the gut feeling that it would happen for
> this particular thing, and I am not convinced myself enough to sell
> this to "patch" maintainers and expect to be taken seriously.
ok.
> When reviewing anything complex that would be helped by moved code
> highlighting, I do not think a normal person would choose to review
> such a change only inside MUA. I certainly won't. I'd rather apply
> the patch and view it within a larger context than the piece of
> e-mail that was originally sent offers, with better tools like -W
> and --color-moved applied locally. So in that sense, I do not think
> I'd appreciate lines that begin with '~'/'*' as different kind of
> '-'/'+', as helpful hints; at least until my eyes get used to them,
> they would only appear as distraction.
My use case would be patches that are *not* complex, but still shuffling
lots of code around, e.g. reordering functions/paragraphs in a file.
> In other words, I have this nagging suspicion that people who
> suggested to you that this would help in e-mail workflow are
> misguided and they do not understand e-mail workflow in the first
> place, but perhaps it is just me.
There are no other people that suggested this.
It was really just a quick shot "while at it" as we had the
refactoring in place that enables this, and I think for trivial
patches (non-complex, but lots of changes) it *may* be beneficial.
But it is more for corner cases, I guess.
Stefan
next prev parent reply other threads:[~2018-08-06 6:07 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-08-04 1:53 [PATCH 0/7] improve range-diffs coloring and [RFC] move detection Stefan Beller
2018-08-04 1:53 ` [PATCH 1/7] diff.c: emit_line_0 to take string instead of first sign Stefan Beller
2018-08-04 1:53 ` [PATCH 2/7] diff.c: add --output-indicator-{new, old, context} Stefan Beller
2018-08-04 1:53 ` [PATCH 3/7] range-diff: make use of different output indicators Stefan Beller
2018-08-04 1:53 ` [PATCH 4/7] range-diff: indent special lines as context Stefan Beller
2018-08-04 1:53 ` [PATCH 5/7] diff.c: rename color_moved to markup_moved Stefan Beller
2018-08-04 1:53 ` [PATCH 6/7] diff.c: factor determine_line_color out of emit_diff_symbol_from_struct Stefan Beller
2018-08-04 1:53 ` [RFC PATCH 7/7] diff/am: enhance diff format to use */~ for moved lines Stefan Beller
2018-08-04 17:15 ` Junio C Hamano
2018-08-06 6:07 ` Stefan Beller [this message]
2018-08-04 16:57 ` [PATCH 0/7] improve range-diffs coloring and [RFC] move detection Junio C Hamano
2018-08-06 6:01 ` Stefan Beller
2018-08-06 20:18 ` 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='CAGZ79kYA4BNe1N+_S=vqwsn2wNH6qQz=kcgwc9Ufr8nfQT85rA@mail.gmail.com' \
--to=sbeller@google.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.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).