{"thread":{"id":"58259","subject":"[PATCH 0/2] mergetools: vimdiff3: fix regression","startedAt":"2022-08-02T21:41:41Z","lastAt":"2022-08-07T00:55:23Z","messageCount":15,"participants":["Felipe Contreras","Fernando Ramos"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"460507","messageId":"20220802214134.681300-1-felipe.contreras@gmail.com","threadId":"58259","inReplyTo":null,"subject":"[PATCH 0/2] mergetools: vimdiff3: fix regression","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2022-08-02T21:41:32Z","receivedAt":"2022-08-02T21:41:41Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Hello,\n\nI wrote vimdiff3 to leverage both the power of git's diff3 and vim's\ndiff mode, but commit 0041797449 broke that.\n\nHere you can see how it used to work:\n\nhttps://i.snipboard.io/hSdfkj.jpg\n\nThe added and changed lines are properly highlighted.\n\nAfter I fix the conflicts vim still properly highlights which lines were\nchanged, and even what specific characters were modified:\n\nhttps://i.snipboard.io/HvpULI.jpg\n\nNow I get absolutely nothing:\n\nhttps://i.snipboard.io/HXMui4.jpg\n\nTo get the highlighting the content has to be in a window, and only\n*after* the diff mode has done its job can it be hidden. The current\ncode does nothing of the sort.\n\nAdditionally, every time I run the command I get an annoying message:\n\n  \"./content_LOCAL_8975\" 6L, 28B\n  \"./content_BASE_8975\" 6 lines, 29 bytes\n  \"./content_REMOTE_8975\" 6 lines, 29 bytes\n  \"content\" 16 lines, 115 bytes\n  Press ENTER or type command to continue\n\nBecause that's what `bufdo` does\n\nHere's the patch that restores the intended behavior so vimdiff3\nactually does something.\n\nAdditionally I noticed that vimdiff3 relied on specific values of\n`diffopt`, specifically `closeoff` not being set. This worked fine in my\nsetup, but vim has `closeoff` enabled by default. So I'm sending a patch\nto make it work regardless of the user configuration.\n\nFelipe Contreras (2):\n  mergetools: vimdiff3: make it work as intended\n  mergetools: vimdiff3: fix diffopt options\n\n mergetools/vimdiff | 38 ++++++++++++++++++++++++--------------\n 1 file changed, 24 insertions(+), 14 deletions(-)\n\n-- \n2.37.1.313.ge269dbcbc5\n\n"},{"id":"460508","messageId":"20220802214134.681300-2-felipe.contreras@gmail.com","threadId":"58259","inReplyTo":"20220802214134.681300-1-felipe.contreras@gmail.com","subject":"[PATCH 1/2] mergetools: vimdiff3: make it work as intended","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2022-08-02T21:41:33Z","receivedAt":"2022-08-02T21:41:43Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"When vimdiff3 was added in 7c147b77d3 (mergetools: add vimdiff3 mode,\n2014-04-20), the description made clear the intention:\n\nIt's similar to the default, except that the other windows are hidden.\nThis ensures that removed/added colors are still visible on the main\nmerge window, but the other windows not visible.\n\nHowever, in 0041797449 (vimdiff: new implementation with layout support,\n2022-03-30) this was broken by generating a command that never creates\nwindows, and therefore vim never shows the diff.\n\nIn order to show the diff, the windows need to be created first, and\nthen when they are hidden the diff remains (if hidenoff isn't set).\n\nThe layout support implementation broke the whole purpose of vimdiff3,\nand simply shows MERGED, which is no different from simply opening the\nfile with vim.\n\nWe could restore the correct behavior by modifying gen_cmd to open all\nthe windows, and then hide them, but there's no need to do that when the\n-d option of vim (vimdiff) does precisely that.\n\nSo let's skip the whole gen_cmd function for vimdiff3, and hide the\nwindows, therefore restoring the previous intended behavior.\n\nCc: Fernando Ramos <greenfoo@u92.eu>\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n mergetools/vimdiff | 38 ++++++++++++++++++++++++--------------\n 1 file changed, 24 insertions(+), 14 deletions(-)\n\ndiff --git a/mergetools/vimdiff b/mergetools/vimdiff\nindex f770b8fe24..f4c3bf6d11 100644\n--- a/mergetools/vimdiff\n+++ b/mergetools/vimdiff\n@@ -388,26 +388,36 @@ merge_cmd () {\n \tlayout=$(git config mergetool.vimdiff.layout)\n \n \tcase \"$1\" in\n-\t*vimdiff)\n-\t\tif test -z \"$layout\"\n+\t*vimdiff3)\n+\t\tif $base_present\n \t\tthen\n-\t\t\t# Default layout when none is specified\n-\t\t\tlayout=\"(LOCAL,BASE,REMOTE)/MERGED\"\n+\t\t\tCMD='hid | hid | hid'\n+\t\telse\n+\t\t\tCMD='hid | hid'\n \t\tfi\n+\t\tFINAL_CMD=\"-d -c '$CMD'\"\n \t\t;;\n-\t*vimdiff1)\n-\t\tlayout=\"@LOCAL,REMOTE\"\n-\t\t;;\n-\t*vimdiff2)\n-\t\tlayout=\"LOCAL,MERGED,REMOTE\"\n-\t\t;;\n-\t*vimdiff3)\n-\t\tlayout=\"MERGED\"\n+\t*)\n+\t\tcase \"$1\" in\n+\t\t*vimdiff)\n+\t\t\tif test -z \"$layout\"\n+\t\t\tthen\n+\t\t\t\t# Default layout when none is specified\n+\t\t\t\tlayout=\"(LOCAL,BASE,REMOTE)/MERGED\"\n+\t\t\tfi\n+\t\t\t;;\n+\t\t*vimdiff1)\n+\t\t\tlayout=\"@LOCAL,REMOTE\"\n+\t\t\t;;\n+\t\t*vimdiff2)\n+\t\t\tlayout=\"LOCAL,MERGED,REMOTE\"\n+\t\t\t;;\n+\t\tesac\n+\n+\t\tgen_cmd \"$layout\"\n \t\t;;\n \tesac\n \n-\tgen_cmd \"$layout\"\n-\n \tdebug_print \"\"\n \tdebug_print \"FINAL CMD : $FINAL_CMD\"\n \tdebug_print \"FINAL TAR : $FINAL_TARGET\"\n-- \n2.37.1.313.ge269dbcbc5\n\n"},{"id":"460509","messageId":"20220802214134.681300-3-felipe.contreras@gmail.com","threadId":"58259","inReplyTo":"20220802214134.681300-1-felipe.contreras@gmail.com","subject":"[PATCH 2/2] mergetools: vimdiff3: fix diffopt options","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2022-08-02T21:41:34Z","receivedAt":"2022-08-02T21:41:44Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"If either closeoff or hiddenoff are enabled, vimdiff3 doesn't work as\nintended, so turn them off.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n mergetools/vimdiff | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/mergetools/vimdiff b/mergetools/vimdiff\nindex f4c3bf6d11..baccabc403 100644\n--- a/mergetools/vimdiff\n+++ b/mergetools/vimdiff\n@@ -395,7 +395,7 @@ merge_cmd () {\n \t\telse\n \t\t\tCMD='hid | hid'\n \t\tfi\n-\t\tFINAL_CMD=\"-d -c '$CMD'\"\n+\t\tFINAL_CMD=\"-d -c 'setl diffopt-=closeoff diffopt-=hiddenoff | $CMD'\"\n \t\t;;\n \t*)\n \t\tcase \"$1\" in\n-- \n2.37.1.313.ge269dbcbc5\n\n"},{"id":"460748","messageId":"Yu6V4cIajhoMhB3t@zacax395.localdomain","threadId":"58259","inReplyTo":"20220802214134.681300-1-felipe.contreras@gmail.com","subject":"Re: [PATCH 0/2] mergetools: vimdiff3: fix regression","fromName":"Fernando Ramos","fromEmail":"greenfoo@u92.eu","sentAt":"2022-08-06T16:25:05Z","receivedAt":"2022-08-06T16:25:20Z","isPatch":true,"sender":{"key":"greenfoo@u92.eu","avatar":"https://avatars.githubusercontent.com/u/42691086?v=4"},"body":"On 22/08/02 04:41PM, Felipe Contreras wrote:\n> Hello,\n> \n> I wrote vimdiff3 to leverage both the power of git's diff3 and vim's\n> diff mode, but commit 0041797449 broke that.\n> \n\nHi Felipe,\n\nThis is the command that runs now when using 'git merge -t vimdiff3':\n\n    vim -c \"echo | 4b | bufdo diffthis\" -c \"tabfirst\" LOCAL BASE REMOTE MERGED\n\n...and this is the command that runs after your patch:\n\n    vim -d -c \"setl diffopt-=closeoff diffopt-=hiddenoff | hid | hid | hid\" LOCAL BASE REMOTE MERGED\n\nThe new command you suggest is meant to improve two aspects:\n\n    1. Preserves diff colors.\n    2. Removes the \"Press ENTER\" message.\n\nRegarding (1) I never noticed this because in my tests colors were always\nshown...  but I just tried to run with '-u NONE' (which prevents .vimrc from\nbeing loaded) and you are right: there are now no colors.\n\n    vim -u NONE -c \"echo | 4b | bufdo diffthis\" -c \"tabfirst\" LOCAL BASE REMOTE MERGED\n        ^^^^^^^\n        `-> Tell vim not to load .vimrc\n\nSo... I started looking into my .vimrc and found the \"problem\":\n\n    set hidden\n\nBy default this option is *not* set, which means buffers are discarded when\nhidden (and that's why diff colors dissapear). By setting this option colors are\nback even with '-u NONE':\n\n    vim -u NONE -c \"echo | set hidden | 4b | bufdo diffthis\" -c \"tabfirst\" LOCAL BASE REMOTE MERGED\n                           ^^^^^^^^^^\n\nRegarding (2) we can remove the \"Press ENTER\" message by adding \"silent\" to both\n\"4b\" and \"bufdo\", like this:\n\n    vim -u NONE -c \"echo | set hidden | silent 4b | silent bufdo diffthis\" -c \"tabfirst\" LOCAL BASE REMOTE MERGED\n                                        ^^^^^^      ^^^^^^\n\nSo... by making two changes to the current implementation (adding \"set hidden\"\nand \"silent\") we can make it work. The nice thing is that, this way, \"vimdiff3\"\ndoes not need to be treated as an exception and thus it will be (hopefully)\neasier to maintain.\n\nWhat do you think? :)\n\nThanks!\n\nFernando.\n\nPS: I'll reply to this message with a patch that implements this in case we\ndecide to go this route.\n"},{"id":"460749","messageId":"Yu6WcRM7NNVXjt5D@zacax395.localdomain","threadId":"58259","inReplyTo":"Yu6V4cIajhoMhB3t@zacax395.localdomain","subject":"Re: [PATCH 0/2] mergetools: vimdiff3: fix regression","fromName":"Fernando Ramos","fromEmail":"greenfoo@u92.eu","sentAt":"2022-08-06T16:27:29Z","receivedAt":"2022-08-06T16:27:38Z","isPatch":true,"sender":{"key":"greenfoo@u92.eu","avatar":"https://avatars.githubusercontent.com/u/42691086?v=4"},"body":"vimdiff3 was introduced in 7c147b77d3 (mergetools: add vimdiff3 mode,\n2014-04-20) and then partially broken in 0041797449 (vimdiff: new\nimplementation with layout support, 2022-03-30) in two ways:\n\n  - It does not show colors unless the user has \"set hidden\" in his\n    .vimrc file\n\n  - It prompts the user to \"Press ENTER\" every time it runs.\n\nThis patch fixes both issues by adding \"set hidden\" and \"silent\" to the\ngenerated command string that is used to run vim.\n\nCc: Felipe Contreras <felipe.contreras@gmail.com>\nSigned-off-by: Fernando Ramos <greenfoo@u92.eu>\n---\n mergetools/vimdiff | 48 +++++++++++++++++++++++-----------------------\n 1 file changed, 24 insertions(+), 24 deletions(-)\n\ndiff --git a/mergetools/vimdiff b/mergetools/vimdiff\nindex f770b8fe24..a64134364c 100644\n--- a/mergetools/vimdiff\n+++ b/mergetools/vimdiff\n@@ -261,19 +261,19 @@ gen_cmd_aux () {\n \n \tif test \"$target\" = \"LOCAL\"\n \tthen\n-\t\tCMD=\"$CMD | 1b\"\n+\t\tCMD=\"$CMD | silent 1b\"\n \n \telif test \"$target\" = \"BASE\"\n \tthen\n-\t\tCMD=\"$CMD | 2b\"\n+\t\tCMD=\"$CMD | silent 2b\"\n \n \telif test \"$target\" = \"REMOTE\"\n \tthen\n-\t\tCMD=\"$CMD | 3b\"\n+\t\tCMD=\"$CMD | silent 3b\"\n \n \telif test \"$target\" = \"MERGED\"\n \tthen\n-\t\tCMD=\"$CMD | 4b\"\n+\t\tCMD=\"$CMD | silent 4b\"\n \n \telse\n \t\tCMD=\"$CMD | ERROR: >$target<\"\n@@ -310,7 +310,7 @@ gen_cmd () {\n \t#\n \t#     gen_cmd \"@LOCAL , REMOTE\"\n \t#     |\n-\t#     `-> FINAL_CMD    == \"-c \\\"echo | leftabove vertical split | 1b | wincmd l | 3b | tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n+\t#     `-> FINAL_CMD    == \"-c \\\"echo | leftabove vertical split | silent 1b | wincmd l | silent 3b | silent tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n \t#         FINAL_TARGET == \"LOCAL\"\n \n \tLAYOUT=$1\n@@ -341,9 +341,9 @@ gen_cmd () {\n \n \tif echo \"$LAYOUT\" | grep \",\\|/\" >/dev/null\n \tthen\n-\t\tCMD=\"$CMD | tabdo windo diffthis\"\n+\t\tCMD=\"$CMD | silent tabdo windo diffthis\"\n \telse\n-\t\tCMD=\"$CMD | bufdo diffthis\"\n+\t\tCMD=\"$CMD | set hidden | silent bufdo diffthis\"\n \tfi\n \n \n@@ -555,22 +555,22 @@ run_unit_tests () {\n \tTEST_CASE_15=\"  ((  (LOCAL , BASE , REMOTE) / MERGED))   +(BASE)   , LOCAL+ BASE , REMOTE+ (((LOCAL / BASE / REMOTE)) ,    MERGED   )  \"\n \tTEST_CASE_16=\"LOCAL,BASE,REMOTE / MERGED + BASE,LOCAL + BASE,REMOTE + (LOCAL / BASE / REMOTE),MERGED\"\n \n-\tEXPECTED_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\\\"\"\n-\tEXPECTED_CMD_02=\"-c \\\"echo | leftabove vertical split | 1b | wincmd l | 3b | tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n-\tEXPECTED_CMD_03=\"-c \\\"echo | leftabove vertical split | 1b | wincmd l | leftabove vertical split | 4b | wincmd l | 3b | tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n-\tEXPECTED_CMD_04=\"-c \\\"echo | 4b | bufdo diffthis\\\" -c \\\"tabfirst\\\"\"\n-\tEXPECTED_CMD_05=\"-c \\\"echo | leftabove split | 1b | wincmd j | leftabove split | 4b | wincmd j | 3b | tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n-\tEXPECTED_CMD_06=\"-c \\\"echo | leftabove vertical split | leftabove split | 1b | wincmd j | 3b | wincmd l | 4b | tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n-\tEXPECTED_CMD_07=\"-c \\\"echo | leftabove vertical split | 4b | wincmd l | leftabove split | 1b | wincmd j | 3b | tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n-\tEXPECTED_CMD_08=\"-c \\\"echo | leftabove split | leftabove vertical split | 1b | wincmd l | 3b | wincmd j | 4b | tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n-\tEXPECTED_CMD_09=\"-c \\\"echo | leftabove split | 4b | wincmd j | leftabove vertical split | 1b | wincmd l | 3b | tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n-\tEXPECTED_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\\\"\"\n-\tEXPECTED_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\\\"\"\n-\tEXPECTED_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\\\"\"\n-\tEXPECTED_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\\\"\"\n-\tEXPECTED_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\\\"\"\n-\tEXPECTED_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\\\"\"\n-\tEXPECTED_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\\\"\"\n+\tEXPECTED_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 | silent tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n+\tEXPECTED_CMD_02=\"-c \\\"echo | leftabove vertical split | silent 1b | wincmd l | silent 3b | silent tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n+\tEXPECTED_CMD_03=\"-c \\\"echo | leftabove vertical split | silent 1b | wincmd l | leftabove vertical split | silent 4b | wincmd l | silent 3b | silent tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n+\tEXPECTED_CMD_04=\"-c \\\"echo | silent 4b | set hidden | silent bufdo diffthis\\\" -c \\\"tabfirst\\\"\"\n+\tEXPECTED_CMD_05=\"-c \\\"echo | leftabove split | silent 1b | wincmd j | leftabove split | silent 4b | wincmd j | silent 3b | silent tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n+\tEXPECTED_CMD_06=\"-c \\\"echo | leftabove vertical split | leftabove split | silent 1b | wincmd j | silent 3b | wincmd l | silent 4b | silent tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n+\tEXPECTED_CMD_07=\"-c \\\"echo | leftabove vertical split | silent 4b | wincmd l | leftabove split | silent 1b | wincmd j | silent 3b | silent tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n+\tEXPECTED_CMD_08=\"-c \\\"echo | leftabove split | leftabove vertical split | silent 1b | wincmd l | silent 3b | wincmd j | silent 4b | silent tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n+\tEXPECTED_CMD_09=\"-c \\\"echo | leftabove split | silent 4b | wincmd j | leftabove vertical split | silent 1b | wincmd l | silent 3b | silent tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n+\tEXPECTED_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 | silent tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n+\tEXPECTED_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 | silent tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n+\tEXPECTED_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 | silent tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n+\tEXPECTED_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 | silent tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n+\tEXPECTED_CMD_14=\"-c \\\"echo | -tabnew | leftabove vertical split | silent 2b | wincmd l | silent 3b | tabnext | leftabove vertical split | silent 2b | wincmd l | silent 1b | silent tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n+\tEXPECTED_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 | silent tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n+\tEXPECTED_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 | silent tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n \n \tEXPECTED_TARGET_01=\"MERGED\"\n \tEXPECTED_TARGET_02=\"LOCAL\"\n@@ -635,7 +635,7 @@ run_unit_tests () {\n \tcat >expect <<-\\EOF\n \t-f\n \t-c\n-\techo | leftabove split | leftabove vertical split | 1b | wincmd l | leftabove vertical split | quit | wincmd l | 2b | wincmd j | 3b | tabdo windo diffthis\n+\techo | leftabove split | leftabove vertical split | silent 1b | wincmd l | leftabove vertical split | silent quit | wincmd l | silent 2b | wincmd j | silent 3b | silent tabdo windo diffthis\n \t-c\n \ttabfirst\n \tlo cal\n-- \n2.37.1\n\n"},{"id":"460750","messageId":"CAMP44s1uPFGYVJ7dzf1pFXENnUjTTwxHye2iT_HPNiMcmPjD9A@mail.gmail.com","threadId":"58259","inReplyTo":"Yu6V4cIajhoMhB3t@zacax395.localdomain","subject":"Re: [PATCH 0/2] mergetools: vimdiff3: fix regression","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2022-08-06T17:53:24Z","receivedAt":"2022-08-06T17:53:40Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Hello,\n\nOn Sat, Aug 6, 2022 at 11:25 AM Fernando Ramos <greenfoo@u92.eu> wrote:\n> On 22/08/02 04:41PM, Felipe Contreras wrote:\n\n> > I wrote vimdiff3 to leverage both the power of git's diff3 and vim's\n> > diff mode, but commit 0041797449 broke that.\n\n> By default this option is *not* set, which means buffers are discarded when\n> hidden (and that's why diff colors dissapear). By setting this option colors are\n> back even with '-u NONE':\n>\n>     vim -u NONE -c \"echo | set hidden | 4b | bufdo diffthis\" -c \"tabfirst\" LOCAL BASE REMOTE MERGED\n>                            ^^^^^^^^^^\n\nCorrect.\n\n> Regarding (2) we can remove the \"Press ENTER\" message by adding \"silent\" to both\n> \"4b\" and \"bufdo\", like this:\n>\n>     vim -u NONE -c \"echo | set hidden | silent 4b | silent bufdo diffthis\" -c \"tabfirst\" LOCAL BASE REMOTE MERGED\n>                                         ^^^^^^      ^^^^^^\n\nCorrect.\n\n> So... by making two changes to the current implementation (adding \"set hidden\"\n> and \"silent\") we can make it work. The nice thing is that, this way, \"vimdiff3\"\n> does not need to be treated as an exception and thus it will be (hopefully)\n> easier to maintain.\n>\n> What do you think? :)\n\nThis could work. The result is not quite the same as with vimdiff, but\nthe difference is minimal.\n\nTwo observations though.\n\n1. The \"silent 4b\" is ignored, since bufdo makes the last buffer the\ncurrent buffer, so if you want a different buffer you have to make the\nswitch *after* bufdo.\n\n2. You probably want to do \"set hidden\" on all the modes.\n\nI don't see the need for all this complexity for this simple mode, but\nanything that actually works is fine by me.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"460751","messageId":"Yu6zEiknXKFMJUVn@zacax395.localdomain","threadId":"58259","inReplyTo":"CAMP44s1uPFGYVJ7dzf1pFXENnUjTTwxHye2iT_HPNiMcmPjD9A@mail.gmail.com","subject":"Re: [PATCH 0/2] mergetools: vimdiff3: fix regression","fromName":"Fernando Ramos","fromEmail":"greenfoo@u92.eu","sentAt":"2022-08-06T18:29:38Z","receivedAt":"2022-08-06T18:29:49Z","isPatch":true,"sender":{"key":"greenfoo@u92.eu","avatar":"https://avatars.githubusercontent.com/u/42691086?v=4"},"body":"On 22/08/06 12:53PM, Felipe Contreras wrote:\n> Two observations though.\n> \n> 1. The \"silent 4b\" is ignored, since bufdo makes the last buffer the\n> current buffer, so if you want a different buffer you have to make the\n> switch *after* bufdo.\n> \n\nYes, you are right. For the particular case where there are no windows (only\nhidden buffers) it does not have any effect. It's presence there comes from\nthe fact that the command generation function works in the most \"generic\" way\n(ie. producing output that works for all cases: windows, tabs and buffers).\n\nIn order not to have another special case in the generation logic I left it\nthere, but you are right in that it is not needed (fortunately it also doesn't\nmake any harm :)\n\n\n> 2. You probably want to do \"set hidden\" on all the modes.\n> \n\nYou are right. It also makes the logic more symmetric. I'll add it.\n\n\n> I don't see the need for all this complexity for this simple mode, but\n> anything that actually works is fine by me.\n> \n\n...in fact, back in May I just wanted to add a new \"vimdiff4\" mode and what\noriginally was a 5 lines patch became the current 1000+ lines patch monster\nafter all the (very welcomed, I'm not complaining!) suggestions :)\n\n\nI'll reply to this message with a new version of the patch with your \"set\nhidden\" suggestion. Thanks!\n\n\n"},{"id":"460752","messageId":"20220806183757.72168-1-greenfoo@u92.eu","threadId":"58259","inReplyTo":"Yu6zEiknXKFMJUVn@zacax395.localdomain","subject":"[PATCH] vimdiff: fix 'vimdiff3' behavior (colors + no extra key press)","fromName":"Fernando Ramos","fromEmail":"greenfoo@u92.eu","sentAt":"2022-08-06T18:37:57Z","receivedAt":"2022-08-06T18:38:07Z","isPatch":true,"sender":{"key":"greenfoo@u92.eu","avatar":"https://avatars.githubusercontent.com/u/42691086?v=4"},"body":"vimdiff3 was introduced in 7c147b77d3 (mergetools: add vimdiff3 mode,\n2014-04-20) and then partially broken in 0041797449 (vimdiff: new\nimplementation with layout support, 2022-03-30) in two ways:\n\n  - It does not show colors unless the user has \"set hidden\" in his\n    .vimrc file\n\n  - It prompts the user to \"Press ENTER\" every time it runs.\n\nThis patch fixes both issues by adding \"set hidden\" and \"silent\" to the\ngenerated command string that is used to run vim.\n\nCc: Felipe Contreras <felipe.contreras@gmail.com>\nSigned-off-by: Fernando Ramos <greenfoo@u92.eu>\n---\n mergetools/vimdiff | 48 +++++++++++++++++++++++-----------------------\n 1 file changed, 24 insertions(+), 24 deletions(-)\n\ndiff --git a/mergetools/vimdiff b/mergetools/vimdiff\nindex f770b8fe24..461b8f394f 100644\n--- a/mergetools/vimdiff\n+++ b/mergetools/vimdiff\n@@ -261,19 +261,19 @@ gen_cmd_aux () {\n \n \tif test \"$target\" = \"LOCAL\"\n \tthen\n-\t\tCMD=\"$CMD | 1b\"\n+\t\tCMD=\"$CMD | silent 1b\"\n \n \telif test \"$target\" = \"BASE\"\n \tthen\n-\t\tCMD=\"$CMD | 2b\"\n+\t\tCMD=\"$CMD | silent 2b\"\n \n \telif test \"$target\" = \"REMOTE\"\n \tthen\n-\t\tCMD=\"$CMD | 3b\"\n+\t\tCMD=\"$CMD | silent 3b\"\n \n \telif test \"$target\" = \"MERGED\"\n \tthen\n-\t\tCMD=\"$CMD | 4b\"\n+\t\tCMD=\"$CMD | silent 4b\"\n \n \telse\n \t\tCMD=\"$CMD | ERROR: >$target<\"\n@@ -310,7 +310,7 @@ gen_cmd () {\n \t#\n \t#     gen_cmd \"@LOCAL , REMOTE\"\n \t#     |\n-\t#     `-> FINAL_CMD    == \"-c \\\"echo | leftabove vertical split | 1b | wincmd l | 3b | tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n+\t#     `-> FINAL_CMD    == \"-c \\\"echo | leftabove vertical split | silent 1b | wincmd l | silent 3b | set hidden | silent tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n \t#         FINAL_TARGET == \"LOCAL\"\n \n \tLAYOUT=$1\n@@ -341,9 +341,9 @@ gen_cmd () {\n \n \tif echo \"$LAYOUT\" | grep \",\\|/\" >/dev/null\n \tthen\n-\t\tCMD=\"$CMD | tabdo windo diffthis\"\n+\t\tCMD=\"$CMD | set hidden | silent tabdo windo diffthis\"\n \telse\n-\t\tCMD=\"$CMD | bufdo diffthis\"\n+\t\tCMD=\"$CMD | set hidden | silent bufdo diffthis\"\n \tfi\n \n \n@@ -555,22 +555,22 @@ run_unit_tests () {\n \tTEST_CASE_15=\"  ((  (LOCAL , BASE , REMOTE) / MERGED))   +(BASE)   , LOCAL+ BASE , REMOTE+ (((LOCAL / BASE / REMOTE)) ,    MERGED   )  \"\n \tTEST_CASE_16=\"LOCAL,BASE,REMOTE / MERGED + BASE,LOCAL + BASE,REMOTE + (LOCAL / BASE / REMOTE),MERGED\"\n \n-\tEXPECTED_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\\\"\"\n-\tEXPECTED_CMD_02=\"-c \\\"echo | leftabove vertical split | 1b | wincmd l | 3b | tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n-\tEXPECTED_CMD_03=\"-c \\\"echo | leftabove vertical split | 1b | wincmd l | leftabove vertical split | 4b | wincmd l | 3b | tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n-\tEXPECTED_CMD_04=\"-c \\\"echo | 4b | bufdo diffthis\\\" -c \\\"tabfirst\\\"\"\n-\tEXPECTED_CMD_05=\"-c \\\"echo | leftabove split | 1b | wincmd j | leftabove split | 4b | wincmd j | 3b | tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n-\tEXPECTED_CMD_06=\"-c \\\"echo | leftabove vertical split | leftabove split | 1b | wincmd j | 3b | wincmd l | 4b | tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n-\tEXPECTED_CMD_07=\"-c \\\"echo | leftabove vertical split | 4b | wincmd l | leftabove split | 1b | wincmd j | 3b | tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n-\tEXPECTED_CMD_08=\"-c \\\"echo | leftabove split | leftabove vertical split | 1b | wincmd l | 3b | wincmd j | 4b | tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n-\tEXPECTED_CMD_09=\"-c \\\"echo | leftabove split | 4b | wincmd j | leftabove vertical split | 1b | wincmd l | 3b | tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n-\tEXPECTED_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\\\"\"\n-\tEXPECTED_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\\\"\"\n-\tEXPECTED_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\\\"\"\n-\tEXPECTED_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\\\"\"\n-\tEXPECTED_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\\\"\"\n-\tEXPECTED_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\\\"\"\n-\tEXPECTED_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\\\"\"\n+\tEXPECTED_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\\\"\"\n+\tEXPECTED_CMD_02=\"-c \\\"echo | leftabove vertical split | silent 1b | wincmd l | silent 3b | set hidden | silent tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n+\tEXPECTED_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\\\"\"\n+\tEXPECTED_CMD_04=\"-c \\\"echo | silent 4b | set hidden | silent bufdo diffthis\\\" -c \\\"tabfirst\\\"\"\n+\tEXPECTED_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\\\"\"\n+\tEXPECTED_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\\\"\"\n+\tEXPECTED_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\\\"\"\n+\tEXPECTED_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\\\"\"\n+\tEXPECTED_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\\\"\"\n+\tEXPECTED_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\\\"\"\n+\tEXPECTED_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\\\"\"\n+\tEXPECTED_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\\\"\"\n+\tEXPECTED_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\\\"\"\n+\tEXPECTED_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\\\"\"\n+\tEXPECTED_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\\\"\"\n+\tEXPECTED_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\\\"\"\n \n \tEXPECTED_TARGET_01=\"MERGED\"\n \tEXPECTED_TARGET_02=\"LOCAL\"\n@@ -635,7 +635,7 @@ run_unit_tests () {\n \tcat >expect <<-\\EOF\n \t-f\n \t-c\n-\techo | leftabove split | leftabove vertical split | 1b | wincmd l | leftabove vertical split | quit | wincmd l | 2b | wincmd j | 3b | tabdo windo diffthis\n+\techo | 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\n \t-c\n \ttabfirst\n \tlo cal\n-- \n2.37.1\n\n"},{"id":"460754","messageId":"CAMP44s3-RG5k4ZkhAFG_9JtbxcyDhkUmeBh0jCH9+Xwyumyu9w@mail.gmail.com","threadId":"58259","inReplyTo":"Yu6zEiknXKFMJUVn@zacax395.localdomain","subject":"Re: [PATCH 0/2] mergetools: vimdiff3: fix regression","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2022-08-06T19:17:36Z","receivedAt":"2022-08-06T19:20:45Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sat, Aug 6, 2022 at 1:29 PM Fernando Ramos <greenfoo@u92.eu> wrote:\n>\n> On 22/08/06 12:53PM, Felipe Contreras wrote:\n> > Two observations though.\n> >\n> > 1. The \"silent 4b\" is ignored, since bufdo makes the last buffer the\n> > current buffer, so if you want a different buffer you have to make the\n> > switch *after* bufdo.\n> >\n>\n> Yes, you are right. For the particular case where there are no windows (only\n> hidden buffers) it does not have any effect. It's presence there comes from\n> the fact that the command generation function works in the most \"generic\" way\n> (ie. producing output that works for all cases: windows, tabs and buffers).\n>\n> In order not to have another special case in the generation logic I left it\n> there, but you are right in that it is not needed (fortunately it also doesn't\n> make any harm :)\n\nThat's not my point. vimdiff3 is essentially the same as vimdiff with:\n\n    git config --global mergetool.vimdiff.layout MERGED\n\nBut the code is written in such a way as to allow:\n\n    git config --global mergetool.vimdiff.layout LOCAL\n\nI don't know why anyone would want to do that, but the code interprets\nthat as the user wanting '1b', which is completely ignored.\n\nIf we are not going to care about these cases, we can just remove all this code:\n\n--- a/mergetools/vimdiff\n+++ b/mergetools/vimdiff\n@@ -254,30 +254,7 @@ gen_cmd_aux () {\n\n        # Step 4:\n        #\n-       # If we reach this point, it means there are no separators and we just\n-       # need to print the command to display the specified buffer\n-\n-       target=$(substring \"$LAYOUT\" \"$start\" \"$(( end - start ))\" |\nsed 's:[ @();|-]::g')\n-\n-       if test \"$target\" = \"LOCAL\"\n-       then\n-               CMD=\"$CMD | 1b\"\n-\n-       elif test \"$target\" = \"BASE\"\n-       then\n-               CMD=\"$CMD | 2b\"\n-\n-       elif test \"$target\" = \"REMOTE\"\n-       then\n-               CMD=\"$CMD | 3b\"\n-\n-       elif test \"$target\" = \"MERGED\"\n-       then\n-               CMD=\"$CMD | 4b\"\n-\n-       else\n-               CMD=\"$CMD | ERROR: >$target<\"\n-       fi\n+       # If we reach this point, it means there are no separators.\n\n        echo \"$CMD\"\n        return\n\n> > I don't see the need for all this complexity for this simple mode, but\n> > anything that actually works is fine by me.\n>\n> ...in fact, back in May I just wanted to add a new \"vimdiff4\" mode and what\n> originally was a 5 lines patch became the current 1000+ lines patch monster\n> after all the (very welcomed, I'm not complaining!) suggestions :)\n\nI understand the need if you want a complex layout, like\n\"MERGED+LOCAL,BASE,REMOTE\", that's very nice, but if you just want\n\"MERGED\", most of the code does nothing, the extra -c \"tabfirst\" isn't\nneeded either.\n\nEither way, adding the silent stuff and \"set hidden\" make vimdiff3\nwork, which is all I care about.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"460756","messageId":"CAMP44s1YegUqFzw8L==q2QOmh-6WCJdXYSXUvz8GPCrXuYRVPQ@mail.gmail.com","threadId":"58259","inReplyTo":"20220806183757.72168-1-greenfoo@u92.eu","subject":"Re: [PATCH] vimdiff: fix 'vimdiff3' behavior (colors + no extra key press)","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2022-08-06T19:39:37Z","receivedAt":"2022-08-06T19:39:54Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sat, Aug 6, 2022 at 1:38 PM Fernando Ramos <greenfoo@u92.eu> wrote:\n>\n> vimdiff3 was introduced in 7c147b77d3 (mergetools: add vimdiff3 mode,\n> 2014-04-20) and then partially broken in 0041797449 (vimdiff: new\n> implementation with layout support, 2022-03-30) in two ways:\n>\n>   - It does not show colors unless the user has \"set hidden\" in his\n>     .vimrc file\n>\n>   - It prompts the user to \"Press ENTER\" every time it runs.\n>\n> This patch fixes both issues by adding \"set hidden\" and \"silent\" to the\n> generated command string that is used to run vim.\n\nAlthough I don't see the point of the extra complexity in the case of\na single window, especially since it doesn't really work for anything\nother than \"MERGED\", this does make vimdiff3 work again for me.\n\nTested-by: Felipe Contreras <felipe.contreras@gmail.com>\n\n-- \nFelipe Contreras\n"},{"id":"460757","messageId":"Yu7byRmn4VtSpyWd@zacax395.localdomain","threadId":"58259","inReplyTo":"CAMP44s3-RG5k4ZkhAFG_9JtbxcyDhkUmeBh0jCH9+Xwyumyu9w@mail.gmail.com","subject":"Re: [PATCH 0/2] mergetools: vimdiff3: fix regression","fromName":"Fernando Ramos","fromEmail":"greenfoo@u92.eu","sentAt":"2022-08-06T21:23:21Z","receivedAt":"2022-08-06T21:23:34Z","isPatch":true,"sender":{"key":"greenfoo@u92.eu","avatar":"https://avatars.githubusercontent.com/u/42691086?v=4"},"body":"On 22/08/06 02:17PM, Felipe Contreras wrote:\n> \n> I don't know why anyone would want to do that, but the code interprets\n> that as the user wanting '1b', which is completely ignored.\n> \n> If we are not going to care about these cases, we can just remove all this code:\n> \n> ...\n> \n\nAh! I see now. You are completely right: it wouldn't make sense for anyone to\nspecify \"layout=LOCAL\" (or REMOTE or BASE), but if he did *it wouldn't work*\n(only works with \"layout=MERGED\").\n\nThat should be fixed. I'll update the patch with a new version to generate this\ncommand string:\n\n     echo | silent 4b | set hidden | let tmp=bufnr('%') | silent bufdo diffthis | exe 'buffer '.tmp\n                                     ^^^^^^^^^^^^^^^^^^                           ^^^^^^^^^^^^^^^^^\n                                     NEW                                          NEW\n\nNotes:\n\n  - This is \"easier\" than moving \"silent 4b\" to the end, due to the way the\n    code is structured.\n\n  - I agree that this is absurdly complex for what we want to achieve with\n    \"vimdiff3\" but let's put it this way: now everything can be achieved with\n    the \"layout\" configuration option, even \"useless\" things such as setting it\n    to \"LOCAL\".\n\n\n> I understand the need if you want a complex layout, like\n> \"MERGED+LOCAL,BASE,REMOTE\", that's very nice, but if you just want\n> \"MERGED\", most of the code does nothing, \n\nWith the fix above that shouldn't be a problem anymore: even if someone\nspecifies \"LOCAL\" it will work, in an absurd way, but it will work :)\n\n\n> the extra -c \"tabfirst\" isn't needed either.\n\nGood catch. I'm also removing it.\n\n\n"},{"id":"460758","messageId":"Yu7ccuxYATwsJ7CF@zacax395.localdomain","threadId":"58259","inReplyTo":"CAMP44s1YegUqFzw8L==q2QOmh-6WCJdXYSXUvz8GPCrXuYRVPQ@mail.gmail.com","subject":"Re: [PATCH] vimdiff: fix 'vimdiff3' behavior (colors + no extra key press)","fromName":"Fernando Ramos","fromEmail":"greenfoo@u92.eu","sentAt":"2022-08-06T21:26:10Z","receivedAt":"2022-08-06T21:26:19Z","isPatch":true,"sender":{"key":"greenfoo@u92.eu","avatar":"https://avatars.githubusercontent.com/u/42691086?v=4"},"body":"I will reply to this message with a new patch that, as discussed in the previous\nemail, will also work with crazy layouts such as \"layout=LOCAL\" :)\n"},{"id":"460759","messageId":"20220806213005.92045-1-greenfoo@u92.eu","threadId":"58259","inReplyTo":"Yu7ccuxYATwsJ7CF@zacax395.localdomain","subject":"[PATCH] vimdiff: fix 'vimdiff3' behavior (colors + no extra key press)","fromName":"Fernando Ramos","fromEmail":"greenfoo@u92.eu","sentAt":"2022-08-06T21:30:05Z","receivedAt":"2022-08-06T21:30:21Z","isPatch":true,"sender":{"key":"greenfoo@u92.eu","avatar":"https://avatars.githubusercontent.com/u/42691086?v=4"},"body":"vimdiff3 was introduced in 7c147b77d3 (mergetools: add vimdiff3 mode,\n2014-04-20) and then partially broken in 0041797449 (vimdiff: new\nimplementation with layout support, 2022-03-30) in two ways:\n\n  - It does not show colors unless the user has \"set hidden\" in his\n    .vimrc file\n\n  - It prompts the user to \"Press ENTER\" every time it runs.\n\nThis patch fixes both issues by adding \"set hidden\" and \"silent\" to the\ngenerated command string that is used to run vim.\n\nCc: Felipe Contreras <felipe.contreras@gmail.com>\nSigned-off-by: Fernando Ramos <greenfoo@u92.eu>\n---\n mergetools/vimdiff | 65 +++++++++++++++++++++++-----------------------\n 1 file changed, 33 insertions(+), 32 deletions(-)\n\ndiff --git a/mergetools/vimdiff b/mergetools/vimdiff\nindex f770b8fe24..238963071a 100644\n--- a/mergetools/vimdiff\n+++ b/mergetools/vimdiff\n@@ -29,8 +29,8 @@\n ################################################################################\n \n debug_print () {\n-\t# Send message to stderr if global variable GIT_MERGETOOL_VIMDIFF is set\n-\t# to \"true\"\n+        # Send message to stderr if global variable GIT_MERGETOOL_VIMDIFF_DEBUG\n+        # is set to \"true\"\n \n \tif test -n \"$GIT_MERGETOOL_VIMDIFF_DEBUG\"\n \tthen\n@@ -261,19 +261,19 @@ gen_cmd_aux () {\n \n \tif test \"$target\" = \"LOCAL\"\n \tthen\n-\t\tCMD=\"$CMD | 1b\"\n+\t\tCMD=\"$CMD | silent 1b\"\n \n \telif test \"$target\" = \"BASE\"\n \tthen\n-\t\tCMD=\"$CMD | 2b\"\n+\t\tCMD=\"$CMD | silent 2b\"\n \n \telif test \"$target\" = \"REMOTE\"\n \tthen\n-\t\tCMD=\"$CMD | 3b\"\n+\t\tCMD=\"$CMD | silent 3b\"\n \n \telif test \"$target\" = \"MERGED\"\n \tthen\n-\t\tCMD=\"$CMD | 4b\"\n+\t\tCMD=\"$CMD | silent 4b\"\n \n \telse\n \t\tCMD=\"$CMD | ERROR: >$target<\"\n@@ -310,7 +310,7 @@ gen_cmd () {\n \t#\n \t#     gen_cmd \"@LOCAL , REMOTE\"\n \t#     |\n-\t#     `-> FINAL_CMD    == \"-c \\\"echo | leftabove vertical split | 1b | wincmd l | 3b | tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n+\t#     `-> FINAL_CMD    == \"-c \\\"echo | leftabove vertical split | silent 1b | wincmd l | silent 3b | set hidden | silent tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n \t#         FINAL_TARGET == \"LOCAL\"\n \n \tLAYOUT=$1\n@@ -341,17 +341,18 @@ gen_cmd () {\n \n \tif echo \"$LAYOUT\" | grep \",\\|/\" >/dev/null\n \tthen\n-\t\tCMD=\"$CMD | tabdo windo diffthis\"\n-\telse\n-\t\tCMD=\"$CMD | bufdo diffthis\"\n-\tfi\n+\t\tCMD=\"$CMD | set hidden | silent tabdo windo diffthis\"\n \n+\t\t# Add an extra \"-c\" option to move to the first tab (notice that we\n+\t\t# can't simply append the command to the previous \"-c\" string as\n+\t\t# explained here: https://github.com/vim/vim/issues/9076\n \n-\t# Add an extra \"-c\" option to move to the first tab (notice that we\n-\t# can't simply append the command to the previous \"-c\" string as\n-\t# explained here: https://github.com/vim/vim/issues/9076\n+\t\tFINAL_CMD=\"-c \\\"$CMD\\\" -c \\\"tabfirst\\\"\"\n+\telse\n+\t\tCMD=\"$CMD | set hidden | let tmp=bufnr('%') | silent bufdo diffthis | exe 'buffer '.tmp\"\n \n-\tFINAL_CMD=\"-c \\\"$CMD\\\" -c \\\"tabfirst\\\"\"\n+\t\tFINAL_CMD=\"-c \\\"$CMD\\\"\"\n+\tfi\n }\n \n \n@@ -555,22 +556,22 @@ run_unit_tests () {\n \tTEST_CASE_15=\"  ((  (LOCAL , BASE , REMOTE) / MERGED))   +(BASE)   , LOCAL+ BASE , REMOTE+ (((LOCAL / BASE / REMOTE)) ,    MERGED   )  \"\n \tTEST_CASE_16=\"LOCAL,BASE,REMOTE / MERGED + BASE,LOCAL + BASE,REMOTE + (LOCAL / BASE / REMOTE),MERGED\"\n \n-\tEXPECTED_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\\\"\"\n-\tEXPECTED_CMD_02=\"-c \\\"echo | leftabove vertical split | 1b | wincmd l | 3b | tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n-\tEXPECTED_CMD_03=\"-c \\\"echo | leftabove vertical split | 1b | wincmd l | leftabove vertical split | 4b | wincmd l | 3b | tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n-\tEXPECTED_CMD_04=\"-c \\\"echo | 4b | bufdo diffthis\\\" -c \\\"tabfirst\\\"\"\n-\tEXPECTED_CMD_05=\"-c \\\"echo | leftabove split | 1b | wincmd j | leftabove split | 4b | wincmd j | 3b | tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n-\tEXPECTED_CMD_06=\"-c \\\"echo | leftabove vertical split | leftabove split | 1b | wincmd j | 3b | wincmd l | 4b | tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n-\tEXPECTED_CMD_07=\"-c \\\"echo | leftabove vertical split | 4b | wincmd l | leftabove split | 1b | wincmd j | 3b | tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n-\tEXPECTED_CMD_08=\"-c \\\"echo | leftabove split | leftabove vertical split | 1b | wincmd l | 3b | wincmd j | 4b | tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n-\tEXPECTED_CMD_09=\"-c \\\"echo | leftabove split | 4b | wincmd j | leftabove vertical split | 1b | wincmd l | 3b | tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n-\tEXPECTED_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\\\"\"\n-\tEXPECTED_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\\\"\"\n-\tEXPECTED_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\\\"\"\n-\tEXPECTED_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\\\"\"\n-\tEXPECTED_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\\\"\"\n-\tEXPECTED_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\\\"\"\n-\tEXPECTED_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\\\"\"\n+\tEXPECTED_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\\\"\"\n+\tEXPECTED_CMD_02=\"-c \\\"echo | leftabove vertical split | silent 1b | wincmd l | silent 3b | set hidden | silent tabdo windo diffthis\\\" -c \\\"tabfirst\\\"\"\n+\tEXPECTED_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\\\"\"\n+\tEXPECTED_CMD_04=\"-c \\\"echo | silent 4b | set hidden | let tmp=bufnr('%') | silent bufdo diffthis | exe 'buffer '.tmp\\\"\"\n+\tEXPECTED_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\\\"\"\n+\tEXPECTED_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\\\"\"\n+\tEXPECTED_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\\\"\"\n+\tEXPECTED_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\\\"\"\n+\tEXPECTED_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\\\"\"\n+\tEXPECTED_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\\\"\"\n+\tEXPECTED_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\\\"\"\n+\tEXPECTED_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\\\"\"\n+\tEXPECTED_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\\\"\"\n+\tEXPECTED_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\\\"\"\n+\tEXPECTED_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\\\"\"\n+\tEXPECTED_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\\\"\"\n \n \tEXPECTED_TARGET_01=\"MERGED\"\n \tEXPECTED_TARGET_02=\"LOCAL\"\n@@ -635,7 +636,7 @@ run_unit_tests () {\n \tcat >expect <<-\\EOF\n \t-f\n \t-c\n-\techo | leftabove split | leftabove vertical split | 1b | wincmd l | leftabove vertical split | quit | wincmd l | 2b | wincmd j | 3b | tabdo windo diffthis\n+\techo | 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\n \t-c\n \ttabfirst\n \tlo cal\n-- \n2.37.1\n\n"},{"id":"460760","messageId":"CAMP44s39TW2nq+H7VHhFmXGQwFPs1-ZDm1XRcoFRzYJL+9RSvA@mail.gmail.com","threadId":"58259","inReplyTo":"Yu7byRmn4VtSpyWd@zacax395.localdomain","subject":"Re: [PATCH 0/2] mergetools: vimdiff3: fix regression","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2022-08-07T00:44:41Z","receivedAt":"2022-08-07T00:44:57Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sat, Aug 6, 2022 at 4:23 PM Fernando Ramos <greenfoo@u92.eu> wrote:\n>\n> On 22/08/06 02:17PM, Felipe Contreras wrote:\n> >\n> > I don't know why anyone would want to do that, but the code interprets\n> > that as the user wanting '1b', which is completely ignored.\n> >\n> > If we are not going to care about these cases, we can just remove all this code:\n> >\n> > ...\n> >\n>\n> Ah! I see now. You are completely right: it wouldn't make sense for anyone to\n> specify \"layout=LOCAL\" (or REMOTE or BASE), but if he did *it wouldn't work*\n> (only works with \"layout=MERGED\").\n>\n> That should be fixed. I'll update the patch with a new version to generate this\n> command string:\n>\n>      echo | silent 4b | set hidden | let tmp=bufnr('%') | silent bufdo diffthis | exe 'buffer '.tmp\n>                                      ^^^^^^^^^^^^^^^^^^                           ^^^^^^^^^^^^^^^^^\n>                                      NEW                                          NEW\n\nThat's not correct: `exe 'buffer '.tmp` would be executed in every\nbuffer. To split the commands you need to do the bufdo in a separate\nexecute command.\n\n> Notes:\n>\n>   - This is \"easier\" than moving \"silent 4b\" to the end, due to the way the\n>     code is structured.\n\nWhich is a clear hint that the code should be restructured.\n\n>   - I agree that this is absurdly complex for what we want to achieve with\n>     \"vimdiff3\" but let's put it this way: now everything can be achieved with\n>     the \"layout\" configuration option, even \"useless\" things such as setting it\n>     to \"LOCAL\".\n\nYes, but even that can be achieved in simpler ways (see the patch below).\n\n> > I understand the need if you want a complex layout, like\n> > \"MERGED+LOCAL,BASE,REMOTE\", that's very nice, but if you just want\n> > \"MERGED\", most of the code does nothing,\n>\n> With the fix above that shouldn't be a problem anymore: even if someone\n> specifies \"LOCAL\" it will work, in an absurd way, but it will work :)\n\nBut the objective isn't to make \"everything\" work, the objective is to\nmake everything the user might reasonably want to work. If some\nunreasonable use case can be supported with a minimal burden to\nmaintenance, sure, support that too.\n\n\"LOCAL\" is not that: it's an unreasonable use case that requires a\nbunch of extra code.\n\nEither way, I think you are resisting too much a reshuffling of the\ncode when it's very clear the single window mode would benefit from\nthat since it doesn't need gen_cmd_aux() at all.\n\nHere's a patch that shuffles the code around and makes it much easier\nto read and maintain (plus it keeps and fixes the support for\nunreasonable use cases like \"LOCAL\"):\n\n(apologies in advance for gmail's possible wrapping)\n\n--- a/mergetools/vimdiff\n+++ b/mergetools/vimdiff\n@@ -251,39 +251,34 @@ gen_cmd_aux () {\n  return\n  fi\n\n+ # Shouldn't happen\n+ echo \"$CMD | echoerr 'BUG'\"\n+}\n\n- # Step 4:\n- #\n- # If we reach this point, it means there are no separators and we just\n- # need to print the command to display the specified buffer\n-\n- target=$(substring \"$LAYOUT\" \"$start\" \"$(( end - start ))\" | sed\n's:[ @();|-]::g')\n+get_buf () {\n+ target=$(echo \"$1\" | sed 's:[ @();|-]::g')\n+ buf=\"1\"\n\n  if test \"$target\" = \"LOCAL\"\n  then\n- CMD=\"$CMD | 1b\"\n+ buf=\"1\"\n\n  elif test \"$target\" = \"BASE\"\n  then\n- CMD=\"$CMD | 2b\"\n+ buf=\"2\"\n\n  elif test \"$target\" = \"REMOTE\"\n  then\n- CMD=\"$CMD | 3b\"\n+ buf=\"3\"\n\n  elif test \"$target\" = \"MERGED\"\n  then\n- CMD=\"$CMD | 4b\"\n-\n- else\n- CMD=\"$CMD | ERROR: >$target<\"\n+ buf=\"4\"\n  fi\n\n- echo \"$CMD\"\n- return\n+ echo \"$buf\"\n }\n\n-\n gen_cmd () {\n  # This function returns (in global variable FINAL_CMD) the string that\n  # you can use when invoking \"vim\" (as shown next) to obtain a given\n@@ -315,6 +310,14 @@ gen_cmd () {\n\n  LAYOUT=$1\n\n+ # A single window is handled specially\n+\n+ if ! echo \"$LAYOUT\" | grep \",\\|/\" >/dev/null\n+ then\n+ buf=$(get_buf \"$LAYOUT\")\n+ FINAL_CMD=\"-c \\\"set hidden | silent bufdo diffthis\\\" -c \\\"silent ${buf}b\\\"\"\n+ return\n+ fi\n\n  # Search for a \"@\" in one of the files identifiers (\"LOCAL\", \"BASE\",\n  # \"REMOTE\", \"MERGED\"). If not found, use \"MERGE\" as the default file\n@@ -335,17 +338,7 @@ gen_cmd () {\n\n  CMD=$(gen_cmd_aux \"$LAYOUT\")\n\n-\n- # Adjust the just obtained script depending on whether more than one\n- # windows are visible or not\n-\n- if echo \"$LAYOUT\" | grep \",\\|/\" >/dev/null\n- then\n- CMD=\"$CMD | tabdo windo diffthis\"\n- else\n- CMD=\"$CMD | bufdo diffthis\"\n- fi\n-\n+ CMD=\"$CMD | tabdo windo diffthis\"\n\n  # Add an extra \"-c\" option to move to the first tab (notice that we\n  # can't simply append the command to the previous \"-c\" string as\n\n-- \nFelipe Contreras\n"},{"id":"460761","messageId":"CAMP44s0QjKzt7VNFPWAi9RKThP2H2VP=4hYiDP4X9-CzrmYrew@mail.gmail.com","threadId":"58259","inReplyTo":"20220806213005.92045-1-greenfoo@u92.eu","subject":"Re: [PATCH] vimdiff: fix 'vimdiff3' behavior (colors + no extra key press)","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2022-08-07T00:55:07Z","receivedAt":"2022-08-07T00:55:23Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sat, Aug 6, 2022 at 4:30 PM Fernando Ramos <greenfoo@u92.eu> wrote:\n\n>\n> +               # Add an extra \"-c\" option to move to the first tab (notice that we\n> +               # can't simply append the command to the previous \"-c\" string as\n> +               # explained here: https://github.com/vim/vim/issues/9076\n>\n> -       # Add an extra \"-c\" option to move to the first tab (notice that we\n> -       # can't simply append the command to the previous \"-c\" string as\n> -       # explained here: https://github.com/vim/vim/issues/9076\n> +               FINAL_CMD=\"-c \\\"$CMD\\\" -c \\\"tabfirst\\\"\"\n> +       else\n> +               CMD=\"$CMD | set hidden | let tmp=bufnr('%') | silent bufdo diffthis | exe 'buffer '.tmp\"\n\nNAK: This runs `exe 'buffer #'` in every buffer. While switching to\nthe desired buffer 3 extra times isn't a problem, it's also not\ncorrect.\n\nYou need something like:\n\n    silent exe 'bufdo diffthis' | exe 'buffer '.tmp\"\n\nBut at this point it seems like we are working around the current\norganization of the code, when we could simply reorganize it.\n\nCheers.\n\n-- \nFelipe Contreras\n"}]}