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

[Stefan Beller <sbeller@google.com>]

On Mon, Oct 23, 2017 at 6:54 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> 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.