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

Re: [PATCH] log,diff-tree: add --combined-with-paths options for merges with renames

From
Elijah Newren <newren@gmail.com>
Date
Jan 26, 2019, 22:12 UTC
Message-ID
<CABPp-BFaVwwM17Oe+-RoP5wt2V58AD=dr01uNPuV=9r4yoF3Qg@mail.gmail.com>
In-Reply-To
<xmqq8sz8fkfi.fsf@gitster-ct.c.googlers.com>
On Fri, Jan 25, 2019 at 2:21 PM Junio C Hamano <gitster@pobox.com> wrote:
Show 22 quoted lines
>
> Elijah Newren <newren@gmail.com> writes:
>
> > Also, my first version of the patch actually showed all names, on all
> > lines, but I found the heavy repetition really annoying, and not in
> > keeping with how non-merge commits are handled (where original
> > filenames are only shown when they differ).  Granted, my change isn't
> > the only one.  We could just have all names shown if they are not all
> > identical, as you suggest and I also considered, but I liked this way
> > slightly better.  If others feel strongly, I can change it, that was
> > just my gut feel and preference.
>
> I do not have a strong _preference_; I am only worried about ease of
> use by consumers of the raw output, i.e. scripts.  From that point
> of view, --all-names showing all names always even when there is no
> rename is involved would certainly be the easiest to write a parser
> for, and lines being overly repetitive is a non-issue.  For human
> consumption, it would be a separate matter, but they won't be
> reading from the --raw output that lists blob object names
> repetitively (the object names may or may not be different, but they
> are all equally gibberish) before showing the human-readble part,
> which are names.
Okay, I've switched it over.
Show 23 quoted lines
> > You could kind of ask the same question of -c -p, actually.  I looked
> > into that, but I was only interested in raw format output and --cc is
> > only about coalescing uninteresting hunks in patches.  Whenever git
> > shows a combined diff in patch format, it always lists two files in
> > the header, e.g.:
> >   a/foo.c
> >   b/foo.c
>
> Yeah, but with a new option, it does not have to stay that way.  It
> is OK to show a/foo.c, b/bar.c, c/baz.c, or perhaps an easy way to
> avoid overlong unreadable lines in these human readable output may
> be to only show b/foo.c to report the result and list the original
> names using the "rename from" "rename to" extended diff-headers
> shown in normal diff output with rename detection enabled.
>
> > ...  I guess the "combined" patch is already kind of
> > special, so it could make sense, ...
>
> Absolutely.  -c/--cc output with -p departs from the usual "diff is
> to compare two things" by having two +/- indicator columns and more
> than two '@'s in hunk header to begin with.  It is only natural if
> we are showing more/all names in --raw, we should avoid losing info
> in the same way.

This turned out to be easier to implement than I expected, with one small wrinkle: We have the "a/" and "b/" prefixes, and switching to "a/", "b/" and "c/" doesn't work due to things like the diff.mnemonicPrefix setting -- plus I didn't want to plumb multiple prefix options everywhere they'd be needed. Instead, I decided that instead of having a "from" and a "to" header, we would have N "from" headers (since all parents are what we came from) and 1 "to" header. I hope that makes sense, because otherwise I think this become to unwieldy of a change.

Previous: Junio C HamanoNext: Junio C Hamano
Message 38 of 39 in “log,diff-tree: add --combined-with-paths options for merges with renames”
  1. log,diff-tree: add --combined-with-paths options for merges with renamesElijah Newren, Jan 24, 2019
  2. brian m. carlsonJan 25, 2019
  3. Elijah NewrenJan 25, 2019
  4. Derrick StoleeJan 25, 2019
  5. Elijah NewrenJan 25, 2019
  6. log,diff-tree: add --combined-with-paths options for merges with renamesElijah Newren, Jan 25, 2019
  7. Derrick StoleeJan 25, 2019
  8. Elijah NewrenJan 25, 2019
  9. Eric SunshineJan 25, 2019
  10. Elijah NewrenJan 26, 2019
  11. brian m. carlsonJan 27, 2019
  12. log,diff-tree: add --combined-all-names optionElijah Newren, Jan 26, 2019
  13. log,diff-tree: add --combined-all-names optionElijah Newren, Feb 4, 2019
  14. Junio C HamanoFeb 4, 2019
  15. Elijah NewrenFeb 5, 2019
  16. Junio C HamanoFeb 5, 2019
  17. Elijah NewrenFeb 7, 2019
  18. Junio C HamanoFeb 7, 2019
  19. Elijah NewrenFeb 7, 2019
  20. Junio C HamanoFeb 7, 2019
  21. Elijah NewrenFeb 7, 2019
  22. Johannes SchindelinFeb 5, 2019
  23. Elijah NewrenFeb 5, 2019
  24. Junio C HamanoFeb 5, 2019
  25. Junio C HamanoFeb 7, 2019
  26. Elijah NewrenFeb 7, 2019
  27. 0/2 add --combined-all-paths option to log and diff-treeElijah Newren, Feb 8, 2019
  28. 2/2 squash! log,diff-tree: add --combined-all-paths optionElijah Newren, Feb 8, 2019
  29. Junio C HamanoFeb 8, 2019
  30. Elijah NewrenFeb 8, 2019
  31. 1/2 log,diff-tree: add --combined-all-paths optionElijah Newren, Feb 8, 2019
  32. Junio C HamanoFeb 8, 2019
  33. Elijah NewrenFeb 8, 2019
  34. Junio C HamanoFeb 8, 2019
  35. Junio C HamanoJan 25, 2019
  36. Elijah NewrenJan 25, 2019
  37. Junio C HamanoJan 25, 2019
  38. Elijah NewrenJan 26, 2019
  39. Junio C HamanoJan 28, 2019

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.