{"thread":{"id":"54623","subject":"[PATCH 0/2] difftool: fix user-defined Beyond Compare setups","startedAt":"2020-11-11T20:33:23Z","lastAt":"2020-11-11T20:59:47Z","messageCount":4,"participants":["Johannes Schindelin via GitGitGadget","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"409714","messageId":"pull.787.git.1605126798.gitgitgadget@gmail.com","threadId":"54623","inReplyTo":null,"subject":"[PATCH 0/2] difftool: fix user-defined Beyond Compare setups","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-11-11T20:33:16Z","receivedAt":"2020-11-11T20:33:23Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Git v2.29.0 includes patches that try to support Beyond Compare better by\ndefault. However, as reported in \nhttps://github.com/git-for-windows/git/issues/2893, they broke user-defined\nsetups that use bc4 as the name for the difftool.\n\nThis patch series fixes that and is based on pd/mergetool-nvimdiff.\n\nJohannes Schindelin (2):\n mergetools/bc: add `bc4` to the alias list for Beyond Compare\n mergetool: avoid letting `list_tool_variants` break user-defined\n setups\n\n git-mergetool--lib.sh | 4 ++++\n mergetools/bc | 1 +\n 2 files changed, 5 insertions(+)\n\n\nbase-commit: 11868978c7c80d3c29071b29e7964e3d62523819\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-787%2Fdscho%2Ffix-beyond-compare-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-787/dscho/fix-beyond-compare-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/787\n-- \ngitgitgadget\n"},{"id":"409715","messageId":"c157c07d5d4917e6eef6717894f61c54cbc9af22.1605126798.git.gitgitgadget@gmail.com","threadId":"54623","inReplyTo":"pull.787.git.1605126798.gitgitgadget@gmail.com","subject":"[PATCH 2/2] mergetool: avoid letting `list_tool_variants` break user-defined setups","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-11-11T20:33:18Z","receivedAt":"2020-11-11T20:33:24Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIn 83bbf9b92ea8 (mergetool--lib: improve support for vimdiff-style tool\nvariants, 2020-07-29), we introduced a `list_tool_variants` function\nin the spirit of Postel's Law: be lenient in what you accept as input.\nIn this particular instance, we wanted to allow not only `bc` but also\n`bc3` as name for the Beyond Compare tool.\n\nHowever, what this patch overlooked is that it is totally allowed for\nusers to override the defaults in `mergetools/`. But now that we strip\noff trailing digits, the name that the user gave the tool might not\nactually be in the list produced by `list_tool_variants`.\n\nSo let's do the same as for the `diff_cmd` and the `merge_cmd`: override\nit with the trivial version in case a user-defined setup was detected.\n\nSigned-off-by: Johannes Schindelin \n---\n git-mergetool--lib.sh | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh\nindex 2defef28cd..7225abd811 100644\n--- a/git-mergetool--lib.sh\n+++ b/git-mergetool--lib.sh\n@@ -138,6 +138,10 @@ setup_user_tool () {\n \tmerge_cmd () {\n \t\t( eval $merge_tool_cmd )\n \t}\n+\n+\tlist_tool_variants () {\n+\t\techo \"$tool\"\n+\t}\n }\n \n setup_tool () {\n-- \ngitgitgadget\n"},{"id":"409716","messageId":"0e2b11ae9851525d24e5f4e2bb0919bf4f0fd788.1605126798.git.gitgitgadget@gmail.com","threadId":"54623","inReplyTo":"pull.787.git.1605126798.gitgitgadget@gmail.com","subject":"[PATCH 1/2] mergetools/bc: add `bc4` to the alias list for Beyond Compare","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-11-11T20:33:17Z","receivedAt":"2020-11-11T20:33:25Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nAs of 83bbf9b92ea8 (mergetool--lib: improve support for vimdiff-style\ntool variants, 2020-07-29), we already list `bc` and `bc3` as aliases\nfor that mergetool/difftool.\n\nHowever, the current Beyond Compare version is _4_, therefore the `bc4`\nalias is missing from that list.\n\nMost notably, this is the root cause of the breakage reported in\nhttps://github.com/git-for-windows/git/issues/2893 where a\nwell-configured `bc4` difftool stopped working as of v2.29.0:\n`setup_tool` would notice that after stripping off the trailing digit,\nit finds a match in `mergetools/` (the `bc` file), source it, and then\nthe alias would not match the list offered by the `list_tool_variants`\nfunction, and simply exit without doing anything, but pretending\nsuccess.\n\nSigned-off-by: Johannes Schindelin \n---\n mergetools/bc | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/mergetools/bc b/mergetools/bc\nindex a89086ee72..26c19d46a5 100644\n--- a/mergetools/bc\n+++ b/mergetools/bc\n@@ -25,4 +25,5 @@ translate_merge_tool_path() {\n list_tool_variants () {\n \techo bc\n \techo bc3\n+\techo bc4\n }\n-- \ngitgitgadget\n\n"},{"id":"409721","messageId":"xmqq1rgz4nk3.fsf@gitster.c.googlers.com","threadId":"54623","inReplyTo":"c157c07d5d4917e6eef6717894f61c54cbc9af22.1605126798.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] mergetool: avoid letting `list_tool_variants` break user-defined setups","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-11-11T20:59:40Z","receivedAt":"2020-11-11T20:59:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> From: Johannes Schindelin \n>\n> In 83bbf9b92ea8 (mergetool--lib: improve support for vimdiff-style tool\n> variants, 2020-07-29), we introduced a `list_tool_variants` function\n> in the spirit of Postel's Law: be lenient in what you accept as input.\n> In this particular instance, we wanted to allow not only `bc` but also\n> `bc3` as name for the Beyond Compare tool.\n>\n> However, what this patch overlooked is that it is totally allowed for\n> users to override the defaults in `mergetools/`. But now that we strip\n> off trailing digits, the name that the user gave the tool might not\n> actually be in the list produced by `list_tool_variants`.\n>\n> So let's do the same as for the `diff_cmd` and the `merge_cmd`: override\n> it with the trivial version in case a user-defined setup was detected.\n\nMakes sense. Will queue. Thanks.\n\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> git-mergetool--lib.sh | 4 ++++\n> 1 file changed, 4 insertions(+)\n>\n> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh\n> index 2defef28cd..7225abd811 100644\n> --- a/git-mergetool--lib.sh\n> +++ b/git-mergetool--lib.sh\n> @@ -138,6 +138,10 @@ setup_user_tool () {\n> \tmerge_cmd () {\n> \t\t( eval $merge_tool_cmd )\n> \t}\n> +\n> +\tlist_tool_variants () {\n> +\t\techo \"$tool\"\n> +\t}\n> }\n> \n> setup_tool () {\n"}]}