All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Jeff King <peff@peff.net>, Elijah Newren <newren@gmail.com>,
	Sergey Organov <sorganov@gmail.com>, Johannes Sixt <j6t@kdbg.org>,
	Phillip Wood <phillip.wood123@gmail.com>,
	Elijah Newren <newren@gmail.com>
Subject: [PATCH v2 0/2] RFC: implement new zdiff3 conflict style
Date: Sat, 11 Sep 2021 17:03:47 +0000	[thread overview]
Message-ID: <pull.1036.v2.git.git.1631379829.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1036.git.git.1623734171.gitgitgadget@gmail.com>

Implement a zealous diff3, or "zdiff3". This new mode is identical to
ordinary diff3 except that it allows compaction of common lines between the
two sides of history, if those common lines occur at the beginning or end of
a conflict hunk.

This is still just RFC, because one of the testcases I added fails and
really should be fixed before this series is ready to merge. Also, while
testing I noted one suboptimal merge that I thought I'd be able to easily
reproduce later, but which I wasn't able to reproduce again and for which
I've lost the details. I'd like to keep testing and find it again. Sorry for
the long delay here; been concentrating mostly on merge-ort and sparse-index
and untracked/cwd cleanup things instead.

NOTE: You may want to just eject this topic from seen. It needs more fixes
and while I do plan to eventually get back to it, I'm concentrating more on
other topics.

Changes since v1:

 * Included fixes from Phillip (thanks!)
 * Added some testcases

Elijah Newren (2):
  xdiff: implement a zealous diff3, or "zdiff3"
  update documentation for new zdiff3 conflictStyle

 Documentation/config/merge.txt         |  9 +++-
 Documentation/git-checkout.txt         |  3 +-
 Documentation/git-merge-file.txt       |  3 ++
 Documentation/git-merge.txt            | 32 ++++++++++--
 Documentation/git-rebase.txt           |  6 +--
 Documentation/git-restore.txt          |  3 +-
 Documentation/git-switch.txt           |  3 +-
 Documentation/technical/rerere.txt     | 10 ++--
 builtin/checkout.c                     |  2 +-
 builtin/merge-file.c                   |  2 +
 contrib/completion/git-completion.bash |  6 +--
 t/t6427-diff3-conflict-markers.sh      | 56 +++++++++++++++++++++
 xdiff-interface.c                      |  2 +
 xdiff/xdiff.h                          |  1 +
 xdiff/xmerge.c                         | 68 +++++++++++++++++++++++---
 15 files changed, 176 insertions(+), 30 deletions(-)


base-commit: 670b81a890388c60b7032a4f5b879f2ece8c4558
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1036%2Fnewren%2Fzdiff3-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1036/newren/zdiff3-v2
Pull-Request: https://github.com/git/git/pull/1036

Range-diff vs v1:

 1:  b7561a67c19 ! 1:  06e04c88dea xdiff: implement a zealous diff3, or "zdiff3"
     @@ Commit message
          because zdiff3 shows the base version too and the base version cannot be
          reasonably split.
      
     -    Initial-patch-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
     +    Based-on-patch-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
     +    Helped-by: Phillip Wood <phillip.wood123@gmail.com>
          Signed-off-by: Elijah Newren <newren@gmail.com>
      
       ## builtin/merge-file.c ##
     @@ contrib/completion/git-completion.bash: _git_checkout ()
       		;;
       	--*)
       		__gitcomp_builtin checkout
     +@@ contrib/completion/git-completion.bash: _git_switch ()
     + 
     + 	case "$cur" in
     + 	--conflict=*)
     +-		__gitcomp "diff3 merge" "" "${cur##--conflict=}"
     ++		__gitcomp "diff3 merge zdiff3" "" "${cur##--conflict=}"
     + 		;;
     + 	--*)
     + 		__gitcomp_builtin switch
     +@@ contrib/completion/git-completion.bash: _git_restore ()
     + 
     + 	case "$cur" in
     + 	--conflict=*)
     +-		__gitcomp "diff3 merge" "" "${cur##--conflict=}"
     ++		__gitcomp "diff3 merge zdiff3" "" "${cur##--conflict=}"
     + 		;;
     + 	--source=*)
     + 		__git_complete_refs --cur="${cur##--source=}"
     +
     + ## t/t6427-diff3-conflict-markers.sh ##
     +@@ t/t6427-diff3-conflict-markers.sh: test_expect_success 'rebase --apply describes fake ancestor base' '
     + 	)
     + '
     + 
     ++test_setup_zdiff3 () {
     ++	test_create_repo zdiff3 &&
     ++	(
     ++		cd zdiff3 &&
     ++
     ++		test_write_lines 1 2 3 4 5 6 7 8 9 >basic &&
     ++		test_write_lines 1 2 3 AA 4 5 BB 6 7 8 >middle-common &&
     ++		test_write_lines 1 2 3 4 5 6 7 8 9 >interesting &&
     ++
     ++		git add basic middle-common &&
     ++		git commit -m base &&
     ++
     ++		git branch left &&
     ++		git branch right &&
     ++
     ++		git checkout left &&
     ++		test_write_lines 1 2 3 4 A B C D E 7 8 9 >basic &&
     ++		test_write_lines 1 2 3 CC 4 5 DD 6 7 8 >middle-common &&
     ++		test_write_lines 1 2 3 4 A B C D E F G H I J 7 8 9 >interesting &&
     ++		git add -u &&
     ++		git commit -m letters &&
     ++
     ++		git checkout right &&
     ++		test_write_lines 1 2 3 4 A X C Y E 7 8 9 >basic &&
     ++		test_write_lines 1 2 3 EE 4 5 FF 6 7 8 >middle-common &&
     ++		test_write_lines 1 2 3 4 A B C 5 6 G H I J 7 8 9 >interesting &&
     ++		git add -u &&
     ++		git commit -m permuted
     ++	)
     ++}
     ++
     ++test_expect_failure 'check zdiff3 markers' '
     ++	test_setup_zdiff3 &&
     ++	(
     ++		cd zdiff3 &&
     ++
     ++		git checkout left^0 &&
     ++
     ++		test_must_fail git -c merge.conflictstyle=zdiff3 merge -s recursive right^0 &&
     ++
     ++		test_write_lines 1 2 3 4 A "<<<<<<< HEAD" B C D "||||||| $(git rev-parse --short HEAD^1)" 5 6 ======= X C Y ">>>>>>> right^0" E 7 8 9 >expect &&
     ++		test_cmp expect basic &&
     ++
     ++		test_write_lines 1 2 3 "<<<<<<< HEAD" CC "||||||| $(git rev-parse --short HEAD^1)" AA ======= EE ">>>>>>> right^0" 4 5 "<<<<<<< HEAD" DD "||||||| $(git rev-parse --short HEAD^1)" BB ======= FF ">>>>>>> right^0" 6 7 8 >expect &&
     ++		test_cmp expect middle-common &&
     ++
     ++		# Not passing this one yet.  For some reason, after extracting
     ++		# the common lines "A B C" and "G H I J", the remaining part
     ++		# is comparing "5 6" in the base to "5 6" on the left and
     ++		# "D E F" on the right.  And zdiff3 currently picks the side
     ++		# that matches the base as the merge result.  Weird.
     ++		test_write_lines 1 2 3 4 A B C D E F G H I J 7 8 9 >expect &&
     ++		test_cmp expect interesting
     ++	)
     ++'
     ++
     + test_done
      
       ## xdiff-interface.c ##
      @@ xdiff-interface.c: int git_xmerge_config(const char *var, const char *value, void *cb)
     @@ xdiff/xmerge.c: static int xdl_refine_conflicts(xdfenv_t *xe1, xdfenv_t *xe2, xd
       		m->i1 = xscr->i1 + i1;
       		m->chg1 = xscr->chg1;
       		m->i2 = xscr->i2 + i2;
     +@@ xdiff/xmerge.c: static int lines_contain_alnum(xdfenv_t *xe, int i, int chg)
     + static void xdl_merge_two_conflicts(xdmerge_t *m)
     + {
     + 	xdmerge_t *next_m = m->next;
     ++	m->chg0 = next_m->i0 + next_m->chg0 - m->i0;
     + 	m->chg1 = next_m->i1 + next_m->chg1 - m->i1;
     + 	m->chg2 = next_m->i2 + next_m->chg2 - m->i2;
     + 	m->next = next_m->next;
     +@@ xdiff/xmerge.c: static void xdl_merge_two_conflicts(xdmerge_t *m)
     +  * it appears simpler -- because it takes up less (or as many) lines --
     +  * if the lines are moved into the conflicts.
     +  */
     +-static int xdl_simplify_non_conflicts(xdfenv_t *xe1, xdmerge_t *m,
     ++static int xdl_simplify_non_conflicts(xdfenv_t *xe1, xdmerge_t *m, int style,
     + 				      int simplify_if_no_alnum)
     + {
     + 	int result = 0;
     + 
     +-	if (!m)
     ++	if (!m || style == XDL_MERGE_ZEALOUS_DIFF3)
     + 		return result;
     + 	for (;;) {
     + 		xdmerge_t *next_m = m->next;
      @@ xdiff/xmerge.c: static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1,
       	int style = xmp->style;
       	int favor = xmp->favor;
     @@ xdiff/xmerge.c: static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1,
      +	/*
      +	 * XDL_MERGE_DIFF3 does not attempt to refine conflicts by looking
      +	 * at common areas of sides 1 & 2, because the base (side 0) does
     -+	 * not match and is being shown.
     ++	 * not match and is being shown.  Similarly, simplification of
     ++	 * non-conflicts is also skipped due to the skipping of conflict
     ++	 * refinement.
     ++	 *
     ++	 * XDL_MERGE_ZEALOUS_DIFF3, on the other hand, will attempt to
     ++	 * refine conflicts looking for common areas of sides 1 & 2.
     ++	 * However, since the base is being shown and does not match,
     ++	 * it will only look for common areas at the beginning or end
     ++	 * of the conflict block.  Since XDL_MERGE_ZEALOUS_DIFF3's
     ++	 * conflict refinement is much more limited in this fashion, the
     ++	 * conflict simplification will be skipped.
      +	 *
     -+	 * XDL_MERGE_ZEALOUS_DIFF3 will attempt to refine conflicts
     -+	 * looking for common areas of sides 1 & 2, despite the base
     -+	 * not matching and being shown, but will only look for common
     -+	 * areas at the beginning or ending of the conflict block.
     ++	 * See xdl_refine_conflicts() and xdl_simplify_non_conflicts()
     ++	 * for more details, particularly looking for
     ++	 * XDL_MERGE_ZEALOUS_DIFF3.
      +	 */
       	if (style == XDL_MERGE_DIFF3) {
       		/*
     @@ xdiff/xmerge.c: static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1,
       	/* refine conflicts */
       	if (XDL_MERGE_ZEALOUS <= level &&
      -	    (xdl_refine_conflicts(xe1, xe2, changes, xpp) < 0 ||
     +-	     xdl_simplify_non_conflicts(xe1, changes,
      +	    (xdl_refine_conflicts(xe1, xe2, changes, xpp, style) < 0 ||
     - 	     xdl_simplify_non_conflicts(xe1, changes,
     ++	     xdl_simplify_non_conflicts(xe1, changes, style,
       					XDL_MERGE_ZEALOUS < level) < 0)) {
       		xdl_cleanup_merge(changes);
     + 		return -1;
 2:  50e82a7a32c ! 2:  9ce7246c0e9 update documentation for new zdiff3 conflictStyle
     @@ Documentation/config/merge.txt: merge.conflictStyle::
      +	when a subset of lines match on the two sides they are just pulled
      +	out of the conflict region.  Another alternate style, "zdiff3", is
      +	similar to diff3 but removes matching lines on the two sides from
     -+	the conflict region when those matching lines appear near the
     -+	beginning or ending of a conflict region.
     ++	the conflict region when those matching lines appear near either
     ++	the beginning or end of a conflict region.
       
       merge.defaultToUpstream::
       	If merge is called without any commit argument, merge the upstream
     @@ builtin/checkout.c: static struct option *add_common_options(struct checkout_opt
       		OPT_END()
       	};
       	struct option *newopts = parse_options_concat(prevopts, options);
     -
     - ## contrib/completion/git-completion.bash ##
     -@@ contrib/completion/git-completion.bash: _git_switch ()
     - 
     - 	case "$cur" in
     - 	--conflict=*)
     --		__gitcomp "diff3 merge" "" "${cur##--conflict=}"
     -+		__gitcomp "diff3 merge zdiff3" "" "${cur##--conflict=}"
     - 		;;
     - 	--*)
     - 		__gitcomp_builtin switch
     -@@ contrib/completion/git-completion.bash: _git_restore ()
     - 
     - 	case "$cur" in
     - 	--conflict=*)
     --		__gitcomp "diff3 merge" "" "${cur##--conflict=}"
     -+		__gitcomp "diff3 merge zdiff3" "" "${cur##--conflict=}"
     - 		;;
     - 	--source=*)
     - 		__git_complete_refs --cur="${cur##--source=}"

-- 
gitgitgadget

  parent reply	other threads:[~2021-09-11 17:03 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-15  5:16 [PATCH 0/2] RFC: implement new zdiff3 conflict style Elijah Newren via GitGitGadget
2021-06-15  5:16 ` [PATCH 1/2] xdiff: implement a zealous diff3, or "zdiff3" Elijah Newren via GitGitGadget
2021-06-15  6:13   ` Junio C Hamano
2021-06-15  9:40   ` Felipe Contreras
2021-06-15 18:12     ` Elijah Newren
2021-06-15 18:50       ` Sergey Organov
2021-06-15  5:16 ` [PATCH 2/2] update documentation for new zdiff3 conflictStyle Elijah Newren via GitGitGadget
2021-06-15  6:21   ` Junio C Hamano
2021-06-15  9:43 ` [PATCH 0/2] RFC: implement new zdiff3 conflict style Jeff King
2021-06-15 19:35   ` Elijah Newren
2021-06-16  8:57     ` Phillip Wood
2021-06-16 10:31       ` Jeff King
2021-06-23  9:53         ` Phillip Wood
2021-06-23 22:28           ` Jeff King
2021-06-17  5:03       ` Elijah Newren
2021-06-15 21:36 ` Johannes Sixt
2021-06-15 21:45   ` Elijah Newren
2021-06-16  6:16     ` Johannes Sixt
2021-06-16  8:14       ` Elijah Newren
2021-09-11 17:03 ` Elijah Newren via GitGitGadget [this message]
2021-09-11 17:03   ` [PATCH v2 1/2] xdiff: implement a zealous diff3, or "zdiff3" Elijah Newren via GitGitGadget
2021-09-15 10:25     ` Phillip Wood
2021-09-15 11:21       ` Phillip Wood
2021-09-18 22:06         ` Elijah Newren
2021-09-24 10:09           ` Phillip Wood
2021-09-18 22:04       ` Elijah Newren
2021-09-24 10:16         ` Phillip Wood
2021-09-11 17:03   ` [PATCH v2 2/2] update documentation for new zdiff3 conflictStyle Elijah Newren via GitGitGadget
2021-09-18 23:02   ` [PATCH v3 0/2] RFC: implement new zdiff3 conflict style Elijah Newren via GitGitGadget
2021-09-18 23:02     ` [PATCH v3 1/2] xdiff: implement a zealous diff3, or "zdiff3" Elijah Newren via GitGitGadget
2021-09-18 23:02     ` [PATCH v3 2/2] update documentation for new zdiff3 conflictStyle Elijah Newren via GitGitGadget
2021-11-16  2:13     ` [PATCH v4 0/2] Implement new zdiff3 conflict style Elijah Newren via GitGitGadget
2021-11-16  2:13       ` [PATCH v4 1/2] xdiff: implement a zealous diff3, or "zdiff3" Phillip Wood via GitGitGadget
2021-11-16  2:13       ` [PATCH v4 2/2] update documentation for new zdiff3 conflictStyle Elijah Newren via GitGitGadget
2021-12-01  0:05       ` [PATCH v5 0/2] Implement new zdiff3 conflict style Elijah Newren via GitGitGadget
2021-12-01  0:05         ` [PATCH v5 1/2] xdiff: implement a zealous diff3, or "zdiff3" Phillip Wood via GitGitGadget
2021-12-01  0:05         ` [PATCH v5 2/2] update documentation for new zdiff3 conflictStyle Elijah Newren via GitGitGadget
2021-12-02  8:42           ` Bagas Sanjaya
2021-12-02 13:28             ` Eric Sunshine

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.1036.v2.git.git.1631379829.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=j6t@kdbg.org \
    --cc=newren@gmail.com \
    --cc=peff@peff.net \
    --cc=phillip.wood123@gmail.com \
    --cc=sorganov@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.