# [PATCH 0/2] difftool: fix user-defined Beyond Compare setups 4 messages from 2020-11-11 to 2020-11-11. Participants: Johannes Schindelin via GitGitGadget, Junio C Hamano. Thread: https://gitlist.dev/t/54623 ## Johannes Schindelin via GitGitGadget, 2020-11-11 20:33 Subject: [PATCH 0/2] difftool: fix user-defined Beyond Compare setups Message-ID: URL: https://gitlist.dev/e/pull.787.git.1605126798.gitgitgadget%40gmail.com ``` Git v2.29.0 includes patches that try to support Beyond Compare better by default. However, as reported in https://github.com/git-for-windows/git/issues/2893, they broke user-defined setups that use bc4 as the name for the difftool. This patch series fixes that and is based on pd/mergetool-nvimdiff. Johannes Schindelin (2): mergetools/bc: add `bc4` to the alias list for Beyond Compare mergetool: avoid letting `list_tool_variants` break user-defined setups git-mergetool--lib.sh | 4 ++++ mergetools/bc | 1 + 2 files changed, 5 insertions(+) base-commit: 11868978c7c80d3c29071b29e7964e3d62523819 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-787%2Fdscho%2Ffix-beyond-compare-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-787/dscho/fix-beyond-compare-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/787 -- gitgitgadget ``` ## Johannes Schindelin via GitGitGadget, 2020-11-11 20:33 Subject: [PATCH 2/2] mergetool: avoid letting `list_tool_variants` break user-defined setups Message-ID: URL: https://gitlist.dev/e/c157c07d5d4917e6eef6717894f61c54cbc9af22.1605126798.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Johannes Schindelin In 83bbf9b92ea8 (mergetool--lib: improve support for vimdiff-style tool variants, 2020-07-29), we introduced a `list_tool_variants` function in the spirit of Postel's Law: be lenient in what you accept as input. In this particular instance, we wanted to allow not only `bc` but also `bc3` as name for the Beyond Compare tool. However, what this patch overlooked is that it is totally allowed for users to override the defaults in `mergetools/`. But now that we strip off trailing digits, the name that the user gave the tool might not actually be in the list produced by `list_tool_variants`. So let's do the same as for the `diff_cmd` and the `merge_cmd`: override it with the trivial version in case a user-defined setup was detected. Signed-off-by: Johannes Schindelin --- git-mergetool--lib.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index 2defef28cd..7225abd811 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -138,6 +138,10 @@ setup_user_tool () { merge_cmd () { ( eval $merge_tool_cmd ) } + + list_tool_variants () { + echo "$tool" + } } setup_tool () { -- gitgitgadget ``` ## Johannes Schindelin via GitGitGadget, 2020-11-11 20:33 Subject: [PATCH 1/2] mergetools/bc: add `bc4` to the alias list for Beyond Compare Message-ID: <0e2b11ae9851525d24e5f4e2bb0919bf4f0fd788.1605126798.git.gitgitgadget@gmail.com> URL: https://gitlist.dev/e/0e2b11ae9851525d24e5f4e2bb0919bf4f0fd788.1605126798.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Johannes Schindelin As of 83bbf9b92ea8 (mergetool--lib: improve support for vimdiff-style tool variants, 2020-07-29), we already list `bc` and `bc3` as aliases for that mergetool/difftool. However, the current Beyond Compare version is _4_, therefore the `bc4` alias is missing from that list. Most notably, this is the root cause of the breakage reported in https://github.com/git-for-windows/git/issues/2893 where a well-configured `bc4` difftool stopped working as of v2.29.0: `setup_tool` would notice that after stripping off the trailing digit, it finds a match in `mergetools/` (the `bc` file), source it, and then the alias would not match the list offered by the `list_tool_variants` function, and simply exit without doing anything, but pretending success. Signed-off-by: Johannes Schindelin --- mergetools/bc | 1 + 1 file changed, 1 insertion(+) diff --git a/mergetools/bc b/mergetools/bc index a89086ee72..26c19d46a5 100644 --- a/mergetools/bc +++ b/mergetools/bc @@ -25,4 +25,5 @@ translate_merge_tool_path() { list_tool_variants () { echo bc echo bc3 + echo bc4 } -- gitgitgadget ``` ## Junio C Hamano, 2020-11-11 20:59 Subject: Re: [PATCH 2/2] mergetool: avoid letting `list_tool_variants` break user-defined setups Message-ID: URL: https://gitlist.dev/e/xmqq1rgz4nk3.fsf%40gitster.c.googlers.com In-Reply-To: ``` "Johannes Schindelin via GitGitGadget" writes: > From: Johannes Schindelin > > In 83bbf9b92ea8 (mergetool--lib: improve support for vimdiff-style tool > variants, 2020-07-29), we introduced a `list_tool_variants` function > in the spirit of Postel's Law: be lenient in what you accept as input. > In this particular instance, we wanted to allow not only `bc` but also > `bc3` as name for the Beyond Compare tool. > > However, what this patch overlooked is that it is totally allowed for > users to override the defaults in `mergetools/`. But now that we strip > off trailing digits, the name that the user gave the tool might not > actually be in the list produced by `list_tool_variants`. > > So let's do the same as for the `diff_cmd` and the `merge_cmd`: override > it with the trivial version in case a user-defined setup was detected. Makes sense. Will queue. Thanks. > > Signed-off-by: Johannes Schindelin > --- > git-mergetool--lib.sh | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh > index 2defef28cd..7225abd811 100644 > --- a/git-mergetool--lib.sh > +++ b/git-mergetool--lib.sh > @@ -138,6 +138,10 @@ setup_user_tool () { > merge_cmd () { > ( eval $merge_tool_cmd ) > } > + > + list_tool_variants () { > + echo "$tool" > + } > } > > setup_tool () { ```