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