Thread (2 messages) flat view 2 messages, 2 authors, 2017-10-24

Re: [PATCH 1/5] connect: split git:// setup into a separate function

From: Stefan Beller <hidden>
Date: 2017-10-24 02:52:28

On Mon, Oct 23, 2017 at 6:54 PM, Junio C Hamano [off-list ref] wrote:
Stefan Beller [off-list ref] writes:
quoted
I think once this option is given, all we have to do is pay attention to
this option in diff.c#moved_entry_cmp/next_byte, which is best built
on top of Peffs recent fixes origin/jk/diff-color-moved-fix.
Would that be of interest for people?
Two things and a half.

 * I was hoping that the next_byte() and string_hash() thing, once
   they are cleaned up, will eventually be shared with the xdiff/
   code at the lower layer, which needs to do pretty much the same
   in order to implement various whitespace ignoring options.  I am
   not sure how well the approach taken by the WIP patch meshes with
   the needs of the lower layer.
Good point. I'll keep that in mind when redoing that patch.
(I might even try to clean up the xdiff stuff and reuse the code
here)
 * I agree that -w that applies only one or the other and not both
   may sometimes produce a better/readable result, but the more
   important part is how the user can tell when to exercise the
   option.  Would it be realistic to expect them to try -w in
   different combinations and see which looks the best?  What if we
   have a patch that touch two files, one looks better with -w only
   for coloring moved and the other looks better with -w for both?

 * As moved-lines display is mostly a presentation thing, I wonder
   if it makes sense to always match loosely wrt whitespace
   differences.  It is tempting because if it is true, we do not
   have to worry about the second issue above.
Well, sometimes the user wants to know if it is byte-for-byte identical
(unlikely to be code, but maybe column oriented data for input;
think of all our FORTRAN users. ;)
and at other times the user just wants to approximately know
if it is the same (i.e. code ignoring white space changes).
If Git was *really* smart w.r.t. languages it might even ignore
identifier renames in the programming language.

So I think an "unconditionally ignore all white space in
move detection" is not the best (it could be a good default though!)
but the user wants to have a possibility to byte-for-byte comparision
(maybe even including CRLFs/LFs)

Thanks,
Stefan
Thanks.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help