Skip to contentSkip to conversation

difftool: fix user-defined Beyond Compare setups

PATCH In review Johannes Schindelin via GitGitGadget 2 participants 4 messages Nov 11, 2020

lore
Share & tools
lore
#0 lore raw

cover difftool: fix user-defined Beyond Compare setups

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

Signature · 1 line
gitgitgadget
#2 lore raw

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

From: Johannes Schindelin <[email protected]>

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.

---
 git-mergetool--lib.sh | 4 ++++
 1 file changed, 4 insertions(+)
git-mergetool--lib.sh +4 -0
index 2defef28cd..7225abd811 100644
@@ -138,6 +138,10 @@ setup_user_tool () {
 	merge_cmd () {
 		( eval $merge_tool_cmd )
 	}
+
+	list_tool_variants () {
+		echo "$tool"
+	}
 }
 
 setup_tool () {
Signature · 1 line
gitgitgadget
#1 lore raw

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

In reply to Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

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.

---
 mergetools/bc | 1 +
 1 file changed, 1 insertion(+)
mergetools/bc +1 -0
index a89086ee72..26c19d46a5 100644
@@ -25,4 +25,5 @@ translate_merge_tool_path() {
 list_tool_variants () {
 	echo bc
 	echo bc3
+	echo bc4
 }
Signature · 1 line
gitgitgadget
#3 lore raw

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

In reply to Johannes Schindelin via GitGitGadget

"Johannes Schindelin via GitGitGadget" <[email protected]>

Quoted 8 lines · writes:

From: Johannes Schindelin <[email protected]>

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.

Quoted 7 lines

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.

Quoted 21 lines
---
 git-mergetool--lib.sh | 4 ++++
 1 file changed, 4 insertions(+)
git-mergetool--lib.sh +4 -0
index 2defef28cd..7225abd811 100644
@@ -138,6 +138,10 @@ setup_user_tool () {
 	merge_cmd () {
 		( eval $merge_tool_cmd )
 	}
+
+	list_tool_variants () {
+		echo "$tool"
+	}
 }
 
 setup_tool () {