From e2d74193c0e74666498004f5ef7a3473e8993063 Mon Sep 17 00:00:00 2001 From: Fernando Ramos Date: Tue, 25 Mar 2025 23:23:10 +0100 Subject: [PATCH 1/2] mergetools: vimdiff: fix layout where REMOTE is the target "mergetool.vimdiff.layout" is used to define the vim layout (ie. how windows, tabs and buffers are physically organized) when resolving conflicts. For example, if we set it to this: "(LOCAL,BASE,REMOTE)/MERGED" ...vim will open and show this layout: ------------------------------------------ | | | | | LOCAL | BASE | REMOTE | | | | | ------------------------------------------ | | | MERGED | | | ------------------------------------------ By default, whatever ends up been written to the "MERGED" window will become the file which conflict we are resolving. However, it is possible to use the "@" symbol to specify a different one. For example, if we use this slightly different version of the previously used string: "(LOCAL,BASE,@REMOTE)/MERGED" ...then the user should proceed to edit the contents of the top right window (instead of the bottom window) as *that* is what will become the conflicts free file once vim is closed. Before this commit, the "@" marker worked for all targets *except* for "REMOTE". In other words, these worked as expected: "(@LOCAL,BASE,REMOTE)/MERGED" "(LOCAL,@BASE,REMOTE)/MERGED" "(LOCAL,BASE,REMOTE)/@MERGED" ...but this didn't: "(LOCAL,BASE,@REMOTE)/MERGED" This commit fixes that. Reported-by: kawarimidoll Suggested-by: D. Ben Knoble Signed-off-by: Fernando Ramos Signed-off-by: Junio C Hamano --- mergetools/vimdiff | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mergetools/vimdiff b/mergetools/vimdiff index f8ad6b35d4..e536ab6bfe 100644 --- a/mergetools/vimdiff +++ b/mergetools/vimdiff @@ -305,6 +305,9 @@ gen_cmd () { elif echo "$LAYOUT" | grep @BASE >/dev/null then FINAL_TARGET="BASE" + elif echo "$LAYOUT" | grep @REMOTE >/dev/null + then + FINAL_TARGET="REMOTE" else FINAL_TARGET="MERGED" fi From 93bab2d04be7ac304a5dd899daccd3e357126f79 Mon Sep 17 00:00:00 2001 From: Fernando Ramos Date: Tue, 25 Mar 2025 23:23:11 +0100 Subject: [PATCH 2/2] mergetools: vimdiff: add tests for layout with REMOTE as the target Add some tests to make sure that now "REMOTE" can be used as a target (ie. can be used together with the "@" marker) inside "mergetool.vimdiff.layout" Signed-off-by: Fernando Ramos Signed-off-by: Junio C Hamano --- mergetools/vimdiff | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/mergetools/vimdiff b/mergetools/vimdiff index e536ab6bfe..133fbc45d4 100644 --- a/mergetools/vimdiff +++ b/mergetools/vimdiff @@ -532,7 +532,7 @@ run_unit_tests () { # Function to make sure that we don't break anything when modifying this # script. - NUMBER_OF_TEST_CASES=16 + NUMBER_OF_TEST_CASES=19 TEST_CASE_01="(LOCAL,BASE,REMOTE)/MERGED" # default behaviour TEST_CASE_02="@LOCAL,REMOTE" # when using vimdiff1 @@ -550,6 +550,9 @@ run_unit_tests () { TEST_CASE_14="BASE,REMOTE+BASE,LOCAL" 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" + TEST_CASE_17="(LOCAL,@BASE,REMOTE)/MERGED" + TEST_CASE_18="LOCAL,@REMOTE" + TEST_CASE_19="@REMOTE" EXPECTED_CMD_01="-c \"set hidden diffopt-=hiddenoff | echo | leftabove split | leftabove vertical split | 1b | wincmd l | leftabove vertical split | 2b | wincmd l | 3b | wincmd j | 4b | execute 'tabdo windo diffthis' | tabfirst\"" EXPECTED_CMD_02="-c \"set hidden diffopt-=hiddenoff | echo | leftabove vertical split | 1b | wincmd l | 3b | execute 'tabdo windo diffthis' | tabfirst\"" @@ -567,6 +570,9 @@ run_unit_tests () { EXPECTED_CMD_14="-c \"set hidden diffopt-=hiddenoff | echo | leftabove vertical split | 2b | wincmd l | 3b | tabnew | leftabove vertical split | 2b | wincmd l | 1b | execute 'tabdo windo diffthis' | tabfirst\"" EXPECTED_CMD_15="-c \"set hidden diffopt-=hiddenoff | echo | leftabove split | leftabove vertical split | 1b | wincmd l | leftabove vertical split | 2b | wincmd l | 3b | wincmd j | 4b | tabnew | leftabove vertical split | 2b | wincmd l | 1b | tabnew | leftabove vertical split | 2b | wincmd l | 3b | tabnew | leftabove vertical split | leftabove split | 1b | wincmd j | leftabove split | 2b | wincmd j | 3b | wincmd l | 4b | execute 'tabdo windo diffthis' | tabfirst\"" EXPECTED_CMD_16="-c \"set hidden diffopt-=hiddenoff | echo | leftabove split | leftabove vertical split | 1b | wincmd l | leftabove vertical split | 2b | wincmd l | 3b | wincmd j | 4b | tabnew | leftabove vertical split | 2b | wincmd l | 1b | tabnew | leftabove vertical split | 2b | wincmd l | 3b | tabnew | leftabove vertical split | leftabove split | 1b | wincmd j | leftabove split | 2b | wincmd j | 3b | wincmd l | 4b | execute 'tabdo windo diffthis' | tabfirst\"" + EXPECTED_CMD_17="-c \"set hidden diffopt-=hiddenoff | echo | leftabove split | leftabove vertical split | 1b | wincmd l | leftabove vertical split | 2b | wincmd l | 3b | wincmd j | 4b | execute 'tabdo windo diffthis' | tabfirst\"" + EXPECTED_CMD_18="-c \"set hidden diffopt-=hiddenoff | echo | leftabove vertical split | 1b | wincmd l | 3b | execute 'tabdo windo diffthis' | tabfirst\"" + EXPECTED_CMD_19="-c \"set hidden diffopt-=hiddenoff | echo | silent execute 'bufdo diffthis' | 3b | execute 'tabdo windo diffthis' | tabfirst\"" EXPECTED_TARGET_01="MERGED" EXPECTED_TARGET_02="LOCAL" @@ -584,6 +590,9 @@ run_unit_tests () { EXPECTED_TARGET_14="MERGED" EXPECTED_TARGET_15="MERGED" EXPECTED_TARGET_16="MERGED" + EXPECTED_TARGET_17="BASE" + EXPECTED_TARGET_18="REMOTE" + EXPECTED_TARGET_19="REMOTE" at_least_one_ko="false"