Re: Lines missing from git diff-tree -p -c output?

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

 



Matthijs Kooijman <matthijs@xxxxxxxx> writes:

> Hi Junio,
>
>> I think the coalescing of two adjacent hunks into one is painting
>> leading lines "interesting to show context but not worth showing
>> deletion before it" incorrectly.
> Yup, that seems to be the case.
>
>> Does this patch fix the issue?
>
> Yes, it fixes the issue. However, I think that this patch actually hides
> the real problem (in a way that will always work with the current code,
> though).

Could you explain why you think it hides the real problem, and what
kind of future enhancement may break it?

This is *not* my usual rhetorical question "Please explain yourself,
because I think you are wrong", but is "I do not understand the
reasoning behind your statement, and I (and the reasoning behind my
patch) must be missing something important, so please enlighten me
by pointing out where I am wrong, so that I won't stick to my flawed
patch".

The painting with no_pre_delete is applied when we extend the common
context back to lines we _know_ otherwise not worth showing (because
there is no difference) only because we want to show them as the
context lines and we do not need to show deletions that come before
these common context.  By forcing (k == j + context) case, that is,
there are exactly "context" number of lines between the end of the
current hunk and the next hunk, which the old code would have showed
"context" lines at the beginning of the next hunk, to go back to the
"again" label, we are coalescing the two hunks that _should_ have
been shown together anyway, without painting the context lines
incorrectly with "before this line, do not show deletion" mark.

> I had come up with a different fix myself (similar to the one I sent to
> the list as a followup, but that one still had a bug), which I think
> might be better. In any case, it includes a testcase for this bug which
> seems good to include.
>
> I'll send my patch as a followup in a minute, feel free to use it
> entirely or only partially.
>
> Gr.
>
> Matthijs
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[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]