From: tobias.boesch@miele.com Date: Mon, 16 Sep 2024 08:42:06 GMT Subject: AW: [PATCH v5] git gui: add directly calling merge tool from configuration Message-ID: In-Reply-To: <2ee3a148-23eb-48cb-8e10-551437fb03d3@kdbg.org> > -----Ursprüngliche Nachricht----- > Von: Johannes Sixt > Gesendet: Samstag, 14. September 2024 15:33 > An: Boesch, Tobias > Cc: git@vger.kernel.org; ToBoMi via GitGitGadget > 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: "\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..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..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 > + Signed-off-by: Johannes Sixt > > ## 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