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

From: Johannes Sixt <j6t@kdbg.org>
Date: Sep 14, 2024, 13:32 UTC
Message-ID: <2ee3a148-23eb-48cb-8e10-551437fb03d3@kdbg.org>
In-Reply-To: <pull.1773.v5.git.1726136277300.gitgitgadget@gmail.com>

Am 12.09.24 um 12:17 schrieb ToBoMi via GitGitGadget:

Show 6 quoted lines

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

Show 10 quoted lines

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

Show 12 quoted lines

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

Show 13 quoted lines

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