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

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