Re: --color-moved feedback, was Re: [PATCH v2 19/29] packed-backend: new module for handling packed references

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

 



On Fri, Jun 23, 2017 at 01:23:52PM -0700, Stefan Beller wrote:

> > In the end, I just did --color-moved=plain,  ...
> > "yep, this is all a giant moved chunk, so I don't have to look carefully
> > at it".
> 
> This is dangerous, as "plain" does not care about permutations.
> See the 7f5af90798 (diff.c: color moved lines differently, 2017-06-19)
> for details. You would want at least "zebra", which would show you
> permutations.

Ah, I see. I think what I'd really want is some way of correlating two
particular hunks. That's hard to do with color, though. I guess that's
the "you would need a ton of colors" discussion I saw going on before?

It would depend on how many hunks there are, and how close together they
are. For instance, your 6cd5757c8 seems like a good candidate, but I
have to admit with --color-moved=zebra I have no idea what's going on.
Some things are definitely colored, but I'm not sure what corresponds to
what.

> > That feels more dangerous to me, just because the heuristics seem to
> > find a lot of "moves" of repeated code. Imagine a simple patch like
> > "switch return 0 to return -1". If the code you're touching went away,
> > there's a very good chance that another "return 0" popped up somewhere
> > else in the code base. A conflict is what you want there; silently
> > changing some other site would be not only wrong, but quite subtle and
> > hard to find.
> 
> I agree, that is the danger, but the scale of danger is the size of the moved
> chunk. A file is usually a very large chunk, such that it is obviously a good
> idea to fix that. Maybe we'd introduce a threshold, that the fix must not be in
> range of the block boundaries of say 3 lines.
> (Then the moved block must be at least 7 lines of code such that a one liner
> fix in the middle would kick in)

Yes, I'd agree it's really a continuum from "one line" to "whole file".
I think right now the --color-moved stuff is too close to the former to
be safe, but pushing it farther towards the middle would remedy that.

-Peff



[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