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

Re: [PATCH v2 4/6] add --simplify-combined-diff option

From
Junio C Hamano <gitster@pobox.com>
Date
Jul 29, 2014, 20:02 UTC
Message-ID
<xmqqd2co2asg.fsf@gitster.dls.corp.google.com>
In-Reply-To
<20140729175712.GD31181@peff.net>
Jeff King <peff@peff.net> writes:
Show 62 quoted lines
> When showing a combined diff, we are comparing two (or more)
> parents to a final state, and some of these states may be
> the same.  Here's a table of the possible contents for a
> given path (for two parents, but we will generalize to more
> in a moment; we also omit isomorphic cases where the parents
> are swapped):
>
>   case | M | P1 | P2
>   -------------------
>   1    | A | A  | A
>   2    | A | A  | B
>   3    | A | B  | B
>   4    | A | B  | C
>
> In case 1, the path was not relevant to the merge at all,
> and we omit it as uninteresting. In case 2, we did resolve
> the path, but in favor of one side. We also consider this
> uninteresting and do not show the diff.
>
> In case 4, we had a real content-level merge, and the
> combined diff is interesting. We show it.
>
> That leaves case 3, in which both parents are the same, but
> the merge picks a new content. This should be rare in
> normal merges, though it could happen if you updated an
> unrelated file due to a resolution elsewhere (i.e., an evil
> merge that crosses a file boundary). But it happens
> frequently in the fake merge commits we create for stashes,
> in which one parent is the base of the stash and the other
> is the index (in which case it simply means that the index
> entry for the path was not touched).
>
> Right now, we treat it the same as case 4, and show a normal
> combined diff. However, the result is harder to read, and
> the combined nature of the diff gives no extra information;
> every marker in the combined diff will be identical for both
> parents.
>
> This patch adds a new option, "--simplify-combined-diff",
> which converts this case into a normal, non-combined diff.
> It would not be correct to simply omit it, because there
> really is an interesting change going from B..A. It's just
> that there are not two interesting changes, which the
> combined diff would show.
>
> When generalizing this to more than two parents, we have two
> options:
>
>   1. Either simply to a single parent content, or not at all.
>
>   2. Omit parents whose contents are duplicates of other
>      parents.
>
> For a case like "A B B C", option (2) would still result in
> a combined diff, but one with fewer sources. However, it
> would also be ambiguous. The parents in a combined diff are
> marked only by their position, so omitting a position means
> that a reader can no longer tell which line goes with which
> parent.
>
> Instead, we choose option (1). Either you get the full
> combined diff, or you get a normal non-combined diff.

Very nicely analyzed. The changes to the code looked also good from a cursory read.

Thanks.
Previous: Jeff KingNext: Jeff King
Message 14 of 20 in “improving "git stash list -p"”
  1. 0/2 improving "git stash list -p"Jeff King, Jul 29, 2014
  2. 1/2 add --simplify-combined-diff optionJeff King, Jul 29, 2014
  3. Jeff KingJul 29, 2014
  4. 2/2 stash: default listing to "--cc --simplify-combined-diff"Jeff King, Jul 29, 2014
  5. 3/2 stash: show combined diff with "stash show"Jeff King, Jul 29, 2014
  6. Junio C HamanoJul 29, 2014
  7. Jeff KingJul 31, 2014
  8. Junio C HamanoJul 31, 2014
  9. 0/6 improving "git stash list -p"Jeff King, Jul 29, 2014
  10. 1/6 revision: drop useless string offset when parsing "--pretty"Jeff King, Jul 29, 2014
  11. 2/6 pretty: treat "--format=" as an empty userformatJeff King, Jul 29, 2014
  12. 3/6 pretty: make empty userformats truly emptyJeff King, Jul 29, 2014
  13. 4/6 add --simplify-combined-diff optionJeff King, Jul 29, 2014
  14. Junio C HamanoJul 29, 2014
  15. 5/6 stash: default listing to "--cc --simplify-combined-diff"Jeff King, Jul 29, 2014
  16. Junio C HamanoJul 29, 2014
  17. Junio C HamanoJul 30, 2014
  18. Jeff KingJul 31, 2014
  19. Keller, Jacob EAug 12, 2014
  20. 6/6 stash: show combined diff with "stash show"Jeff King, Jul 29, 2014

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.