git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCHv3] diff.c: emit moved lines with a different color

From
Jacob Keller <jacob.keller@gmail.com>
Date
Sep 7, 2016, 21:21 UTC
Message-ID
<CA+P7+xp8KeeKY3jkWeSNzUvV1_cXOGpES4Ymj3BRTmM06jhY0Q@mail.gmail.com>
In-Reply-To
<CAGZ79kbSG-9xdmG3LWEsVoJnhEr5pcL8D-95E99a+=JaXGOXCA@mail.gmail.com>
On Wed, Sep 7, 2016 at 11:02 AM, Stefan Beller <sbeller@google.com> wrote:
Show 33 quoted lines
> On Wed, Sep 7, 2016 at 10:54 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Stefan Beller <sbeller@google.com> writes:
>>
>>> as we do not want to see dashes ('moved-old'), I think I'l go with
>>> "movedfrom" and "movedto".
>>
>> OK.  They would be color.diff.movedFrom and color.diff.movedTo in
>> the doc, and "movedfrom" and "movedto" in the code (as the caller
>> already downcased them for you to strcmp()).
>>
>>> When going through the second pass and actually emitting colored lines
>>> we only find matches in the hash map if the current line AND the previous line
>>> match as we lookup by hash code, i.e. if we have a moved line, but the
>>> previous line
>>> changed we do not find it in the hashmap and we don't color it, such
>>> that the reviewer
>>> can spot a permutation.
>>
>> Hmph.  Does this have impact on a line that was at the beginning or
>> the end of a file that got moved to the beginning or the end of a
>> file (four permutations, of 9 if you throw in "middle"), because
>> some cases it does not have a "previous" line?
>
> I spotted that problem as well. We need to adapt the algorithm a bit more:
>
>   If the previous line is of the same kind (i.e. starting with + or -),
>     then we have to take it into account,
>   otherwise (i.e. previous line is different, such as header, hunk header,
>   or different sign)
>     then ignore previous line, as the previous line is already
>     having a different color.
>
> That works for the very first line of a file as well.

That should work to resolve some issues, and ensure the first line of a hunk works correctly.

Thanks, Jake

Previous: Stefan Beller
Message 7 of 7 in “[PATCHv3] diff.c: emit moved lines with a different color”
  1. Stefan BellerSep 4, 2016
  2. Jacob KellerSep 6, 2016
  3. Stefan BellerSep 6, 2016
  4. Jacob KellerSep 6, 2016
  5. Junio C HamanoSep 7, 2016
  6. Stefan BellerSep 7, 2016
  7. Jacob KellerSep 7, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.