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

Re: [PATCH v8] diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible

From
Junio C Hamano <gitster@pobox.com>
Date
Oct 22, 2013, 18:09 UTC
Message-ID
<xmqqob6htbx9.fsf@gitster.dls.corp.google.com>
In-Reply-To
<BB9AEFCE-0E64-4EAA-8DEA-9A8125B8C553@gmail.com>
Yoshioka Tsuneo <yoshiokatsuneo@gmail.com> writes:
> Also, I guess Junio might be suspicious to the idea to keep arrow("=>") itself, maybe ?

I think there is no single "right" solution to this issue, and it has to boils down to the taste.

When you are viewing "diff --stat -M" output in wide-enough medium, you are seeing three pieces of information: what the source path was, what the destination path will be, and what amount of change is made with the change. When the output width is too narrow to show these paths, with the current code, you see truncated destination path, possibly without the source path, but this patch will show the source and the destination paths, both of which are truncated even more severely, because it always has to spend display columns for an extra "..." (to show truncation of the source side), " => " (to show that it is a rename), and <"{","}"> pair (again to show that it is a rename). If the destination does not fit, the output before this patch would have thrown these away as part of left-truncation, to show the destination path as maximally as possible. We do not have even half the width of the current "truncated to be destination only" output for each path.

I am afraid that in the cases where the patch makes a difference, what happens would be that you can no longer tell what source or destination paths really are, because the leading directory part gets truncated too much, and if we didn't have this patch, at least you can tell what destination path is affected. We would trade the guessability of at least one path (the destination) with just a single bit of information (an unidentifiable path got renamed to another unidentifiable path).

I am not yet convinced that it is a good trade-off. Especially given the diffstat output is not about files but more about contents, between an output in the extreme case the version after the patch needs to produce

	{... => ...}/controller/Makefile | 7 +++++++

that tells us "7 lines were updated in the procedure to build some unknown controller by copying or renaming from the build procedure of some other unknown controller", and the output the current code would give to the same rename

	.}/fooGadget/controller/Makefile | 7 +++++++
        
that tells us "7 lines were updated in the build procedure for the
foo Gadget", I think the latter contains more useful information,
even though it does lose one bit of information ("there was a rename
involved in producing this final path") compared to the version with
the patch.
So you are correct to say that I am still skeptical.

In any case, the output from "diff --stat -M" should match the output from "apply --stat -M", I think.

Previous: Yoshioka TsuneoNext: Yoshioka Tsuneo
Message 28 of 31 in “diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible.”
  1. diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible.Yoshioka Tsuneo, Oct 11, 2013
  2. diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible.Yoshioka Tsuneo, Oct 11, 2013
  3. Sam VilainOct 11, 2013
  4. Keshav KiniOct 12, 2013
  5. Yoshioka TsuneoOct 12, 2013
  6. diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible.Yoshioka Tsuneo, Oct 12, 2013
  7. Thomas RastOct 13, 2013
  8. Yoshioka TsuneoOct 15, 2013
  9. Duy NguyenOct 14, 2013
  10. Yoshioka TsuneoOct 15, 2013
  11. diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible.Yoshioka Tsuneo, Oct 15, 2013
  12. Felipe ContrerasOct 15, 2013
  13. Yoshioka TsuneoOct 15, 2013
  14. diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible.Yoshioka Tsuneo, Oct 15, 2013
  15. Junio C HamanoOct 15, 2013
  16. Keshav KiniOct 15, 2013
  17. Yoshioka TsuneoOct 16, 2013
  18. diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible.Yoshioka Tsuneo, Oct 16, 2013
  19. Junio C HamanoOct 17, 2013
  20. Yoshioka TsuneoOct 17, 2013
  21. Junio C HamanoOct 17, 2013
  22. Yoshioka TsuneoOct 18, 2013
  23. Junio C HamanoOct 17, 2013
  24. diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visibleYoshioka Tsuneo, Oct 17, 2013
  25. diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visibleYoshioka Tsuneo, Oct 18, 2013
  26. Thomas RastOct 19, 2013
  27. Yoshioka TsuneoOct 20, 2013
  28. Junio C HamanoOct 22, 2013
  29. Yoshioka TsuneoOct 22, 2013
  30. Junio C HamanoOct 22, 2013
  31. diff.c: keep arrow(=>) on show_stats()'s shortened filename part to make rename visible.Yoshioka Tsuneo, Oct 12, 2013

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.