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

Re: [PATCH 8/8] diff: improve positioning of add/delete blocks in diffs

From
Jacob Keller <jacob.keller@gmail.com>
Date
Aug 4, 2016, 04:47 UTC
Message-ID
<CA+P7+xp8n5s0EkUCXVyqohyuXW0iPd80niq2_CPHTLr00=Ahzw@mail.gmail.com>
In-Reply-To
<f7f35993-39a8-b788-d2a7-d030ac442b9b@alum.mit.edu>
On Wed, Aug 3, 2016 at 3:36 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
Show 26 quoted lines
> On 08/04/2016 12:29 AM, Jacob Keller wrote:
>> On Wed, Aug 3, 2016 at 3:00 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
>> It seems odd to be that a line with "199" spaces and nothing else will
>> return "-1" but a line with 200 spaces and nothing else will return
>> 200..? Would it be safe to just return -1 in both cases (if a line is
>> all spaces or starts with more than 200 spaces just return -1)?
>>
>>> +}
>>> +
>
> Thanks for your feedback.
>
> I was implicitly assuming that such lines would have text somewhere
> after those 200 spaces (or 25 TABs or whatever). But you're right, the
> line could consist only of whitespace. Unfortunately, the only way to
> distinguish these two cases is to read the rest of the line, which is
> exactly what we *don't* want to do.
>
> But I think it doesn't matter anyway. Such "text" will likely never be
> read by a human, so it's not a big deal if the slider position is not
> picked perfectly. And remember, this whole saga is just to improve the
> aesthetics of the diff. The diff is *correct* (e.g., in the sense of
> applicable) regardless of where we position the sliders.
>
> Michael
>

I think in this case treating it as "all whitespace" is more natural than treating it as "200 characters with something following it" because the only thing we've found so far is all white space.

Either way it's not really a big deal here.

Thanks, Jake

Previous: Michael HaggertyNext: Junio C Hamano
Message 33 of 57 in “Better heuristics make prettier diffs”
  1. 0/8 Better heuristics make prettier diffsMichael Haggerty, Aug 3, 2016
  2. 2/8 xdl_change_compact(): clarify codeMichael Haggerty, Aug 3, 2016
  3. Stefan BellerAug 3, 2016
  4. Michael HaggertyAug 3, 2016
  5. Stefan BellerAug 3, 2016
  6. Jeff KingAug 4, 2016
  7. Michael HaggertyAug 10, 2016
  8. Stefan BellerAug 10, 2016
  9. 5/8 xdl_change_compact(): fix compaction heuristic to adjust ioMichael Haggerty, Aug 3, 2016
  10. Jeff KingAug 4, 2016
  11. Michael HaggertyAug 10, 2016
  12. Michael HaggertyAug 10, 2016
  13. Jeff KingAug 11, 2016
  14. Junio C HamanoAug 4, 2016
  15. Michael HaggertyAug 10, 2016
  16. 7/8 is_blank_line: take a single xrecord_t as argumentMichael Haggerty, Aug 3, 2016
  17. Junio C HamanoAug 4, 2016
  18. 6/8 xdl_change_compact(): keep track of the earliest endMichael Haggerty, Aug 3, 2016
  19. Junio C HamanoAug 4, 2016
  20. Michael HaggertyAug 10, 2016
  21. 4/8 xdl_change_compact(): do one final shift or the other, not bothMichael Haggerty, Aug 3, 2016
  22. 1/8 xdl_change_compact(): rename some local variables for clarityMichael Haggerty, Aug 3, 2016
  23. Jeff KingAug 4, 2016
  24. Junio C HamanoAug 4, 2016
  25. Michael HaggertyAug 13, 2016
  26. Jeff KingAug 14, 2016
  27. Michael HaggertyAug 3, 2016
  28. 3/8 xdl_change_compact(): rename i to endMichael Haggerty, Aug 3, 2016
  29. Jeff KingAug 4, 2016
  30. 8/8 diff: improve positioning of add/delete blocks in diffsMichael Haggerty, Aug 3, 2016
  31. Jacob KellerAug 3, 2016
  32. Michael HaggertyAug 3, 2016
  33. Jacob KellerAug 4, 2016
  34. Junio C HamanoAug 4, 2016
  35. Michael HaggertyAug 10, 2016
  36. Junio C HamanoAug 10, 2016
  37. Stefan BellerAug 3, 2016
  38. Michael HaggertyAug 3, 2016
  39. Stefan BellerAug 3, 2016
  40. Michael HaggertyAug 3, 2016
  41. Stefan BellerAug 4, 2016
  42. Michael HaggertyAug 10, 2016
  43. Jeff KingAug 4, 2016
  44. Stefan BellerAug 4, 2016
  45. Junio C HamanoAug 4, 2016
  46. Michael HaggertyAug 13, 2016
  47. Michael HaggertyAug 12, 2016
  48. Jeff KingAug 13, 2016
  49. Junio C HamanoAug 13, 2016
  50. Jacob KellerAug 14, 2016
  51. Stefan BellerAug 15, 2016
  52. Junio C HamanoAug 15, 2016
  53. Junio C HamanoAug 4, 2016
  54. Michael HaggertyAug 13, 2016
  55. Jeff KingAug 4, 2016
  56. Junio C HamanoAug 4, 2016
  57. Jeff KingAug 4, 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.