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

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

[ToBoMi via GitGitGadget]

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

* 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>
---
    git gui: add directly calling merge tool from gitconfig

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

 Documentation/config/gui.txt |  4 ++++
 git-gui/lib/mergetool.tcl    | 11 +++++++++--
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/Documentation/config/gui.txt b/Documentation/config/gui.txt
index 171be774d24..e63d0b46e7c 100644
--- a/Documentation/config/gui.txt
+++ b/Documentation/config/gui.txt
@@ -55,3 +55,7 @@ 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
diff --git a/git-gui/lib/mergetool.tcl b/git-gui/lib/mergetool.tcl
index e688b016ef6..fbd0889612a 100644
--- a/git-gui/lib/mergetool.tcl
+++ b/git-gui/lib/mergetool.tcl
@@ -272,8 +272,15 @@ 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]
+		} else {
+			error_popup [mc "Unsupported merge tool '%s'" $tool]
+			return
+		}
 	}
 	}
 

base-commit: b9849e4f7631d80f146d159bf7b60263b3205632
-- 
gitgitgadget

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

[Johannes Sixt]

Am 19.08.24 um 13:29 schrieb ToBoMi via GitGitGadget:
> From: deboeto <tobias.boesch@miele.com>
> 
> * 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

Can we do better than having a new configuration variable? Let's say you
have configured merge.tool=vscode. This tool is not supported, but you
have configured mergetool.vscode.cmd suitably. Can we not use the latter
configuration variable unconditionally?

Likewise, say, you have configured merge.tool=bc3. This one *is*
supported. What could go wrong if mergetool.bc3.cmd is used instead of
the built-in command line? The behavior would change for users that
configured mergetool.$tool.cmd for a supported tool. But would it change
for the worse?

BTW, the code builds different command lines depending on whether a base
file is available or not. How does mergetool.$tool.cmd handle the cases?

> * Disabled by default, since option is most likely
>     never set
> * bc3 and vscode tested
> 
> Signed-off-by: deboeto <tobias.boesch@miele.com>

Some remarks on the commit message:

- The Signed-off-by line has legal consequences. Therefore, we require
that authors use their genuine name, not an alias. Also, the From line
must match the Signed-off-by line.

- Please have a look at the commit messages in the code base. The
formatting presented here is very unusual. Please write in full
sentences including punctuation, and use paragraphs where needed.

- Please state the problem that is being solved (in present tense). This
should motivate the change, i.e., provide a convincing argument why the
change is needed. Then state what the solution is in imperative mood,
that is, an instruction to the code to change in such and such way. Use
examples to clarify how the new feature can be used.

> ---
>     git gui: add directly calling merge tool from gitconfig
> 
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1773%2FToBoMi%2Fadd_merge_tool_from_config_file-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1773/ToBoMi/add_merge_tool_from_config_file-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/1773
> 
>  Documentation/config/gui.txt |  4 ++++
>  git-gui/lib/mergetool.tcl    | 11 +++++++++--
>  2 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/config/gui.txt b/Documentation/config/gui.txt
> index 171be774d24..e63d0b46e7c 100644
> --- a/Documentation/config/gui.txt
> +++ b/Documentation/config/gui.txt
> @@ -55,3 +55,7 @@ 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

Unfortunately, Documentation/config/gui.txt is not part of the Git GUI
repository. Any changes to the documentation must be submitted as
separate patch.

Please be careful not to introduce an incomplete last lines. Take note
of "No newline at end of file". It should not be there.

> diff --git a/git-gui/lib/mergetool.tcl b/git-gui/lib/mergetool.tcl
> index e688b016ef6..fbd0889612a 100644
> --- a/git-gui/lib/mergetool.tcl
> +++ b/git-gui/lib/mergetool.tcl
> @@ -272,8 +272,15 @@ 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]

At this point, the value assigned to $path here is already available in
$merge_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]

I haven't yet taken the time to study what these lines do (I am far from
fluent in Tcl) and have no opinion, yet.

> +		} else {
> +			error_popup [mc "Unsupported merge tool '%s'" $tool]
> +			return
> +		}
>  	}
>  	}
>  
> 
> base-commit: b9849e4f7631d80f146d159bf7b60263b3205632

-- Hannes

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

[tobias.boesch]

> -----Ursprüngliche Nachricht-----
> Von: Johannes Sixt <j6t@kdbg.org>
> Gesendet: Samstag, 24. August 2024 15:38
> An: Boesch, Tobias <tobias.boesch@miele.com>
> Cc: git@vger.kernel.org; ToBoMi via GitGitGadget <gitgitgadget@gmail.com>
> Betreff: Re: [PATCH] git gui: add directly calling merge tool from gitconfig
>

Thanks for the review.

> Am 19.08.24 um 13:29 schrieb ToBoMi via GitGitGadget:
> > From: deboeto <tobias.boesch@miele.com>
> >
> > * 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
>
> Can we do better than having a new configuration variable? Let's say you have
> configured merge.tool=vscode. This tool is not supported, but you have
> configured mergetool.vscode.cmd suitably. Can we not use the latter
> configuration variable unconditionally?
>

I think that would work. I'll change the patch to use the mergetool.cmd variable
as the trigger for using the configured merge tool.
If the mergetool configuration option is set to a supported tool
(merge.tool = vscode) it will cause git gui to use the supported tool (hard coded
into the source code - as before this change).
If the mergetool configuration option is set to an UNsupported tool and
mergetool.cmd is set for the chosen mergetool it will use the command from
that option.

Patch will be submitted soon.

> Likewise, say, you have configured merge.tool=bc3. This one *is* supported.
> What could go wrong if mergetool.bc3.cmd is used instead of the built-in
> command line? The behavior would change for users that configured
> mergetool.$tool.cmd for a supported tool. But would it change for the worse?
>

I tested this together with the change of the activating option mentioned before.
With that in mind I cannot create a case were the mergetool.cmd is used on a
supported tool, because mergetool.cmd is only used for UNsupported tools.
1. Assuming the merge.tool is set to a supported tool:
        - In this case the supported tool is used, no matter if the mergetool.cmd
        is set or not
2. Assuming the merge.tool is set to an UNsupported tool:
        - Then the variable IS evaluated
        - If it is set to an invalid command or a wrong mergetool.path is given,
         Git gui will complain as before this change that the command is not
        Found in PATH

> BTW, the code builds different command lines depending on whether a base
> file is available or not. How does mergetool.$tool.cmd handle the cases?
>

Currently it doesn't.
I don't know if it should, because I guess that git also has no other possibility
than to call this command for a merge unconditionally - even when the base file
name is empty.
I haven't had such a case that I can remember. How can it be triggered?
Doesn't all merges have a common ancestor as long as the histories are related?

> > * Disabled by default, since option is most likely
> >     never set
> > * bc3 and vscode tested
> >
> > Signed-off-by: deboeto <tobias.boesch@miele.com>
>
> Some remarks on the commit message:
>
> - The Signed-off-by line has legal consequences. Therefore, we require that
> authors use their genuine name, not an alias. Also, the From line must match
> the Signed-off-by line.
>
> - Please have a look at the commit messages in the code base. The formatting
> presented here is very unusual. Please write in full sentences including
> punctuation, and use paragraphs where needed.
>
> - Please state the problem that is being solved (in present tense). This should
> motivate the change, i.e., provide a convincing argument why the change is
> needed. Then state what the solution is in imperative mood, that is, an
> instruction to the code to change in such and such way. Use examples to
> clarify how the new feature can be used.

Corrected - please review again.

>
> > ---
> >     git gui: add directly calling merge tool from gitconfig
> >
> > Published-As:
> > https://github.com/gitgitgadget/git/releases/tag/pr-
> 1773%2FToBoMi%2Fad
> > d_merge_tool_from_config_file-v1
> > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git
> > pr-1773/ToBoMi/add_merge_tool_from_config_file-v1
> > Pull-Request: https://github.com/gitgitgadget/git/pull/1773
> >
> >  Documentation/config/gui.txt |  4 ++++
> >  git-gui/lib/mergetool.tcl    | 11 +++++++++--
> >  2 files changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/config/gui.txt
> > b/Documentation/config/gui.txt index 171be774d24..e63d0b46e7c
> 100644
> > --- a/Documentation/config/gui.txt
> > +++ b/Documentation/config/gui.txt
> > @@ -55,3 +55,7 @@ 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
>
> Unfortunately, Documentation/config/gui.txt is not part of the Git GUI
> repository. Any changes to the documentation must be submitted as separate
> patch.
>
> Please be careful not to introduce an incomplete last lines. Take note of "No
> newline at end of file". It should not be there.
>
> > diff --git a/git-gui/lib/mergetool.tcl b/git-gui/lib/mergetool.tcl
> > index e688b016ef6..fbd0889612a 100644
> > --- a/git-gui/lib/mergetool.tcl
> > +++ b/git-gui/lib/mergetool.tcl
> > @@ -272,8 +272,15 @@ 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]
>
> At this point, the value assigned to $path here is already available in
> $merge_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]
>
> I haven't yet taken the time to study what these lines do (I am far from fluent
> in Tcl) and have no opinion, yet.
>
> > +           } else {
> > +                   error_popup [mc "Unsupported merge tool '%s'"
> $tool]
> > +                   return
> > +           }
> >     }
> >     }
> >
> >
> > base-commit: b9849e4f7631d80f146d159bf7b60263b3205632
>
> -- Hannes



-------------------------------------------------------------------------------------------------
imperial-Werke oHG, Sitz Bünde, Registergericht Bad Oeynhausen - HRA 4825

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

[tobias.boesch]

> -----Ursprüngliche Nachricht-----
> Von: Johannes Sixt <j6t@kdbg.org>
> Gesendet: Samstag, 24. August 2024 15:38
> An: Boesch, Tobias <tobias.boesch@miele.com>
> Cc: git@vger.kernel.org; ToBoMi via GitGitGadget <gitgitgadget@gmail.com>
> Betreff: Re: [PATCH] git gui: add directly calling merge tool from gitconfig
>

Continuing my incomplete last reply:

> Am 19.08.24 um 13:29 schrieb ToBoMi via GitGitGadget:
> > From: deboeto <tobias.boesch@miele.com>
> >
> > * 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
>
> Can we do better than having a new configuration variable? Let's say you have
> configured merge.tool=vscode. This tool is not supported, but you have
> configured mergetool.vscode.cmd suitably. Can we not use the latter
> configuration variable unconditionally?
>
> Likewise, say, you have configured merge.tool=bc3. This one *is* supported.
> What could go wrong if mergetool.bc3.cmd is used instead of the built-in
> command line? The behavior would change for users that configured
> mergetool.$tool.cmd for a supported tool. But would it change for the worse?
>
> BTW, the code builds different command lines depending on whether a base
> file is available or not. How does mergetool.$tool.cmd handle the cases?
>
> > * Disabled by default, since option is most likely
> >     never set
> > * bc3 and vscode tested
> >
> > Signed-off-by: deboeto <tobias.boesch@miele.com>
>
> Some remarks on the commit message:
>
> - The Signed-off-by line has legal consequences. Therefore, we require that
> authors use their genuine name, not an alias. Also, the From line must match
> the Signed-off-by line.
>
> - Please have a look at the commit messages in the code base. The formatting
> presented here is very unusual. Please write in full sentences including
> punctuation, and use paragraphs where needed.
>
> - Please state the problem that is being solved (in present tense). This should
> motivate the change, i.e., provide a convincing argument why the change is
> needed. Then state what the solution is in imperative mood, that is, an
> instruction to the code to change in such and such way. Use examples to
> clarify how the new feature can be used.
>
> > ---
> >     git gui: add directly calling merge tool from gitconfig
> >
> > Published-As:
> > https://github.com/gitgitgadget/git/releases/tag/pr-
> 1773%2FToBoMi%2Fad
> > d_merge_tool_from_config_file-v1
> > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git
> > pr-1773/ToBoMi/add_merge_tool_from_config_file-v1
> > Pull-Request: https://github.com/gitgitgadget/git/pull/1773
> >
> >  Documentation/config/gui.txt |  4 ++++
> >  git-gui/lib/mergetool.tcl    | 11 +++++++++--
> >  2 files changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/config/gui.txt
> > b/Documentation/config/gui.txt index 171be774d24..e63d0b46e7c
> 100644
> > --- a/Documentation/config/gui.txt
> > +++ b/Documentation/config/gui.txt
> > @@ -55,3 +55,7 @@ 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
>
> Unfortunately, Documentation/config/gui.txt is not part of the Git GUI
> repository. Any changes to the documentation must be submitted as separate
> patch.
>

Configuration option will be removed in the next patch version. Therefore the
documentation change is no longer needed.

> Please be careful not to introduce an incomplete last lines. Take note of "No
> newline at end of file". It should not be there.
>

See above.

> > diff --git a/git-gui/lib/mergetool.tcl b/git-gui/lib/mergetool.tcl
> > index e688b016ef6..fbd0889612a 100644
> > --- a/git-gui/lib/mergetool.tcl
> > +++ b/git-gui/lib/mergetool.tcl
> > @@ -272,8 +272,15 @@ 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]
>
> At this point, the value assigned to $path here is already available in
> $merge_tool_path.
>

True - corrected in the next patch.

> > +                   set cmdline_config [get_config mergetool.$tool.cmd]
> > +                   set cmdline_substituted [subst -nobackslashes -
> nocommands $cmdline_config]
> > +                   set cmdline [lreplace $cmdline_substituted 0 0 $path]
>
> I haven't yet taken the time to study what these lines do (I am far from fluent
> in Tcl) and have no opinion, yet.
>

They replace the variables one can put into mergetool.cmd like $REMOTE or
$LOCAL.
Without this substitution command they are not replaced with the real file
paths.
See this example for vscode:
cmd = \"Code.exe\" --wait --merge \"$LOCAL\" \"$REMOTE\" \"$BASE\" \"$MERGED\"

> > +           } else {
> > +                   error_popup [mc "Unsupported merge tool '%s'"
> $tool]
> > +                   return
> > +           }
> >     }
> >     }
> >
> >
> > base-commit: b9849e4f7631d80f146d159bf7b60263b3205632
>
> -- Hannes



-------------------------------------------------------------------------------------------------
imperial-Werke oHG, Sitz Bünde, Registergericht Bad Oeynhausen - HRA 4825

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

[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.

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(-)

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
+		}
 	}
 	}
 

base-commit: 159f2d50e75c17382c9f4eb7cbda671a6fa612d1
-- 
gitgitgadget

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

[Junio C Hamano]

"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.

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

[tobias.boesch]

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

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

[Johannes Sixt]

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

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

[tobias.boesch]

> -----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

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

[Johannes Sixt]

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

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

[ToBoMi via GitGitGadget]

From: Tobias Boesch <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. In addtition it also shows a hint to add
a config entry to use the tool as an unsupported tool with degraded
support.

If a wrong mergetool.cmd is configured by accident, it gets 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
    
    Changes since v2:
    
     * Changed commit ident
     * Added hint to add a mergetool as an unsupprted tool
     * Minor typos
     * Highlighted proper nouns in commit message
     * Only using mergetool.cmd now - not using mergetool.path anymore
     * Removed gitconfig term in user message
     * Changed lines length of commit message
     * tcl commands in mergetool.cmd are now detected and not executed
       anymore
     * mergetool.cmd string parts are now substituted as list, not as a
       whole string
     * Made a more clear user hint on how to configure an unsupported
       mergetool

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

Range-diff vs v2:

 1:  e77d6dec6c5 ! 1:  c8c0107ddc5 git gui: add directly calling merge tool from gitconfig
     @@
       ## Metadata ##
     -Author: deboeto <tobias.boesch@miele.com>
     +Author: Tobias Boesch <tobias.boesch@miele.com>
      
       ## Commit message ##
          git gui: add directly calling merge tool from gitconfig
      
     -    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.
     +    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).
     +    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.
     +    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.
     +    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.
     +    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]
     @@ Commit message
                  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.
     +    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. In addtition it also shows a hint to add
     +    a config entry to use the tool as an unsupported tool with degraded
     +    support.
      
     -    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.
     +    If a wrong mergetool.cmd is configured by accident, it gets 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.
     +    "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: proc merge_resolve_tool2 {} {
      -		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]
     ++			if {([string first {[} $tool_cmd] != -1) || ([string first {]} $tool_cmd] != -1)} {
     ++				error_popup [mc "Unable to process square brackets in mergetool.cmd configuration option.\
     ++								Please remove the square brackets."]
     ++				return
     ++			} else {
     ++				foreach command_part $tool_cmd {
     ++					lappend cmdline [subst -nobackslashes -nocommands $command_part]
     ++				}
     ++			}
      +		} else {
     -+			error_popup [mc "Unsupported merge tool '%s'. Is the tool command and path configured properly in gitconfig?" $tool]
     ++			error_popup [mc "Unsupported merge tool '%s'.\n
     ++							Currently unsupported tools can be added and used as unsupported tools with degraded support\
     ++							by adding the command of the tool to the \"mergetool.cmd\" option in the config.
     ++							See the configuration documentation for more details." $tool]
      +			return
      +		}
       	}


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

diff --git a/git-gui/lib/mergetool.tcl b/git-gui/lib/mergetool.tcl
index e688b016ef6..ccbc1a46554 100644
--- a/git-gui/lib/mergetool.tcl
+++ b/git-gui/lib/mergetool.tcl
@@ -272,8 +272,24 @@ 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 {}} {
+			if {([string first {[} $tool_cmd] != -1) || ([string first {]} $tool_cmd] != -1)} {
+				error_popup [mc "Unable to process square brackets in mergetool.cmd configuration option.\
+								Please remove the square brackets."]
+				return
+			} else {
+				foreach command_part $tool_cmd {
+					lappend cmdline [subst -nobackslashes -nocommands $command_part]
+				}
+			}
+		} else {
+			error_popup [mc "Unsupported merge tool '%s'.\n
+							Currently unsupported tools can be added and used as unsupported tools with degraded support\
+							by adding the command of the tool to the \"mergetool.cmd\" option in the config.
+							See the configuration documentation for more details." $tool]
+			return
+		}
 	}
 	}
 

base-commit: 2e7b89e038c0c888acf61f1b4ee5a43d4dd5e94c
-- 
gitgitgadget

Re: [PATCH v3] git gui: add directly calling merge tool from gitconfig

[Johannes Sixt]

Am 06.09.24 um 09:27 schrieb ToBoMi via GitGitGadget:
> From: Tobias Boesch <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.

The configuration is "mergetool.$tool.cmd"!

Personally, I would avoid the words "gitconfig" and "config" (here and
in the rest of the commit message), neither of which are English.
"Configuration" would be OK, IMO.

> 
> 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. In addtition it also shows a hint to add
> a config entry to use the tool as an unsupported tool with degraded
> support.
> 
> If a wrong mergetool.cmd is configured by accident, it gets 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 | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/git-gui/lib/mergetool.tcl b/git-gui/lib/mergetool.tcl
> index e688b016ef6..ccbc1a46554 100644
> --- a/git-gui/lib/mergetool.tcl
> +++ b/git-gui/lib/mergetool.tcl
> @@ -272,8 +272,24 @@ 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 {}} {
> +			if {([string first {[} $tool_cmd] != -1) || ([string first {]} $tool_cmd] != -1)} {
> +				error_popup [mc "Unable to process square brackets in mergetool.cmd configuration option.\
> +								Please remove the square brackets."]
> +				return

Condition and error text are OK. But see below.

> +			} else {
> +				foreach command_part $tool_cmd {
> +					lappend cmdline [subst -nobackslashes -nocommands $command_part]
> +				}

Good.

I have seen a few examples in the Tcl manual with lappend in the loop
body, and it seems to be customary to set the list variable to an empty
value before the loop, i.e.

				set cmdline {}

> +			}
> +		} else {
> +			error_popup [mc "Unsupported merge tool '%s'.\n
> +							Currently unsupported tools can be added and used as unsupported tools with degraded support\
> +							by adding the command of the tool to the \"mergetool.cmd\" option in the config.
> +							See the configuration documentation for more details." $tool]

This error message needs a bit more work (some of this also applies to
the message above):

- A tool is only unsupported as long as there is no usable
configuration. Once mergetool.$tool.cmd is set to something we can
handle, calling the tool "unsupported" isn't appropriate, I would think.
How about

Unsupported merge tool '%s'.

To use this tool, configure "mergetool.%s.cmd" as shown in the
git-config manual page.

- The configuration variable that we use is not mergetool.cmd, but
mergetool.$tool.cmd.

- Continuation lines must not be indented. Indented text appears
indented in the error message.

- Watch out whether an explicit \n is given, whether the line-break is
escaped or not; all of this has meaning.

- Looking at other multi-line error messages in git-gui.sh, the
convention is

	mc["First paragraph goes here.

Second paragraph. All of it is on one line in the source code.

Third paragraph. No \n appears anywhere."]

> +			return
> +		}
>  	}
>  	}

As a matter of personal taste, I prefer to structure code with error
exits like so (but it is totally acceptable if you disagree):

   if {check for error 1} {
       error msg1
       return
   }
   if {check for error 2} {
       error msg2
       return
   }
   regular case
   goes here
   without indentation

Note that there are no else-branches. This reduces the indentation levels.

-- Hannes

AW: [PATCH v3] git gui: add directly calling merge tool from gitconfig

[tobias.boesch]

> -----Ursprüngliche Nachricht-----
> Von: Johannes Sixt <j6t@kdbg.org>
> Gesendet: Sonntag, 8. September 2024 14:21
> An: Boesch, Tobias <tobias.boesch@miele.com>
> Cc: git@vger.kernel.org; ToBoMi via GitGitGadget <gitgitgadget@gmail.com>
> Betreff: Re: [PATCH v3] git gui: add directly calling merge tool from gitconfig
>
> Am 06.09.24 um 09:27 schrieb ToBoMi via GitGitGadget:
> > From: Tobias Boesch <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.
>
> The configuration is "mergetool.$tool.cmd"!
>

Right - changed it to "mergetool.<mergetool name>.cmd", since this is a descriptive
text and not a script. I personally prefer a more human friendly writing.

> Personally, I would avoid the words "gitconfig" and "config" (here and in the
> rest of the commit message), neither of which are English.
> "Configuration" would be OK, IMO.
>

Will be changed.

> >
> > 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. In addtition it also shows a
> > hint to add a config entry to use the tool as an unsupported tool with
> > degraded support.
> >
> > If a wrong mergetool.cmd is configured by accident, it gets 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 | 20 ++++++++++++++++++--
> >  1 file changed, 18 insertions(+), 2 deletions(-)
> >
> > diff --git a/git-gui/lib/mergetool.tcl b/git-gui/lib/mergetool.tcl
> > index e688b016ef6..ccbc1a46554 100644
> > --- a/git-gui/lib/mergetool.tcl
> > +++ b/git-gui/lib/mergetool.tcl
> > @@ -272,8 +272,24 @@ 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 {}} {
> > +                   if {([string first {[} $tool_cmd] != -1) || ([string first {]}
> $tool_cmd] != -1)} {
> > +                           error_popup [mc "Unable to process square
> brackets in mergetool.cmd configuration option.\
> > +                                                           Please remove
> the square brackets."]
> > +                           return
>
> Condition and error text are OK. But see below.
>
> > +                   } else {
> > +                           foreach command_part $tool_cmd {
> > +                                   lappend cmdline [subst -nobackslashes
> -nocommands $command_part]
> > +                           }
>
> Good.
>
> I have seen a few examples in the Tcl manual with lappend in the loop body,
> and it seems to be customary to set the list variable to an empty value before
> the loop, i.e.
>
>                               set cmdline {}
>

Done in next patch.

> > +                   }
> > +           } else {
> > +                   error_popup [mc "Unsupported merge tool '%s'.\n
> > +                                                   Currently unsupported
> tools can be added and used as unsupported tools with degraded support\
> > +                                                   by adding the
> command of the tool to the \"mergetool.cmd\" option in the config.
> > +                                                   See the configuration
> documentation for more details." $tool]
>
> This error message needs a bit more work (some of this also applies to the
> message above):
>
> - A tool is only unsupported as long as there is no usable configuration. Once
> mergetool.$tool.cmd is set to something we can handle, calling the tool
> "unsupported" isn't appropriate, I would think.

Junio C Hamano suggested to mark the tools unsupported in his review.
I'll change it back and remove the unsupported hint.

> How about
>
> Unsupported merge tool '%s'.
>
> To use this tool, configure "mergetool.%s.cmd" as shown in the git-config
> manual page.
>

Will be changed.

> - The configuration variable that we use is not mergetool.cmd, but
> mergetool.$tool.cmd.
>
> - Continuation lines must not be indented. Indented text appears indented in
> the error message.
>
> - Watch out whether an explicit \n is given, whether the line-break is escaped
> or not; all of this has meaning.
>
> - Looking at other multi-line error messages in git-gui.sh, the convention is
>
>       mc["First paragraph goes here.
>
> Second paragraph. All of it is on one line in the source code.
>
> Third paragraph. No \n appears anywhere."]
>

I didn't want to have unindented text ion the source code, but I'll change it.

> > +                   return
> > +           }
> >     }
> >     }
>
> As a matter of personal taste, I prefer to structure code with error exits like so
> (but it is totally acceptable if you disagree):
>
>    if {check for error 1} {
>        error msg1
>        return
>    }
>    if {check for error 2} {
>        error msg2
>        return
>    }
>    regular case
>    goes here
>    without indentation
>
> Note that there are no else-branches. This reduces the indentation levels.
>

I tried to set this up - it failed because the square brackets in the if condition suddenly get "counted" as command executions or in other words; the curly braces are ignored. I was unable to get around that.

> -- Hannes



-------------------------------------------------------------------------------------------------
imperial-Werke oHG, Sitz Bünde, Registergericht Bad Oeynhausen - HRA 4825

[PATCH v4] git gui: add directly calling merge tool from configuration

[ToBoMi via GitGitGadget]

From: Tobias Boesch <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 configuration.
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 configuration while git Gui
doesn't.

With this change git gui can call merge tools configured in the
configuration directly without a change in git gui source code.
It needs a configured "merge.tool" and a configured
"mergetool.<mergetool name>.cmd" configuration entry as shown in the
git-config manual page.

Configuration 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. In addtition it also shows a hint to add
a configuration entry to use the tool as an unsupported tool with degraded
support.

If a wrong "mergetool.cmd" is configured by accident, it gets 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
    
    Changes since v2:
    
     * Changed commit ident
     * Added hint to add a mergetool as an unsupprted tool
     * Minor typos
     * Highlighted proper nouns in commit message
     * Only using mergetool.cmd now - not using mergetool.path anymore
     * Removed gitconfig term in user message
     * Changed lines length of commit message
     * tcl commands in mergetool.cmd are now detected and not executed
       anymore
     * mergetool.cmd string parts are now substituted as list, not as a
       whole string
     * Made a more clear user hint on how to configure an unsupported
       mergetool
    
    Changes since v3:
    
     * Corrected wrong configuration option in commit message
     * Replaced "config" and "gitconfig" with "configuration" in commit
       message
     * Initialised cmdline list before appending values in loop
     * Removed hint that unsupported tools have degraded support
     * Changed popup message formatting in the popup and in the source code

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

Range-diff vs v3:

 1:  c8c0107ddc5 ! 1:  b5db8997b76 git gui: add directly calling merge tool from gitconfig
     @@ Metadata
      Author: Tobias Boesch <tobias.boesch@miele.com>
      
       ## Commit message ##
     -    git gui: add directly calling merge tool from gitconfig
     +    git gui: add directly calling merge tool from configuration
      
          git gui can open a merge tool when conflicts are detected (Right click
          in the diff of the file with conflicts).
     @@ Commit message
          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.
     +    in the git configuration.
          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.
     +    the configured merge tools directly from the configuration 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.
     +    With this change git gui can call merge tools configured in the
     +    configuration directly without a change in git gui source code.
     +    It needs a configured "merge.tool" and a configured
     +    "mergetool.<mergetool name>.cmd" configuration entry as shown in the
     +    git-config manual page.
      
     -    gitconfig example:
     +    Configuration 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
     +    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. In addtition it also shows a hint to add
     -    a config entry to use the tool as an unsupported tool with degraded
     +    a configuration entry to use the tool as an unsupported tool with degraded
          support.
      
     -    If a wrong mergetool.cmd is configured by accident, it gets handled
     +    If a wrong "mergetool.cmd" is configured by accident, it gets 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.
     @@ git-gui/lib/mergetool.tcl: proc merge_resolve_tool2 {} {
      +		set tool_cmd [get_config mergetool.$tool.cmd]
      +		if {$tool_cmd ne {}} {
      +			if {([string first {[} $tool_cmd] != -1) || ([string first {]} $tool_cmd] != -1)} {
     -+				error_popup [mc "Unable to process square brackets in mergetool.cmd configuration option.\
     -+								Please remove the square brackets."]
     ++				error_popup [mc "Unable to process square brackets in mergetool.$tool.cmd configuration option.
     ++
     ++Please remove the square brackets."]
      +				return
      +			} else {
     ++				set cmdline {}
      +				foreach command_part $tool_cmd {
      +					lappend cmdline [subst -nobackslashes -nocommands $command_part]
      +				}
      +			}
      +		} else {
     -+			error_popup [mc "Unsupported merge tool '%s'.\n
     -+							Currently unsupported tools can be added and used as unsupported tools with degraded support\
     -+							by adding the command of the tool to the \"mergetool.cmd\" option in the config.
     -+							See the configuration documentation for more details." $tool]
     ++			error_popup [mc "Unsupported merge tool '%s'.
     ++							
     ++To use this tool, configure \"mergetool.%s.cmd\" as shown in the git-config\
     ++manual page." $tool $tool]
      +			return
      +		}
       	}


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

diff --git a/git-gui/lib/mergetool.tcl b/git-gui/lib/mergetool.tcl
index e688b016ef6..2afc875ea0a 100644
--- a/git-gui/lib/mergetool.tcl
+++ b/git-gui/lib/mergetool.tcl
@@ -272,8 +272,26 @@ 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 {}} {
+			if {([string first {[} $tool_cmd] != -1) || ([string first {]} $tool_cmd] != -1)} {
+				error_popup [mc "Unable to process square brackets in mergetool.$tool.cmd configuration option.
+
+Please remove the square brackets."]
+				return
+			} else {
+				set cmdline {}
+				foreach command_part $tool_cmd {
+					lappend cmdline [subst -nobackslashes -nocommands $command_part]
+				}
+			}
+		} else {
+			error_popup [mc "Unsupported merge tool '%s'.
+							
+To use this tool, configure \"mergetool.%s.cmd\" as shown in the git-config\
+manual page." $tool $tool]
+			return
+		}
 	}
 	}
 

base-commit: c5ee8f2d1c9d336e0a46139bd35236d4a0bb93ff
-- 
gitgitgadget

[PATCH v5] git gui: add directly calling merge tool from configuration

[ToBoMi via GitGitGadget]

From: Tobias Boesch <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 configuration.
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 configuration while git Gui
doesn't.

With this change git gui can call merge tools configured in the
configuration directly without a change in git gui source code.
It needs a configured "merge.tool" and a configured
"mergetool.<mergetool name>.cmd" configuration entry as shown in the
git-config manual page.

Configuration 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. In addtition it also shows a hint to add
a configuration entry to use the tool as an unsupported tool with degraded
support.

If a wrong "mergetool.cmd" is configured by accident, it gets 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
    
    Changes since v2:
    
     * Changed commit ident
     * Added hint to add a mergetool as an unsupprted tool
     * Minor typos
     * Highlighted proper nouns in commit message
     * Only using mergetool.cmd now - not using mergetool.path anymore
     * Removed gitconfig term in user message
     * Changed lines length of commit message
     * tcl commands in mergetool.cmd are now detected and not executed
       anymore
     * mergetool.cmd string parts are now substituted as list, not as a
       whole string
     * Made a more clear user hint on how to configure an unsupported
       mergetool
    
    Changes since v3:
    
     * Corrected wrong configuration option in commit message
     * Replaced "config" and "gitconfig" with "configuration" in commit
       message
     * Initialised cmdline list before appending values in loop
     * Removed hint that unsupported tools have degraded support
     * Changed popup message formatting in the popup and in the source code
    
    Changes since v4:
    
     * Removed trailing whitespace

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

Range-diff vs v4:

 1:  b5db8997b76 ! 1:  c37db60c218 git gui: add directly calling merge tool from configuration
     @@ git-gui/lib/mergetool.tcl: proc merge_resolve_tool2 {} {
      +			}
      +		} else {
      +			error_popup [mc "Unsupported merge tool '%s'.
     -+							
     ++
      +To use this tool, configure \"mergetool.%s.cmd\" as shown in the git-config\
      +manual page." $tool $tool]
      +			return


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

diff --git a/git-gui/lib/mergetool.tcl b/git-gui/lib/mergetool.tcl
index e688b016ef6..50ed530cbdb 100644
--- a/git-gui/lib/mergetool.tcl
+++ b/git-gui/lib/mergetool.tcl
@@ -272,8 +272,26 @@ 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 {}} {
+			if {([string first {[} $tool_cmd] != -1) || ([string first {]} $tool_cmd] != -1)} {
+				error_popup [mc "Unable to process square brackets in mergetool.$tool.cmd configuration option.
+
+Please remove the square brackets."]
+				return
+			} else {
+				set cmdline {}
+				foreach command_part $tool_cmd {
+					lappend cmdline [subst -nobackslashes -nocommands $command_part]
+				}
+			}
+		} else {
+			error_popup [mc "Unsupported merge tool '%s'.
+
+To use this tool, configure \"mergetool.%s.cmd\" as shown in the git-config\
+manual page." $tool $tool]
+			return
+		}
 	}
 	}
 

base-commit: c5ee8f2d1c9d336e0a46139bd35236d4a0bb93ff
-- 
gitgitgadget

Re: [PATCH v5] git gui: add directly calling merge tool from configuration

[Johannes Sixt]

Am 12.09.24 um 12:17 schrieb ToBoMi via GitGitGadget:
> Configuration example:
> [merge]
> 	tool = vscode
> [mergetool "vscode"]
> 	path = the/path/to/Code.exe
> 	cmd = \"Code.exe\" --wait --merge \"$LOCAL\" \"$REMOTE\" \"$BASE\" \"$MERGED\"

This example is not up-to-date anymore, is it?

Also, below are two cases where "mergetool.cmd" is mentioned
incorrectly.

> 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. In addtition it also shows a hint to add
> a configuration entry to use the tool as an unsupported tool with degraded
> support.
> 
> If a wrong "mergetool.cmd" is configured by accident, it gets 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.

> --- a/git-gui/lib/mergetool.tcl
> +++ b/git-gui/lib/mergetool.tcl
> @@ -272,8 +272,26 @@ 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 {}} {
> +			if {([string first {[} $tool_cmd] != -1) || ([string first {]} $tool_cmd] != -1)} {
> +				error_popup [mc "Unable to process square brackets in mergetool.$tool.cmd configuration option.

This $tool in the format string breaks [mc]. It must be %s and an
argument. I'll fix this up while queuing.

> +
> +Please remove the square brackets."]
> +				return
> +			} else {
> +				set cmdline {}
> +				foreach command_part $tool_cmd {
> +					lappend cmdline [subst -nobackslashes -nocommands $command_part]
> +				}
> +			}
> +		} else {
> +			error_popup [mc "Unsupported merge tool '%s'.
> +
> +To use this tool, configure \"mergetool.%s.cmd\" as shown in the git-config\> +manual page." $tool $tool]

I am surprised that the backslash does not paste the two lines together
without a space. "git-config" and "manual" do appear as separate words
in the error message. Nevertheless, since I do not know how this pans
out in the translation files, I will remove the line continuation and
write all on one line.

> +			return
> +		}
>  	}
>  	}

Thank you for your contribution! Below is the range-diff between this
submission and the queued version.

-- Hannes

1:  03e92d6 ! 1:  8ff65c7 git gui: add directly calling merge tool from configuration
    @@ Commit message
         [merge]
                 tool = vscode
         [mergetool "vscode"]
    -            path = the/path/to/Code.exe
    -            cmd = \"Code.exe\" --wait --merge \"$LOCAL\" \"$REMOTE\" \"$BASE\" \"$MERGED\"
    +            cmd = \"the/path/to/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. In addtition it also shows a hint to add
    -    a configuration entry to use the tool as an unsupported tool with degraded
    -    support.
    +    Without the "mergetool.<mergetool name>.cmd" entry and an unsupported
    +    "merge.tool" entry, git gui behaves mainly as before this change and
    +    informs the user about an unsupported merge tool. In addtition, it also
    +    shows a hint to add a configuration entry to use the tool as an
    +    unsupported tool with degraded support.
     
    -    If a wrong "mergetool.cmd" is configured by accident, it gets 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.
    +    If a wrong "mergetool.<mergetool name>.cmd" is configured by accident,
    +    it gets 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>
    +    Signed-off-by: Johannes Sixt <j6t@kdbg.org>
     
      ## lib/mergetool.tcl ##
     @@ lib/mergetool.tcl: proc merge_resolve_tool2 {} {
    @@ lib/mergetool.tcl: proc merge_resolve_tool2 {} {
     +		set tool_cmd [get_config mergetool.$tool.cmd]
     +		if {$tool_cmd ne {}} {
     +			if {([string first {[} $tool_cmd] != -1) || ([string first {]} $tool_cmd] != -1)} {
    -+				error_popup [mc "Unable to process square brackets in mergetool.$tool.cmd configuration option.
    ++				error_popup [mc "Unable to process square brackets in \"mergetool.%s.cmd\" configuration option.
     +
    -+Please remove the square brackets."]
    ++Please remove the square brackets." $tool]
     +				return
     +			} else {
     +				set cmdline {}
    @@ lib/mergetool.tcl: proc merge_resolve_tool2 {} {
     +		} else {
     +			error_popup [mc "Unsupported merge tool '%s'.
     +
    -+To use this tool, configure \"mergetool.%s.cmd\" as shown in the git-config\
    -+manual page." $tool $tool]
    ++To use this tool, configure \"mergetool.%s.cmd\" as shown in the git-config manual page." $tool $tool]
     +			return
     +		}
      	}

AW: [PATCH v5] git gui: add directly calling merge tool from configuration

[tobias.boesch]

> -----Ursprüngliche Nachricht-----
> Von: Johannes Sixt <j6t@kdbg.org>
> Gesendet: Samstag, 14. September 2024 15:33
> An: Boesch, Tobias <tobias.boesch@miele.com>
> Cc: git@vger.kernel.org; ToBoMi via GitGitGadget <gitgitgadget@gmail.com>
> Betreff: Re: [PATCH v5] git gui: add directly calling merge tool from
> configuration
>
> Am 12.09.24 um 12:17 schrieb ToBoMi via GitGitGadget:
> > Configuration example:
> > [merge]
> >     tool = vscode
> > [mergetool "vscode"]
> >     path = the/path/to/Code.exe
> >     cmd = \"Code.exe\" --wait --merge \"$LOCAL\" \"$REMOTE\"
> \"$BASE\" \"$MERGED\"
>
> This example is not up-to-date anymore, is it?
>
> Also, below are two cases where "mergetool.cmd" is mentioned incorrectly.
>
> > 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. In addtition it also shows a
> > hint to add a configuration entry to use the tool as an unsupported
> > tool with degraded support.
> >
> > If a wrong "mergetool.cmd" is configured by accident, it gets 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.
>
> > --- a/git-gui/lib/mergetool.tcl
> > +++ b/git-gui/lib/mergetool.tcl
> > @@ -272,8 +272,26 @@ 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 {}} {
> > +                   if {([string first {[} $tool_cmd] != -1) || ([string first {]}
> $tool_cmd] != -1)} {
> > +                           error_popup [mc "Unable to process square
> brackets in mergetool.$tool.cmd configuration option.
>
> This $tool in the format string breaks [mc]. It must be %s and an argument. I'll
> fix this up while queuing.
>
> > +
> > +Please remove the square brackets."]
> > +                           return
> > +                   } else {
> > +                           set cmdline {}
> > +                           foreach command_part $tool_cmd {
> > +                                   lappend cmdline [subst -nobackslashes
> -nocommands $command_part]
> > +                           }
> > +                   }
> > +           } else {
> > +                   error_popup [mc "Unsupported merge tool '%s'.
> > +
> > +To use this tool, configure \"mergetool.%s.cmd\" as shown in the
> > +git-config\> +manual page." $tool $tool]
>
> I am surprised that the backslash does not paste the two lines together
> without a space. "git-config" and "manual" do appear as separate words in the
> error message. Nevertheless, since I do not know how this pans out in the
> translation files, I will remove the line continuation and write all on one line.
>

True I also don't know why.
You could add a whitespace after the newline and have code matching the documentation of tcl:

"\<newline>whiteSpace
A single space character replaces the backslash, newline, and all spaces and tabs after the newline. [...]"
From https://www.tcl.tk/man/tcl8.7/TclCmd/Tcl.html#M24

That doesn't change the error message (stays good) in my tests and makes the code compliant to the tcl docs.

> > +                   return
> > +           }
> >     }
> >     }
>
> Thank you for your contribution! Below is the range-diff between this
> submission and the queued version.
>

Thank you for fixing the issues left open and your patient review.
(Based on your comments and if there is no further notice - I assume that this patch will be processed by your side without further submissions from my side)

> -- Hannes
>
> 1:  03e92d6 ! 1:  8ff65c7 git gui: add directly calling merge tool from
> configuration
>     @@ Commit message
>          [merge]
>                  tool = vscode
>          [mergetool "vscode"]
>     -            path = the/path/to/Code.exe
>     -            cmd = \"Code.exe\" --wait --merge \"$LOCAL\" \"$REMOTE\"
> \"$BASE\" \"$MERGED\"
>     +            cmd = \"the/path/to/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. In addtition it also shows a hint to add
>     -    a configuration entry to use the tool as an unsupported tool with
> degraded
>     -    support.
>     +    Without the "mergetool.<mergetool name>.cmd" entry and an
> unsupported
>     +    "merge.tool" entry, git gui behaves mainly as before this change and
>     +    informs the user about an unsupported merge tool. In addtition, it also
>     +    shows a hint to add a configuration entry to use the tool as an
>     +    unsupported tool with degraded support.
>
>     -    If a wrong "mergetool.cmd" is configured by accident, it gets 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.
>     +    If a wrong "mergetool.<mergetool name>.cmd" is configured by
> accident,
>     +    it gets 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>
>     +    Signed-off-by: Johannes Sixt <j6t@kdbg.org>
>
>       ## lib/mergetool.tcl ##
>      @@ lib/mergetool.tcl: proc merge_resolve_tool2 {} {
>     @@ lib/mergetool.tcl: proc merge_resolve_tool2 {} {
>      +                set tool_cmd [get_config mergetool.$tool.cmd]
>      +                if {$tool_cmd ne {}} {
>      +                        if {([string first {[} $tool_cmd] != -1) || ([string first {]}
> $tool_cmd] != -1)} {
>     -+                                error_popup [mc "Unable to process square
> brackets in mergetool.$tool.cmd configuration option.
>     ++                                error_popup [mc "Unable to process square
> brackets in \"mergetool.%s.cmd\" configuration option.
>      +
>     -+Please remove the square brackets."]
>     ++Please remove the square brackets." $tool]
>      +                                return
>      +                        } else {
>      +                                set cmdline {}
>     @@ lib/mergetool.tcl: proc merge_resolve_tool2 {} {
>      +                } else {
>      +                        error_popup [mc "Unsupported merge tool '%s'.
>      +
>     -+To use this tool, configure \"mergetool.%s.cmd\" as shown in the git-
> config\
>     -+manual page." $tool $tool]
>     ++To use this tool, configure \"mergetool.%s.cmd\" as shown in the git-
> config manual page." $tool $tool]
>      +                        return
>      +                }
>               }



-------------------------------------------------------------------------------------------------
imperial-Werke oHG, Sitz Bünde, Registergericht Bad Oeynhausen - HRA 4825

AW: [PATCH v5] git gui: add directly calling merge tool from configuration

[tobias.boesch]

> -----Ursprüngliche Nachricht-----
> Von: Boesch, Tobias
> Gesendet: Montag, 16. September 2024 10:42
> An: Johannes Sixt <j6t@kdbg.org>
> Cc: git@vger.kernel.org; ToBoMi via GitGitGadget <gitgitgadget@gmail.com>
> Betreff: AW: [PATCH v5] git gui: add directly calling merge tool from
> configuration
>
>
>
> > -----Ursprüngliche Nachricht-----
> > Von: Johannes Sixt <j6t@kdbg.org>
> > Gesendet: Samstag, 14. September 2024 15:33
> > An: Boesch, Tobias <tobias.boesch@miele.com>
> > Cc: git@vger.kernel.org; ToBoMi via GitGitGadget
> > <gitgitgadget@gmail.com>
> > Betreff: Re: [PATCH v5] git gui: add directly calling merge tool from
> > configuration
> >
> > Am 12.09.24 um 12:17 schrieb ToBoMi via GitGitGadget:
> > > Configuration example:
> > > [merge]
> > >   tool = vscode
> > > [mergetool "vscode"]
> > >   path = the/path/to/Code.exe
> > >   cmd = \"Code.exe\" --wait --merge \"$LOCAL\" \"$REMOTE\"
> > \"$BASE\" \"$MERGED\"
> >
> > This example is not up-to-date anymore, is it?
> >
> > Also, below are two cases where "mergetool.cmd" is mentioned incorrectly.
> >
> > > 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. In addtition it also shows a
> > > hint to add a configuration entry to use the tool as an unsupported
> > > tool with degraded support.
> > >
> > > If a wrong "mergetool.cmd" is configured by accident, it gets
> > > 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.
> >
> > > --- a/git-gui/lib/mergetool.tcl
> > > +++ b/git-gui/lib/mergetool.tcl
> > > @@ -272,8 +272,26 @@ 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 {}} {
> > > +                 if {([string first {[} $tool_cmd] != -1) || ([string first {]}
> > $tool_cmd] != -1)} {
> > > +                         error_popup [mc "Unable to process square
> > brackets in mergetool.$tool.cmd configuration option.
> >
> > This $tool in the format string breaks [mc]. It must be %s and an
> > argument. I'll fix this up while queuing.
> >
> > > +
> > > +Please remove the square brackets."]
> > > +                         return
> > > +                 } else {
> > > +                         set cmdline {}
> > > +                         foreach command_part $tool_cmd {
> > > +                                 lappend cmdline [subst -nobackslashes
> > -nocommands $command_part]
> > > +                         }
> > > +                 }
> > > +         } else {
> > > +                 error_popup [mc "Unsupported merge tool '%s'.
> > > +
> > > +To use this tool, configure \"mergetool.%s.cmd\" as shown in the
> > > +git-config\> +manual page." $tool $tool]
> >
> > I am surprised that the backslash does not paste the two lines
> > together without a space. "git-config" and "manual" do appear as
> > separate words in the error message. Nevertheless, since I do not know
> > how this pans out in the translation files, I will remove the line continuation
> and write all on one line.
> >
>
> True I also don't know why.
> You could add a whitespace after the newline and have code matching the
> documentation of tcl:
>
> "\<newline>whiteSpace
> A single space character replaces the backslash, newline, and all spaces and
> tabs after the newline. [...]"
> From https://www.tcl.tk/man/tcl8.7/TclCmd/Tcl.html#M24
>
> That doesn't change the error message (stays good) in my tests and makes the
> code compliant to the tcl docs.
>
> > > +                 return
> > > +         }
> > >   }
> > >   }
> >
> > Thank you for your contribution! Below is the range-diff between this
> > submission and the queued version.
> >
>
> Thank you for fixing the issues left open and your patient review.
> (Based on your comments and if there is no further notice - I assume that this
> patch will be processed by your side without further submissions from my
> side)

I monitored the git repository at https://github.com/git/git.git and up to today I was unable to find this change in any other branches than the ones I've pushed.
The review of this change is finished as far as I understand.
The documentation (https://git-scm.com/docs/MyFirstContribution#after-approval) says that my "change will be placed into seen fairly early on by the maintainer while it is still in the review process".
Since I cannot find it in seen or anywhere else, I wonder if there is something wrong, if it just takes a little longer than I expected it to be merged or if this change is merged somewhere else.

Can someone help me understanding this?

Tobias
>
> > -- Hannes
> >
> > 1:  03e92d6 ! 1:  8ff65c7 git gui: add directly calling merge tool
> > from configuration
> >     @@ Commit message
> >          [merge]
> >                  tool = vscode
> >          [mergetool "vscode"]
> >     -            path = the/path/to/Code.exe
> >     -            cmd = \"Code.exe\" --wait --merge \"$LOCAL\" \"$REMOTE\"
> > \"$BASE\" \"$MERGED\"
> >     +            cmd = \"the/path/to/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. In addtition it also shows a hint to
> add
> >     -    a configuration entry to use the tool as an unsupported tool with
> > degraded
> >     -    support.
> >     +    Without the "mergetool.<mergetool name>.cmd" entry and an
> > unsupported
> >     +    "merge.tool" entry, git gui behaves mainly as before this change and
> >     +    informs the user about an unsupported merge tool. In addtition, it also
> >     +    shows a hint to add a configuration entry to use the tool as an
> >     +    unsupported tool with degraded support.
> >
> >     -    If a wrong "mergetool.cmd" is configured by accident, it gets 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.
> >     +    If a wrong "mergetool.<mergetool name>.cmd" is configured by
> > accident,
> >     +    it gets 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>
> >     +    Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> >
> >       ## lib/mergetool.tcl ##
> >      @@ lib/mergetool.tcl: proc merge_resolve_tool2 {} {
> >     @@ lib/mergetool.tcl: proc merge_resolve_tool2 {} {
> >      +              set tool_cmd [get_config mergetool.$tool.cmd]
> >      +              if {$tool_cmd ne {}} {
> >      +                      if {([string first {[} $tool_cmd] != -1) || ([string first {]}
> > $tool_cmd] != -1)} {
> >     -+                              error_popup [mc "Unable to process square
> > brackets in mergetool.$tool.cmd configuration option.
> >     ++                              error_popup [mc "Unable to process square
> > brackets in \"mergetool.%s.cmd\" configuration option.
> >      +
> >     -+Please remove the square brackets."]
> >     ++Please remove the square brackets." $tool]
> >      +                              return
> >      +                      } else {
> >      +                              set cmdline {}
> >     @@ lib/mergetool.tcl: proc merge_resolve_tool2 {} {
> >      +              } else {
> >      +                      error_popup [mc "Unsupported merge tool '%s'.
> >      +
> >     -+To use this tool, configure \"mergetool.%s.cmd\" as shown in the
> > git- config\
> >     -+manual page." $tool $tool]
> >     ++To use this tool, configure \"mergetool.%s.cmd\" as shown in the
> > git- config manual page." $tool $tool]
> >      +                      return
> >      +              }
> >             }



-------------------------------------------------------------------------------------------------
imperial-Werke oHG, Sitz Bünde, Registergericht Bad Oeynhausen - HRA 4825

Re: [PATCH v5] git gui: add directly calling merge tool from configuration

[Johannes Sixt]

Am 07.11.24 um 15:16 schrieb tobias.boesch@miele.com:
> The documentation (https://git-scm.com/docs/ 
> MyFirstContribution#after-approval) says that my "change will be 
> placed into seen fairly early on by the maintainer while it is still 
> in the review process".
This document is only about contributions to git.git. Git-GUI has its
own repository and workflow.

> Since I cannot find it in seen or anywhere else, I wonder if there
> is something wrong, if it just takes a little longer than I expected
> it to be merged or if this change is merged somewhere else.
There is nothing wrong, except that it's on me now to submit a pull
request to the Git maintainer. This should happen in the next days.

-- Hannes