diff mbox series

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

Message ID pull.1773.v5.git.1726136277300.gitgitgadget@gmail.com (mailing list archive)
State New
Headers show
Series [v5] git gui: add directly calling merge tool from configuration | expand

Commit Message

tobias.boesch@miele.com Sept. 12, 2024, 10:17 a.m. UTC 
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(-)


base-commit: c5ee8f2d1c9d336e0a46139bd35236d4a0bb93ff

Comments

Johannes Sixt Sept. 14, 2024, 1:32 p.m. UTC | #1 
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
     +		}
      	}
tobias.boesch@miele.com Sept. 16, 2024, 8:42 a.m. UTC | #2 
> -----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
diff mbox series

Patch

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