threads / patch / 54623

patch, 2 partsdifftool: fix user-defined Beyond Compare setups

Subject: [PATCH 0/2] difftool: fix user-defined Beyond Compare setups

## tl;dr

4 messages between Nov 11, 2020 and Nov 11, 2020. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Johannes Schindelin via GitGitGadget· Nov 11, 2020, 20:33 UTC · lore

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· Nov 11, 2020, 20:33 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH 2/2] mergetool: avoid letting `list_tool_variants` break user-defined setups

From: Johannes Schindelin <johannes.schindelin@gmx.de>

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 <johannes.schindelin@gmx.de>
---
 git-mergetool--lib.sh | 4 ++++
 1 file changed, 4 insertions(+)

Show changes to git-mergetool--lib.sh +4 −0

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

Junio C Hamano· Nov 11, 2020, 20:59 UTC · re: Johannes Schindelin via GitGitGadget · lore

Re: [PATCH 2/2] mergetool: avoid letting `list_tool_variants` break user-defined setups

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 15 quoted lines

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> 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.

Show 21 quoted lines

>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  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 () {

Johannes Schindelin via GitGitGadget· Nov 11, 2020, 20:33 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH 1/2] mergetools/bc: add `bc4` to the alias list for Beyond Compare

From: Johannes Schindelin <johannes.schindelin@gmx.de>

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 <johannes.schindelin@gmx.de>
---
 mergetools/bc | 1 +
 1 file changed, 1 insertion(+)

Show changes to mergetools/bc +1 −0

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