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

Re: [RFC PATCH 0/3] Fixes to "diff --color-moved" MIN_BLOCK_LENGTH handling

From
Stefan Beller <sbeller@google.com>
Date
Aug 14, 2017, 19:51 UTC
Message-ID
<CAGZ79kYHW3bpexhiDnoNfyp=etBJ6nPhyLYR09+4jMpw25hR_A@mail.gmail.com>
In-Reply-To
<xmqqbmniuei9.fsf@gitster.mtv.corp.google.com>
On Mon, Aug 14, 2017 at 12:37 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 38 quoted lines
> Stefan Beller <sbeller@google.com> writes:
>
>> On Fri, Aug 11, 2017 at 5:39 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> ...
>>> My preference however is to keep sb/diff-color-move topic as-is
>>> without replacing and fixing it with incremental updates like these
>>> patches.
>>
>> I would have hoped to not need to reroll that topic.
>> Though I do find patches 1&2 valuable either on top or squashed
>> into "[PATCH] diff.c: color moved lines differently" and
>> "[PATCH] diff.c: color moved lines differently, plain mode"
>> respectively.
>>
>> So I'd ask to pick at least patches 1&2 on top of that series, please?
>
> Yeah, that is exactly what I did before reading this message but
> after reading your comments on the patches ;-)
>
>> (I am missing the context for *why* you preference is to not do
>> exactly this).
>
> I see what I wrote can be misread, especially due to its lack of
> ",instead", that I want to keep the broken one as-is, with neither
> reroll nor fixup.  That is not what I meant.
>
>  - If you choose to squash so that the resulting history after the
>    series graduates to 'master' will be simpler to read (due to lack
>    of "oops, that was a mistake"), I do not mind a reroll.
>
>  - On the other hand, as the topic has been in 'next' for some time
>    and presumably people tried it in their real daily work when
>    needed, keeping what is queued as-is has a value---we have a
>    fixed reference point that we can go back to to compare the code
>    with and without the fix.
>
> I do not have a strong preference, but if I were asked to choose,
> I'd choose the latter.
We'll go with the latter then. Thanks!
Other reasons for the latter that I want to add:
* The patches are written 2 month apart, which may indicate that
  there was real usage and hence fixes with a more substantiated
  understanding of the new feature.
* We should not strive for "perfect" history IMHO. That is because
  commit messages provide a lot of reasoning and add a lot of value
  for understanding the code. If I were to squash and reroll, I would
  need to make sure these points are addressed in the commit
  message to have a result that is equally good.
  The history only needs to be "good-enough", which we defined to
  "bisectable on all platforms that we care about", fixups/bugfixes
  are like the cherry on the cake, it draw attention on its own.
  Not a bad thing IMHO.
Previous: Junio C HamanoNext: Junio C Hamano
Message 11 of 30 in “Fixes to "diff --color-moved" MIN_BLOCK_LENGTH handling”
  1. 0/3 Fixes to "diff --color-moved" MIN_BLOCK_LENGTH handlingJonathan Tan, Aug 11, 2017
  2. 1/3 diff: avoid redundantly clearing a flagJonathan Tan, Aug 11, 2017
  3. Stefan BellerAug 14, 2017
  4. 2/3 diff: respect MIN_BLOCK_LENGTH for last blockJonathan Tan, Aug 11, 2017
  5. Stefan BellerAug 14, 2017
  6. 3/3 diff: check MIN_BLOCK_LENGTH at start of new blockJonathan Tan, Aug 11, 2017
  7. Stefan BellerAug 14, 2017
  8. Junio C HamanoAug 12, 2017
  9. Stefan BellerAug 14, 2017
  10. Junio C HamanoAug 14, 2017
  11. Stefan BellerAug 14, 2017
  12. Junio C HamanoAug 15, 2017
  13. 0/3 Fixes to "diff --color-moved" MIN_BLOCK_LENGTH handlingJonathan Tan, Aug 14, 2017
  14. 1/3 diff: avoid redundantly clearing a flagJonathan Tan, Aug 14, 2017
  15. 2/3 diff: respect MIN_BLOCK_LENGTH for last blockJonathan Tan, Aug 14, 2017
  16. 3/3 diff: check MIN_BLOCK_LENGTH at start of new blockJonathan Tan, Aug 14, 2017
  17. Stefan BellerAug 14, 2017
  18. 0/3 "diff --color-moved" with different heuristicJonathan Tan, Aug 14, 2017
  19. 1/3 diff: avoid redundantly clearing a flagJonathan Tan, Aug 14, 2017
  20. 2/3 diff: respect MIN_BLOCK_LENGTH for last blockJonathan Tan, Aug 14, 2017
  21. 3/3 diff: define block by number of non-space charsJonathan Tan, Aug 14, 2017
  22. Stefan BellerAug 15, 2017
  23. Junio C HamanoAug 15, 2017
  24. Stefan BellerAug 15, 2017
  25. Junio C HamanoAug 15, 2017
  26. 0/3 "diff --color-moved" with yet another heuristicJonathan Tan, Aug 16, 2017
  27. 1/3 diff: avoid redundantly clearing a flagJonathan Tan, Aug 16, 2017
  28. 2/3 diff: respect MIN_BLOCK_LENGTH for last blockJonathan Tan, Aug 16, 2017
  29. 3/3 diff: define block by number of alphanumeric charsJonathan Tan, Aug 16, 2017
  30. Stefan BellerAug 16, 2017

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.