diff mbox series

vimdiff: fix 'vimdiff3' behavior (colors + no extra key press)

Message ID 20220806213005.92045-1-greenfoo@u92.eu (mailing list archive)
State New, archived
Headers show
Series vimdiff: fix 'vimdiff3' behavior (colors + no extra key press) | expand

Commit Message

Fernando Ramos Aug. 6, 2022, 9:30 p.m. UTC 
vimdiff3 was introduced in 7c147b77d3 (mergetools: add vimdiff3 mode,
2014-04-20) and then partially broken in 0041797449 (vimdiff: new
implementation with layout support, 2022-03-30) in two ways:

  - It does not show colors unless the user has "set hidden" in his
    .vimrc file

  - It prompts the user to "Press ENTER" every time it runs.

This patch fixes both issues by adding "set hidden" and "silent" to the
generated command string that is used to run vim.

Cc: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Fernando Ramos <greenfoo@u92.eu>
---
 mergetools/vimdiff | 65 +++++++++++++++++++++++-----------------------
 1 file changed, 33 insertions(+), 32 deletions(-)

Comments

Felipe Contreras Aug. 7, 2022, 12:55 a.m. UTC | #1 
On Sat, Aug 6, 2022 at 4:30 PM Fernando Ramos <greenfoo@u92.eu> wrote:

>
> +               # Add an extra "-c" option to move to the first tab (notice that we
> +               # can't simply append the command to the previous "-c" string as
> +               # explained here: https://github.com/vim/vim/issues/9076
>
> -       # Add an extra "-c" option to move to the first tab (notice that we
> -       # can't simply append the command to the previous "-c" string as
> -       # explained here: https://github.com/vim/vim/issues/9076
> +               FINAL_CMD="-c \"$CMD\" -c \"tabfirst\""
> +       else
> +               CMD="$CMD | set hidden | let tmp=bufnr('%') | silent bufdo diffthis | exe 'buffer '.tmp"

NAK: This runs `exe 'buffer #'` in every buffer. While switching to
the desired buffer 3 extra times isn't a problem, it's also not
correct.

You need something like:

    silent exe 'bufdo diffthis' | exe 'buffer '.tmp"

But at this point it seems like we are working around the current
organization of the code, when we could simply reorganize it.

Cheers.
diff mbox series

Patch

diff --git a/mergetools/vimdiff b/mergetools/vimdiff
index f770b8fe24..238963071a 100644
--- a/mergetools/vimdiff
+++ b/mergetools/vimdiff
@@ -29,8 +29,8 @@ 
 ################################################################################
 
 debug_print () {
-	# Send message to stderr if global variable GIT_MERGETOOL_VIMDIFF is set
-	# to "true"
+        # Send message to stderr if global variable GIT_MERGETOOL_VIMDIFF_DEBUG
+        # is set to "true"
 
 	if test -n "$GIT_MERGETOOL_VIMDIFF_DEBUG"
 	then
@@ -261,19 +261,19 @@  gen_cmd_aux () {
 
 	if test "$target" = "LOCAL"
 	then
-		CMD="$CMD | 1b"
+		CMD="$CMD | silent 1b"
 
 	elif test "$target" = "BASE"
 	then
-		CMD="$CMD | 2b"
+		CMD="$CMD | silent 2b"
 
 	elif test "$target" = "REMOTE"
 	then
-		CMD="$CMD | 3b"
+		CMD="$CMD | silent 3b"
 
 	elif test "$target" = "MERGED"
 	then
-		CMD="$CMD | 4b"
+		CMD="$CMD | silent 4b"
 
 	else
 		CMD="$CMD | ERROR: >$target<"
@@ -310,7 +310,7 @@  gen_cmd () {
 	#
 	#     gen_cmd "@LOCAL , REMOTE"
 	#     |
-	#     `-> FINAL_CMD    == "-c \"echo | leftabove vertical split | 1b | wincmd l | 3b | tabdo windo diffthis\" -c \"tabfirst\""
+	#     `-> FINAL_CMD    == "-c \"echo | leftabove vertical split | silent 1b | wincmd l | silent 3b | set hidden | silent tabdo windo diffthis\" -c \"tabfirst\""
 	#         FINAL_TARGET == "LOCAL"
 
 	LAYOUT=$1
@@ -341,17 +341,18 @@  gen_cmd () {
 
 	if echo "$LAYOUT" | grep ",\|/" >/dev/null
 	then
-		CMD="$CMD | tabdo windo diffthis"
-	else
-		CMD="$CMD | bufdo diffthis"
-	fi
+		CMD="$CMD | set hidden | silent tabdo windo diffthis"
 
+		# Add an extra "-c" option to move to the first tab (notice that we
+		# can't simply append the command to the previous "-c" string as
+		# explained here: https://github.com/vim/vim/issues/9076
 
-	# Add an extra "-c" option to move to the first tab (notice that we
-	# can't simply append the command to the previous "-c" string as
-	# explained here: https://github.com/vim/vim/issues/9076
+		FINAL_CMD="-c \"$CMD\" -c \"tabfirst\""
+	else
+		CMD="$CMD | set hidden | let tmp=bufnr('%') | silent bufdo diffthis | exe 'buffer '.tmp"
 
-	FINAL_CMD="-c \"$CMD\" -c \"tabfirst\""
+		FINAL_CMD="-c \"$CMD\""
+	fi
 }
 
 
@@ -555,22 +556,22 @@  run_unit_tests () {
 	TEST_CASE_15="  ((  (LOCAL , BASE , REMOTE) / MERGED))   +(BASE)   , LOCAL+ BASE , REMOTE+ (((LOCAL / BASE / REMOTE)) ,    MERGED   )  "
 	TEST_CASE_16="LOCAL,BASE,REMOTE / MERGED + BASE,LOCAL + BASE,REMOTE + (LOCAL / BASE / REMOTE),MERGED"
 
-	EXPECTED_CMD_01="-c \"echo | leftabove split | leftabove vertical split | 1b | wincmd l | leftabove vertical split | 2b | wincmd l | 3b | wincmd j | 4b | tabdo windo diffthis\" -c \"tabfirst\""
-	EXPECTED_CMD_02="-c \"echo | leftabove vertical split | 1b | wincmd l | 3b | tabdo windo diffthis\" -c \"tabfirst\""
-	EXPECTED_CMD_03="-c \"echo | leftabove vertical split | 1b | wincmd l | leftabove vertical split | 4b | wincmd l | 3b | tabdo windo diffthis\" -c \"tabfirst\""
-	EXPECTED_CMD_04="-c \"echo | 4b | bufdo diffthis\" -c \"tabfirst\""
-	EXPECTED_CMD_05="-c \"echo | leftabove split | 1b | wincmd j | leftabove split | 4b | wincmd j | 3b | tabdo windo diffthis\" -c \"tabfirst\""
-	EXPECTED_CMD_06="-c \"echo | leftabove vertical split | leftabove split | 1b | wincmd j | 3b | wincmd l | 4b | tabdo windo diffthis\" -c \"tabfirst\""
-	EXPECTED_CMD_07="-c \"echo | leftabove vertical split | 4b | wincmd l | leftabove split | 1b | wincmd j | 3b | tabdo windo diffthis\" -c \"tabfirst\""
-	EXPECTED_CMD_08="-c \"echo | leftabove split | leftabove vertical split | 1b | wincmd l | 3b | wincmd j | 4b | tabdo windo diffthis\" -c \"tabfirst\""
-	EXPECTED_CMD_09="-c \"echo | leftabove split | 4b | wincmd j | leftabove vertical split | 1b | wincmd l | 3b | tabdo windo diffthis\" -c \"tabfirst\""
-	EXPECTED_CMD_10="-c \"echo | leftabove vertical split | leftabove split | 1b | wincmd j | leftabove split | 2b | wincmd j | 3b | wincmd l | 4b | tabdo windo diffthis\" -c \"tabfirst\""
-	EXPECTED_CMD_11="-c \"echo | -tabnew | leftabove split | leftabove vertical split | 1b | wincmd l | leftabove vertical split | 2b | wincmd l | 3b | wincmd j | 4b | tabnext | -tabnew | leftabove vertical split | 2b | wincmd l | 1b | tabnext | -tabnew | leftabove vertical split | 2b | wincmd l | 3b | tabnext | leftabove vertical split | leftabove split | 1b | wincmd j | leftabove split | 2b | wincmd j | 3b | wincmd l | 4b | tabdo windo diffthis\" -c \"tabfirst\""
-	EXPECTED_CMD_12="-c \"echo | leftabove vertical split | leftabove split | leftabove vertical split | 1b | wincmd l | 3b | wincmd j | 2b | wincmd l | 4b | tabdo windo diffthis\" -c \"tabfirst\""
-	EXPECTED_CMD_13="-c \"echo | leftabove vertical split | leftabove split | leftabove vertical split | 1b | wincmd l | 3b | wincmd j | 2b | wincmd l | leftabove vertical split | leftabove split | 1b | wincmd j | 3b | wincmd l | 4b | tabdo windo diffthis\" -c \"tabfirst\""
-	EXPECTED_CMD_14="-c \"echo | -tabnew | leftabove vertical split | 2b | wincmd l | 3b | tabnext | leftabove vertical split | 2b | wincmd l | 1b | tabdo windo diffthis\" -c \"tabfirst\""
-	EXPECTED_CMD_15="-c \"echo | -tabnew | leftabove split | leftabove vertical split | 1b | wincmd l | leftabove vertical split | 2b | wincmd l | 3b | wincmd j | 4b | tabnext | -tabnew | leftabove vertical split | 2b | wincmd l | 1b | tabnext | -tabnew | leftabove vertical split | 2b | wincmd l | 3b | tabnext | leftabove vertical split | leftabove split | 1b | wincmd j | leftabove split | 2b | wincmd j | 3b | wincmd l | 4b | tabdo windo diffthis\" -c \"tabfirst\""
-	EXPECTED_CMD_16="-c \"echo | -tabnew | leftabove split | leftabove vertical split | 1b | wincmd l | leftabove vertical split | 2b | wincmd l | 3b | wincmd j | 4b | tabnext | -tabnew | leftabove vertical split | 2b | wincmd l | 1b | tabnext | -tabnew | leftabove vertical split | 2b | wincmd l | 3b | tabnext | leftabove vertical split | leftabove split | 1b | wincmd j | leftabove split | 2b | wincmd j | 3b | wincmd l | 4b | tabdo windo diffthis\" -c \"tabfirst\""
+	EXPECTED_CMD_01="-c \"echo | leftabove split | leftabove vertical split | silent 1b | wincmd l | leftabove vertical split | silent 2b | wincmd l | silent 3b | wincmd j | silent 4b | set hidden | silent tabdo windo diffthis\" -c \"tabfirst\""
+	EXPECTED_CMD_02="-c \"echo | leftabove vertical split | silent 1b | wincmd l | silent 3b | set hidden | silent tabdo windo diffthis\" -c \"tabfirst\""
+	EXPECTED_CMD_03="-c \"echo | leftabove vertical split | silent 1b | wincmd l | leftabove vertical split | silent 4b | wincmd l | silent 3b | set hidden | silent tabdo windo diffthis\" -c \"tabfirst\""
+	EXPECTED_CMD_04="-c \"echo | silent 4b | set hidden | let tmp=bufnr('%') | silent bufdo diffthis | exe 'buffer '.tmp\""
+	EXPECTED_CMD_05="-c \"echo | leftabove split | silent 1b | wincmd j | leftabove split | silent 4b | wincmd j | silent 3b | set hidden | silent tabdo windo diffthis\" -c \"tabfirst\""
+	EXPECTED_CMD_06="-c \"echo | leftabove vertical split | leftabove split | silent 1b | wincmd j | silent 3b | wincmd l | silent 4b | set hidden | silent tabdo windo diffthis\" -c \"tabfirst\""
+	EXPECTED_CMD_07="-c \"echo | leftabove vertical split | silent 4b | wincmd l | leftabove split | silent 1b | wincmd j | silent 3b | set hidden | silent tabdo windo diffthis\" -c \"tabfirst\""
+	EXPECTED_CMD_08="-c \"echo | leftabove split | leftabove vertical split | silent 1b | wincmd l | silent 3b | wincmd j | silent 4b | set hidden | silent tabdo windo diffthis\" -c \"tabfirst\""
+	EXPECTED_CMD_09="-c \"echo | leftabove split | silent 4b | wincmd j | leftabove vertical split | silent 1b | wincmd l | silent 3b | set hidden | silent tabdo windo diffthis\" -c \"tabfirst\""
+	EXPECTED_CMD_10="-c \"echo | leftabove vertical split | leftabove split | silent 1b | wincmd j | leftabove split | silent 2b | wincmd j | silent 3b | wincmd l | silent 4b | set hidden | silent tabdo windo diffthis\" -c \"tabfirst\""
+	EXPECTED_CMD_11="-c \"echo | -tabnew | leftabove split | leftabove vertical split | silent 1b | wincmd l | leftabove vertical split | silent 2b | wincmd l | silent 3b | wincmd j | silent 4b | tabnext | -tabnew | leftabove vertical split | silent 2b | wincmd l | silent 1b | tabnext | -tabnew | leftabove vertical split | silent 2b | wincmd l | silent 3b | tabnext | leftabove vertical split | leftabove split | silent 1b | wincmd j | leftabove split | silent 2b | wincmd j | silent 3b | wincmd l | silent 4b | set hidden | silent tabdo windo diffthis\" -c \"tabfirst\""
+	EXPECTED_CMD_12="-c \"echo | leftabove vertical split | leftabove split | leftabove vertical split | silent 1b | wincmd l | silent 3b | wincmd j | silent 2b | wincmd l | silent 4b | set hidden | silent tabdo windo diffthis\" -c \"tabfirst\""
+	EXPECTED_CMD_13="-c \"echo | leftabove vertical split | leftabove split | leftabove vertical split | silent 1b | wincmd l | silent 3b | wincmd j | silent 2b | wincmd l | leftabove vertical split | leftabove split | silent 1b | wincmd j | silent 3b | wincmd l | silent 4b | set hidden | silent tabdo windo diffthis\" -c \"tabfirst\""
+	EXPECTED_CMD_14="-c \"echo | -tabnew | leftabove vertical split | silent 2b | wincmd l | silent 3b | tabnext | leftabove vertical split | silent 2b | wincmd l | silent 1b | set hidden | silent tabdo windo diffthis\" -c \"tabfirst\""
+	EXPECTED_CMD_15="-c \"echo | -tabnew | leftabove split | leftabove vertical split | silent 1b | wincmd l | leftabove vertical split | silent 2b | wincmd l | silent 3b | wincmd j | silent 4b | tabnext | -tabnew | leftabove vertical split | silent 2b | wincmd l | silent 1b | tabnext | -tabnew | leftabove vertical split | silent 2b | wincmd l | silent 3b | tabnext | leftabove vertical split | leftabove split | silent 1b | wincmd j | leftabove split | silent 2b | wincmd j | silent 3b | wincmd l | silent 4b | set hidden | silent tabdo windo diffthis\" -c \"tabfirst\""
+	EXPECTED_CMD_16="-c \"echo | -tabnew | leftabove split | leftabove vertical split | silent 1b | wincmd l | leftabove vertical split | silent 2b | wincmd l | silent 3b | wincmd j | silent 4b | tabnext | -tabnew | leftabove vertical split | silent 2b | wincmd l | silent 1b | tabnext | -tabnew | leftabove vertical split | silent 2b | wincmd l | silent 3b | tabnext | leftabove vertical split | leftabove split | silent 1b | wincmd j | leftabove split | silent 2b | wincmd j | silent 3b | wincmd l | silent 4b | set hidden | silent tabdo windo diffthis\" -c \"tabfirst\""
 
 	EXPECTED_TARGET_01="MERGED"
 	EXPECTED_TARGET_02="LOCAL"
@@ -635,7 +636,7 @@  run_unit_tests () {
 	cat >expect <<-\EOF
 	-f
 	-c
-	echo | leftabove split | leftabove vertical split | 1b | wincmd l | leftabove vertical split | quit | wincmd l | 2b | wincmd j | 3b | tabdo windo diffthis
+	echo | leftabove split | leftabove vertical split | silent 1b | wincmd l | leftabove vertical split | silent quit | wincmd l | silent 2b | wincmd j | silent 3b | set hidden | silent tabdo windo diffthis
 	-c
 	tabfirst
 	lo cal