diff mbox series

[v2] git gui: add directly calling merge tool from gitconfig

Message ID pull.1773.v2.git.1724833917245.gitgitgadget@gmail.com (mailing list archive)
State Superseded
Headers show
Series [v2] git gui: add directly calling merge tool from gitconfig | expand

Commit Message

tobias.boesch@miele.com Aug. 28, 2024, 8:31 a.m. UTC
From: deboeto <tobias.boesch@miele.com>

git gui can open a merge tool when conflicts are
detected (Right click in the diff of the file with
conflicts).
The merge tools that are allowed to
use are hard coded into git gui.

If one wants to add a new merge tool it has to be
added to git gui through a source code change.
This is not convenient in comparison to how it
works in git (without gui).

git itself has configuration options for a merge tools
path and command in the git config.
New merge tools can be set up there without a
source code change.

Those options are used only by pure git in
contrast to git gui. git calls the configured
merge tools directly from the config while git
Gui doesn't.

With this change git gui can call merge tools
configured in the gitconfig directly without a
change in git gui source code.
It needs a configured merge.tool and a configured
mergetool.cmd config entry.

gitconfig example:
[merge]
	tool = vscode
[mergetool "vscode"]
	path = the/path/to/Code.exe
	cmd = \"Code.exe\" --wait --merge \"$LOCAL\" \"$REMOTE\" \"$BASE\" \"$MERGED\"

Without the mergetool.cmd configuration and an
unsupported merge.tool entry, git gui behaves
mainly as before this change and informs the user
about an unsupported merge tool, but now also
shows a hint to add a config entry for the tool
in gitconfig.

If a wrong mergetool.cmd is configured by accident
it is beeing handled by git gui already. In this
case git gui informs the user that the merge tool
couldn't be opened. This behavior is preserved by
this change and should not change.

Beyond compare 3 and Visual Studio code were
tested as manually configured merge tools.

Signed-off-by: Tobias Boesch <tobias.boesch@miele.com>
---
    git gui: add directly calling merge tool from gitconfig
    
    cc: Johannes Sixt j6t@kdbg.org
    
    Changes since v1:
    
     * Used existing option mergetool.cmd in gitconfig to trigger the direct
       call of the merge tool configured in the config instead adding a new
       option mergeToolFromConfig
     * Removed assignment of merge tool path to a variable and reused the
       already existing one: merget_tool_path
     * Changed formatting of the commit message
     * Added more context and an examples to the commit message

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1773%2FToBoMi%2Fadd_merge_tool_from_config_file-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1773/ToBoMi/add_merge_tool_from_config_file-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1773

Range-diff vs v1:

 1:  59e8f454a70 ! 1:  e77d6dec6c5 git gui: add directly calling merge tool from gitconfig
     @@ Metadata
       ## Commit message ##
          git gui: add directly calling merge tool from gitconfig
      
     -    * git Gui can open a merge tool when conflicts are
     -        detected. The merge tools that are allowed to
     -        call have to be hard coded into git Gui
     -        althgough there are configuration options for
     -        merge tools git in the git config. Git calls
     -        the configured merge tools directly from the
     -        config while git Gui doesn't.
     -    * git Gui can now call the tool configured in the
     -        gitconfig directly.
     -    * Can be enabled through setting
     -        gui.mergeToolFromConfig
     -    * Disabled by default, since option is most likely
     -        never set
     -    * bc3 and vscode tested
     -
     -    Signed-off-by: deboeto <tobias.boesch@miele.com>
     -
     - ## Documentation/config/gui.txt ##
     -@@ Documentation/config/gui.txt: gui.blamehistoryctx::
     - 	linkgit:gitk[1] for the selected commit, when the `Show History
     - 	Context` menu item is invoked from 'git gui blame'. If this
     - 	variable is set to zero, the whole history is shown.
     -+
     -+gui.mergeToolFromConfig::
     -+	If true, allow to call the merge tool configured in gitconfig
     -+	in git gui directly.
     - \ No newline at end of file
     +    git gui can open a merge tool when conflicts are
     +    detected (Right click in the diff of the file with
     +    conflicts).
     +    The merge tools that are allowed to
     +    use are hard coded into git gui.
     +
     +    If one wants to add a new merge tool it has to be
     +    added to git gui through a source code change.
     +    This is not convenient in comparison to how it
     +    works in git (without gui).
     +
     +    git itself has configuration options for a merge tools
     +    path and command in the git config.
     +    New merge tools can be set up there without a
     +    source code change.
     +
     +    Those options are used only by pure git in
     +    contrast to git gui. git calls the configured
     +    merge tools directly from the config while git
     +    Gui doesn't.
     +
     +    With this change git gui can call merge tools
     +    configured in the gitconfig directly without a
     +    change in git gui source code.
     +    It needs a configured merge.tool and a configured
     +    mergetool.cmd config entry.
     +
     +    gitconfig example:
     +    [merge]
     +            tool = vscode
     +    [mergetool "vscode"]
     +            path = the/path/to/Code.exe
     +            cmd = \"Code.exe\" --wait --merge \"$LOCAL\" \"$REMOTE\" \"$BASE\" \"$MERGED\"
     +
     +    Without the mergetool.cmd configuration and an
     +    unsupported merge.tool entry, git gui behaves
     +    mainly as before this change and informs the user
     +    about an unsupported merge tool, but now also
     +    shows a hint to add a config entry for the tool
     +    in gitconfig.
     +
     +    If a wrong mergetool.cmd is configured by accident
     +    it is beeing handled by git gui already. In this
     +    case git gui informs the user that the merge tool
     +    couldn't be opened. This behavior is preserved by
     +    this change and should not change.
     +
     +    Beyond compare 3 and Visual Studio code were
     +    tested as manually configured merge tools.
     +
     +    Signed-off-by: Tobias Boesch <tobias.boesch@miele.com>
      
       ## git-gui/lib/mergetool.tcl ##
      @@ git-gui/lib/mergetool.tcl: proc merge_resolve_tool2 {} {
     @@ git-gui/lib/mergetool.tcl: proc merge_resolve_tool2 {} {
       	default {
      -		error_popup [mc "Unsupported merge tool '%s'" $tool]
      -		return
     -+		if {[is_config_true gui.mergetoolfromconfig]} {
     -+			set path [get_config mergetool.$tool.path]
     -+			set cmdline_config [get_config mergetool.$tool.cmd]
     -+			set cmdline_substituted [subst -nobackslashes -nocommands $cmdline_config]
     -+			set cmdline [lreplace $cmdline_substituted 0 0 $path]
     ++		set tool_cmd [get_config mergetool.$tool.cmd]
     ++		if {$tool_cmd ne {}} {
     ++			set tool_cmd_file_vars_resolved [subst -nobackslashes -nocommands $tool_cmd]
     ++			set cmdline [lreplace $tool_cmd_file_vars_resolved 0 0 $merge_tool_path]
      +		} else {
     -+			error_popup [mc "Unsupported merge tool '%s'" $tool]
     ++			error_popup [mc "Unsupported merge tool '%s'. Is the tool command and path configured properly in gitconfig?" $tool]
      +			return
      +		}
       	}


 git-gui/lib/mergetool.tcl | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)


base-commit: 159f2d50e75c17382c9f4eb7cbda671a6fa612d1

Comments

Junio C Hamano Aug. 28, 2024, 5:08 p.m. UTC | #1
"ToBoMi via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: deboeto <tobias.boesch@miele.com>

Use the same ident (human readable name plus e-mail address) you
have on your Signed-off-by: line below for this "From: " in-body
header.

> git gui can open a merge tool when conflicts are
> detected (Right click in the diff of the file with
> conflicts).
> The merge tools that are allowed to
> use are hard coded into git gui.
>
> If one wants to add a new merge tool it has to be
> added to git gui through a source code change.
> This is not convenient in comparison to how it
> works in git (without gui).
>
> git itself has configuration options for a merge tools
> path and command in the git config.
> New merge tools can be set up there without a
> source code change.

Even if you configure an unknown tool, it would not get any benefit
from what git-{diff,merge}tool--lib.sh gives the known diff/merge
backends, would it?  Instead of a more thorough support for known
tools done in setup_tool(), an unknown tool would be handled by
setup_user_tool() in git-mergetool-lib.sh which gives somewhat
degraded support.

So "can be set up without" may be true, but giving an impression
that a tool that is set up like so would work just like a known tool
is misleading.

By the way, we do ask contributors to avoid overly long lines,
50-col limt is a bit overly short and makes the resulting text
harder to read than necessary.

> Those options are used only by pure git in
> contrast to git gui. git calls the configured
> merge tools directly from the config while git
> Gui doesn't.
>
> With this change git gui can call merge tools
> configured in the gitconfig directly without a
> change in git gui source code.
> It needs a configured merge.tool and a configured
> mergetool.cmd config entry.

OK.

> gitconfig example:
> [merge]
> 	tool = vscode
> [mergetool "vscode"]
> 	path = the/path/to/Code.exe
> 	cmd = \"Code.exe\" --wait --merge \"$LOCAL\" \"$REMOTE\" \"$BASE\" \"$MERGED\"
>
> Without the mergetool.cmd configuration and an
> unsupported merge.tool entry, git gui behaves
> mainly as before this change and informs the user
> about an unsupported merge tool, but now also
> shows a hint to add a config entry for the tool
> in gitconfig.
>
> If a wrong mergetool.cmd is configured by accident
> it is beeing handled by git gui already. In this

"is beeing" -> "is being", but "it gets handled by Git GUI already"
should be sufficient.

> case git gui informs the user that the merge tool
> couldn't be opened. This behavior is preserved by
> this change and should not change.
>
> Beyond compare 3 and Visual Studio code were
> tested as manually configured merge tools.

Quote proper nouns for readability?  E.g. 

    "Beyond Compare 3" and "Visual Studio Code" were ...

Thanks.
Johannes Sixt Aug. 31, 2024, 1:51 p.m. UTC | #2
Am 28.08.24 um 10:31 schrieb ToBoMi via GitGitGadget:
> From: deboeto <tobias.boesch@miele.com>
> 
> git gui can open a merge tool when conflicts are
> detected (Right click in the diff of the file with
> conflicts).
> The merge tools that are allowed to
> use are hard coded into git gui.
> 
> If one wants to add a new merge tool it has to be
> added to git gui through a source code change.
> This is not convenient in comparison to how it
> works in git (without gui).
> 
> git itself has configuration options for a merge tools
> path and command in the git config.
> New merge tools can be set up there without a
> source code change.
> 
> Those options are used only by pure git in
> contrast to git gui. git calls the configured
> merge tools directly from the config while git
> Gui doesn't.
> 
> With this change git gui can call merge tools
> configured in the gitconfig directly without a
> change in git gui source code.
> It needs a configured merge.tool and a configured
> mergetool.cmd config entry.

OK.

> gitconfig example:
> [merge]
> 	tool = vscode
> [mergetool "vscode"]
> 	path = the/path/to/Code.exe
> 	cmd = \"Code.exe\" --wait --merge \"$LOCAL\" \"$REMOTE\" \"$BASE\" \"$MERGED\"

I found it annoying that I had to configure .path in addition to .cmd.
Typically, you would put the correct path into the .cmd configuration.
In fact, `git mergetool` works without .path and fails when .cmd does
not contain the correct path.

> Without the mergetool.cmd configuration and an
> unsupported merge.tool entry, git gui behaves
> mainly as before this change and informs the user
> about an unsupported merge tool, but now also
> shows a hint to add a config entry for the tool
> in gitconfig.

Good.

While testing I configured meld incorrectly once and got no feedback
whatsoever, but I would not attribute this to this patch.

There is no such thing called "gitconfig". Just strike "in gitconfig".

> If a wrong mergetool.cmd is configured by accident
> it is beeing handled by git gui already. In this
> case git gui informs the user that the merge tool
> couldn't be opened. This behavior is preserved by
> this change and should not change.

Good.

> 
> Beyond compare 3 and Visual Studio code were
> tested as manually configured merge tools.
> 
> Signed-off-by: Tobias Boesch <tobias.boesch@miele.com>

You updated this line, but not the From: line. Would you mind
configuring your user.name and then `git commit --amend --reset-author`?

>  git-gui/lib/mergetool.tcl | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/git-gui/lib/mergetool.tcl b/git-gui/lib/mergetool.tcl
> index e688b016ef6..4c4e8f47bb0 100644
> --- a/git-gui/lib/mergetool.tcl
> +++ b/git-gui/lib/mergetool.tcl
> @@ -272,8 +272,14 @@ proc merge_resolve_tool2 {} {
>  		}
>  	}
>  	default {
> -		error_popup [mc "Unsupported merge tool '%s'" $tool]
> -		return
> +		set tool_cmd [get_config mergetool.$tool.cmd]
> +		if {$tool_cmd ne {}} {
> +			set tool_cmd_file_vars_resolved [subst -nobackslashes -nocommands $tool_cmd]

I just learnt that a string value containing double-quotes is broken
into a list in the expected way (keeps quoted parts together as a single
element). However, this form of substitution replaces variable values
with arbitrary text without taking into account that the original string
is actually a list. Should we not break the string into a list first,
and apply the substitution on the list elements?

If there is a straight-forward way to do this (say, an obvious two-liner
at most), we should do it. Otherwise, I can live with this solution for
now because it requires file names with double-quotes to break the
expected list nature.

There is another thing, though, that I would not want to take as
lightly: The -nocommands modifier of `subst` does not live up to its
promises, and it is even the documented behavior: command substitutions
in array indexes are still executed. Consider this configuration:

[merge]
        tool = evil
[mergetool "evil"]
        cmd = meld \"$REMOTE([exit])\"

Guess what happens when I run the merge tool? It exits Git GUI!

I suggest to reject any configuration that contains an opening bracket
'[' or anything else that introduces a command execution.

> +			set cmdline [lreplace $tool_cmd_file_vars_resolved 0 0 $merge_tool_path]
> +		} else {
> +			error_popup [mc "Unsupported merge tool '%s'. Is the tool command and path configured properly in gitconfig?" $tool]

Can we not have a more helpful text? How about

			error_popup [mc "Unsupported merge tool '%s'.

See the git-config manual page how to configure mergetool.%s.cmd
suitably." $tool $tool]

> +			return
> +		}
>  	}
>  	}
>  

-- Hannes
tobias.boesch@miele.com Sept. 5, 2024, 8:09 a.m. UTC | #3
Thanks for he review.

> -----Ursprüngliche Nachricht-----
> Von: Junio C Hamano <gitster@pobox.com>
> Gesendet: Mittwoch, 28. August 2024 19:08
> An: ToBoMi via GitGitGadget <gitgitgadget@gmail.com>
> Cc: git@vger.kernel.org; Boesch, Tobias <tobias.boesch@miele.com>
> Betreff: Re: [PATCH v2] git gui: add directly calling merge tool from gitconfig
>
> "ToBoMi via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: deboeto <tobias.boesch@miele.com>
>
> Use the same ident (human readable name plus e-mail address) you have on
> your Signed-off-by: line below for this "From: " in-body header.
>
> > git gui can open a merge tool when conflicts are detected (Right click
> > in the diff of the file with conflicts).
> > The merge tools that are allowed to
> > use are hard coded into git gui.
> >
> > If one wants to add a new merge tool it has to be added to git gui
> > through a source code change.
> > This is not convenient in comparison to how it works in git (without
> > gui).
> >
> > git itself has configuration options for a merge tools path and
> > command in the git config.
> > New merge tools can be set up there without a source code change.
>
> Even if you configure an unknown tool, it would not get any benefit from
> what git-{diff,merge}tool--lib.sh gives the known diff/merge backends, would
> it?  Instead of a more thorough support for known tools done in setup_tool(),
> an unknown tool would be handled by
> setup_user_tool() in git-mergetool-lib.sh which gives somewhat degraded
> support.
>

That might be. I don't know about this handling.
Would it be a problem to not have this handling for unsupported tools? Since the concept of supported tools is not removed by this patch, tools can still be added as supported, to get this thorough handling.
I personally prefer to have an unsupported tool, that I can configure and use right now and add official support for it later, instead of having some well-supported tools which exclude the tool I want to use right now and no option to add it quickly.
That is why I didn't add support for the tool I want to use right now. I wanted it to be more universal, so that every tool I can configure will work immediately.

> So "can be set up without" may be true, but giving an impression that a tool
> that is set up like so would work just like a known tool is misleading.
>

I don't want this patch to give that impression. How can this be avoided from your point of view?

The degraded functionality for unsupported tools could be mentioned in the message for an unsupported tool introduced enhanced in this patch. It could tell the user that the current tool is not supported, but that it can be setup with degraded support in the config.
An updated message will be part of the next patch.

> By the way, we do ask contributors to avoid overly long lines, 50-col limt is a
> bit overly short and makes the resulting text harder to read than necessary.
>
> > Those options are used only by pure git in contrast to git gui. git
> > calls the configured merge tools directly from the config while git
> > Gui doesn't.
> >
> > With this change git gui can call merge tools configured in the
> > gitconfig directly without a change in git gui source code.
> > It needs a configured merge.tool and a configured mergetool.cmd config
> > entry.
>
> OK.
>
> > gitconfig example:
> > [merge]
> >     tool = vscode
> > [mergetool "vscode"]
> >     path = the/path/to/Code.exe
> >     cmd = \"Code.exe\" --wait --merge \"$LOCAL\" \"$REMOTE\"
> \"$BASE\" \"$MERGED\"
> >
> > Without the mergetool.cmd configuration and an unsupported merge.tool
> > entry, git gui behaves mainly as before this change and informs the
> > user about an unsupported merge tool, but now also shows a hint to add
> > a config entry for the tool in gitconfig.
> >
> > If a wrong mergetool.cmd is configured by accident it is beeing
> > handled by git gui already. In this
>
> "is beeing" -> "is being", but "it gets handled by Git GUI already"
> should be sufficient.
>
> > case git gui informs the user that the merge tool couldn't be opened.
> > This behavior is preserved by this change and should not change.
> >
> > Beyond compare 3 and Visual Studio code were tested as manually
> > configured merge tools.
>
> Quote proper nouns for readability?  E.g.
>
>     "Beyond Compare 3" and "Visual Studio Code" were ...
>
> Thanks.

I'll correct the minor suggestions in the next patch version.


-------------------------------------------------------------------------------------------------
imperial-Werke oHG, Sitz Bünde, Registergericht Bad Oeynhausen - HRA 4825
tobias.boesch@miele.com Sept. 6, 2024, 6:32 a.m. UTC | #4
> -----Ursprüngliche Nachricht-----
> Von: Johannes Sixt <j6t@kdbg.org>
> Gesendet: Samstag, 31. August 2024 15:52
> An: Boesch, Tobias <tobias.boesch@miele.com>
> Cc: ToBoMi via GitGitGadget <gitgitgadget@gmail.com>; git@vger.kernel.org
> Betreff: Re: [PATCH v2] git gui: add directly calling merge tool from gitconfig
>
> Am 28.08.24 um 10:31 schrieb ToBoMi via GitGitGadget:
> > From: deboeto <tobias.boesch@miele.com>
> >
> > git gui can open a merge tool when conflicts are detected (Right click
> > in the diff of the file with conflicts).
> > The merge tools that are allowed to
> > use are hard coded into git gui.
> >
> > If one wants to add a new merge tool it has to be added to git gui
> > through a source code change.
> > This is not convenient in comparison to how it works in git (without
> > gui).
> >
> > git itself has configuration options for a merge tools path and
> > command in the git config.
> > New merge tools can be set up there without a source code change.
> >
> > Those options are used only by pure git in contrast to git gui. git
> > calls the configured merge tools directly from the config while git
> > Gui doesn't.
> >
> > With this change git gui can call merge tools configured in the
> > gitconfig directly without a change in git gui source code.
> > It needs a configured merge.tool and a configured mergetool.cmd config
> > entry.
>
> OK.
>
> > gitconfig example:
> > [merge]
> >     tool = vscode
> > [mergetool "vscode"]
> >     path = the/path/to/Code.exe
> >     cmd = \"Code.exe\" --wait --merge \"$LOCAL\" \"$REMOTE\"
> \"$BASE\" \"$MERGED\"
>
> I found it annoying that I had to configure .path in addition to .cmd.
> Typically, you would put the correct path into the .cmd configuration.
> In fact, `git mergetool` works without .path and fails when .cmd does not
> contain the correct path.
>

I changed it to only use the mergetool.cmd and updated the configuration hint that mentions the configuration variables so that users know to only specify the cmd variable.

> > Without the mergetool.cmd configuration and an unsupported merge.tool
> > entry, git gui behaves mainly as before this change and informs the
> > user about an unsupported merge tool, but now also shows a hint to add
> > a config entry for the tool in gitconfig.
>
> Good.
>
> While testing I configured meld incorrectly once and got no feedback
> whatsoever, but I would not attribute this to this patch.
>

That's odd. I tested this again by setting merge.tool to "meld" and configured mergetool.cmd to "some wrong path". When starting the mergetool I got a popup saying that meld was not found in path.

> There is no such thing called "gitconfig". Just strike "in gitconfig".
>
> > If a wrong mergetool.cmd is configured by accident it is beeing
> > handled by git gui already. In this case git gui informs the user that
> > the merge tool couldn't be opened. This behavior is preserved by this
> > change and should not change.
>
> Good.
>
> >
> > Beyond compare 3 and Visual Studio code were tested as manually
> > configured merge tools.
> >
> > Signed-off-by: Tobias Boesch <tobias.boesch@miele.com>
>
> You updated this line, but not the From: line. Would you mind configuring
> your user.name and then `git commit --amend --reset-author`?
>
> >  git-gui/lib/mergetool.tcl | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/git-gui/lib/mergetool.tcl b/git-gui/lib/mergetool.tcl
> > index e688b016ef6..4c4e8f47bb0 100644
> > --- a/git-gui/lib/mergetool.tcl
> > +++ b/git-gui/lib/mergetool.tcl
> > @@ -272,8 +272,14 @@ proc merge_resolve_tool2 {} {
> >             }
> >     }
> >     default {
> > -           error_popup [mc "Unsupported merge tool '%s'" $tool]
> > -           return
> > +           set tool_cmd [get_config mergetool.$tool.cmd]
> > +           if {$tool_cmd ne {}} {
> > +                   set tool_cmd_file_vars_resolved [subst -nobackslashes
> -nocommands
> > +$tool_cmd]
>
> I just learnt that a string value containing double-quotes is broken into a list in
> the expected way (keeps quoted parts together as a single element). However,
> this form of substitution replaces variable values with arbitrary text without
> taking into account that the original string is actually a list. Should we not
> break the string into a list first, and apply the substitution on the list elements?
>

I can iterate directly on the input string as a list. Will be part of the next patch version.

> If there is a straight-forward way to do this (say, an obvious two-liner at
> most), we should do it. Otherwise, I can live with this solution for now
> because it requires file names with double-quotes to break the expected list
> nature.
>
> There is another thing, though, that I would not want to take as
> lightly: The -nocommands modifier of `subst` does not live up to its promises,
> and it is even the documented behavior: command substitutions in array
> indexes are still executed. Consider this configuration:
>
> [merge]
>         tool = evil
> [mergetool "evil"]
>         cmd = meld \"$REMOTE([exit])\"
>
> Guess what happens when I run the merge tool? It exits Git GUI!
>
> I suggest to reject any configuration that contains an opening bracket '[' or
> anything else that introduces a command execution.
>

Good catch. I added code to prevent this in the next patch version.
When a command sequence is detected (basically square brackets in the command) the user get a hint to avoid square brackets in the mergetool.cmd.

> > +                   set cmdline [lreplace $tool_cmd_file_vars_resolved 0 0
> $merge_tool_path]
> > +           } else {
> > +                   error_popup [mc "Unsupported merge tool '%s'. Is the
> tool command
> > +and path configured properly in gitconfig?" $tool]
>
> Can we not have a more helpful text? How about
>
>                       error_popup [mc "Unsupported merge tool '%s'.
>
> See the git-config manual page how to configure mergetool.%s.cmd suitably."
> $tool $tool]
>

True. Updating the text.

> > +                   return
> > +           }
> >     }
> >     }
> >
>
> -- Hannes


Minor suggestions will be fixed in the next patch.


-------------------------------------------------------------------------------------------------
imperial-Werke oHG, Sitz Bünde, Registergericht Bad Oeynhausen - HRA 4825
Johannes Sixt Sept. 6, 2024, 5:43 p.m. UTC | #5
Am 06.09.24 um 08:32 schrieb tobias.boesch@miele.com:
>> Von: Johannes Sixt <j6t@kdbg.org>
>> While testing I configured meld incorrectly once and got no feedback
>> whatsoever, but I would not attribute this to this patch.
>>
> 
> That's odd. I tested this again by setting merge.tool to "meld" and
> configured mergetool.cmd to "some wrong path". When starting the
> mergetool I got a popup saying that meld was not found in path.
But if the configuration is

   cmd = "meld far too many arguments provided"

and 'meld' *is* in the path, then there is no feedback because meld can
be started successfully, but reports an error only on stdout or stderr,
which is ignored by Git GUI. And the exit code seems to be ignored, too.

But this can be treated in a follow-up patch if necessary.

-- Hannes
diff mbox series

Patch

diff --git a/git-gui/lib/mergetool.tcl b/git-gui/lib/mergetool.tcl
index e688b016ef6..4c4e8f47bb0 100644
--- a/git-gui/lib/mergetool.tcl
+++ b/git-gui/lib/mergetool.tcl
@@ -272,8 +272,14 @@  proc merge_resolve_tool2 {} {
 		}
 	}
 	default {
-		error_popup [mc "Unsupported merge tool '%s'" $tool]
-		return
+		set tool_cmd [get_config mergetool.$tool.cmd]
+		if {$tool_cmd ne {}} {
+			set tool_cmd_file_vars_resolved [subst -nobackslashes -nocommands $tool_cmd]
+			set cmdline [lreplace $tool_cmd_file_vars_resolved 0 0 $merge_tool_path]
+		} else {
+			error_popup [mc "Unsupported merge tool '%s'. Is the tool command and path configured properly in gitconfig?" $tool]
+			return
+		}
 	}
 	}