Re: [RFC PATCH 7/7] diff/am: enhance diff format to use */~ for moved lines

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Aug 4, 2018 at 10:15 AM Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> Stefan Beller <sbeller@xxxxxxxxxx> 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@xxxxxxxxxx>
> > ---
>
> 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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux