* [PATCH 0/2] difftool: fix user-defined Beyond Compare setups
@ 2020-11-11 20:33 Johannes Schindelin via GitGitGadget
2020-11-11 20:33 ` [PATCH 1/2] mergetools/bc: add `bc4` to the alias list for Beyond Compare Johannes Schindelin via GitGitGadget
2020-11-11 20:33 ` [PATCH 2/2] mergetool: avoid letting `list_tool_variants` break user-defined setups Johannes Schindelin via GitGitGadget
0 siblings, 2 replies; 4+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2020-11-11 20:33 UTC (permalink / raw)
To: git; +Cc: pudinha, Johannes Schindelin
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/2] mergetools/bc: add `bc4` to the alias list for Beyond Compare
2020-11-11 20:33 [PATCH 0/2] difftool: fix user-defined Beyond Compare setups Johannes Schindelin via GitGitGadget
@ 2020-11-11 20:33 ` Johannes Schindelin via GitGitGadget
2020-11-11 20:33 ` [PATCH 2/2] mergetool: avoid letting `list_tool_variants` break user-defined setups Johannes Schindelin via GitGitGadget
1 sibling, 0 replies; 4+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2020-11-11 20:33 UTC (permalink / raw)
To: git; +Cc: pudinha, Johannes Schindelin, Johannes Schindelin
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(+)
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] mergetool: avoid letting `list_tool_variants` break user-defined setups
2020-11-11 20:33 [PATCH 0/2] difftool: fix user-defined Beyond Compare setups Johannes Schindelin via GitGitGadget
2020-11-11 20:33 ` [PATCH 1/2] mergetools/bc: add `bc4` to the alias list for Beyond Compare Johannes Schindelin via GitGitGadget
@ 2020-11-11 20:33 ` Johannes Schindelin via GitGitGadget
2020-11-11 20:59 ` Junio C Hamano
1 sibling, 1 reply; 4+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2020-11-11 20:33 UTC (permalink / raw)
To: git; +Cc: pudinha, Johannes Schindelin, Johannes Schindelin
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(+)
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] mergetool: avoid letting `list_tool_variants` break user-defined setups
2020-11-11 20:33 ` [PATCH 2/2] mergetool: avoid letting `list_tool_variants` break user-defined setups Johannes Schindelin via GitGitGadget
@ 2020-11-11 20:59 ` Junio C Hamano
0 siblings, 0 replies; 4+ messages in thread
From: Junio C Hamano @ 2020-11-11 20:59 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget; +Cc: git, pudinha, Johannes Schindelin
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> 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.
>
> 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 () {
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-11-11 20:59 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-11 20:33 [PATCH 0/2] difftool: fix user-defined Beyond Compare setups Johannes Schindelin via GitGitGadget
2020-11-11 20:33 ` [PATCH 1/2] mergetools/bc: add `bc4` to the alias list for Beyond Compare Johannes Schindelin via GitGitGadget
2020-11-11 20:33 ` [PATCH 2/2] mergetool: avoid letting `list_tool_variants` break user-defined setups Johannes Schindelin via GitGitGadget
2020-11-11 20:59 ` Junio C Hamano
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).