{"thread":{"id":"61971","subject":"[PATCH] git gui: add directly calling merge tool from gitconfig","startedAt":"2024-08-19T11:29:08Z","lastAt":"2024-11-07T16:43:48Z","messageCount":19,"participants":["ToBoMi via GitGitGadget","Johannes Sixt","tobias.boesch@miele.com","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"501265","messageId":"pull.1773.git.1724066944786.gitgitgadget@gmail.com","threadId":"61971","inReplyTo":null,"subject":"[PATCH] git gui: add directly calling merge tool from gitconfig","fromName":"ToBoMi via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-08-19T11:29:04Z","receivedAt":"2024-08-19T11:29:08Z","isPatch":true,"sender":{"key":"name:ToBoMi","avatar":null},"body":"From: deboeto \n\n* git Gui can open a merge tool when conflicts are\n detected. The merge tools that are allowed to\n call have to be hard coded into git Gui\n althgough there are configuration options for\n merge tools git in the git config. Git calls\n the configured merge tools directly from the\n config while git Gui doesn't.\n* git Gui can now call the tool configured in the\n gitconfig directly.\n* Can be enabled through setting\n gui.mergeToolFromConfig\n* Disabled by default, since option is most likely\n never set\n* bc3 and vscode tested\n\nSigned-off-by: deboeto \n---\n git gui: add directly calling merge tool from gitconfig\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1773%2FToBoMi%2Fadd_merge_tool_from_config_file-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1773/ToBoMi/add_merge_tool_from_config_file-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1773\n\n Documentation/config/gui.txt | 4 ++++\n git-gui/lib/mergetool.tcl | 11 +++++++++--\n 2 files changed, 13 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/config/gui.txt b/Documentation/config/gui.txt\nindex 171be774d24..e63d0b46e7c 100644\n--- a/Documentation/config/gui.txt\n+++ b/Documentation/config/gui.txt\n@@ -55,3 +55,7 @@ gui.blamehistoryctx::\n \tlinkgit:gitk[1] for the selected commit, when the `Show History\n \tContext` menu item is invoked from 'git gui blame'. If this\n \tvariable is set to zero, the whole history is shown.\n+\n+gui.mergeToolFromConfig::\n+\tIf true, allow to call the merge tool configured in gitconfig\n+\tin git gui directly.\n\\ No newline at end of file\ndiff --git a/git-gui/lib/mergetool.tcl b/git-gui/lib/mergetool.tcl\nindex e688b016ef6..fbd0889612a 100644\n--- a/git-gui/lib/mergetool.tcl\n+++ b/git-gui/lib/mergetool.tcl\n@@ -272,8 +272,15 @@ proc merge_resolve_tool2 {} {\n \t\t}\n \t}\n \tdefault {\n-\t\terror_popup [mc \"Unsupported merge tool '%s'\" $tool]\n-\t\treturn\n+\t\tif {[is_config_true gui.mergetoolfromconfig]} {\n+\t\t\tset path [get_config mergetool.$tool.path]\n+\t\t\tset cmdline_config [get_config mergetool.$tool.cmd]\n+\t\t\tset cmdline_substituted [subst -nobackslashes -nocommands $cmdline_config]\n+\t\t\tset cmdline [lreplace $cmdline_substituted 0 0 $path]\n+\t\t} else {\n+\t\t\terror_popup [mc \"Unsupported merge tool '%s'\" $tool]\n+\t\t\treturn\n+\t\t}\n \t}\n \t}\n \n\nbase-commit: b9849e4f7631d80f146d159bf7b60263b3205632\n-- \ngitgitgadget\n"},{"id":"501625","messageId":"57d00f50-c652-4357-bf9b-02b93f99cfb5@kdbg.org","threadId":"61971","inReplyTo":"pull.1773.git.1724066944786.gitgitgadget@gmail.com","subject":"Re: [PATCH] git gui: add directly calling merge tool from gitconfig","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2024-08-24T13:38:23Z","receivedAt":"2024-08-24T14:13:03Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 19.08.24 um 13:29 schrieb ToBoMi via GitGitGadget:\n> From: deboeto \n> \n> * git Gui can open a merge tool when conflicts are\n> detected. The merge tools that are allowed to\n> call have to be hard coded into git Gui\n> althgough there are configuration options for\n> merge tools git in the git config. Git calls\n> the configured merge tools directly from the\n> config while git Gui doesn't.\n> * git Gui can now call the tool configured in the\n> gitconfig directly.\n> * Can be enabled through setting\n> gui.mergeToolFromConfig\n\nCan we do better than having a new configuration variable? Let's say you\nhave configured merge.tool=vscode. This tool is not supported, but you\nhave configured mergetool.vscode.cmd suitably. Can we not use the latter\nconfiguration variable unconditionally?\n\nLikewise, say, you have configured merge.tool=bc3. This one *is*\nsupported. What could go wrong if mergetool.bc3.cmd is used instead of\nthe built-in command line? The behavior would change for users that\nconfigured mergetool.$tool.cmd for a supported tool. But would it change\nfor the worse?\n\nBTW, the code builds different command lines depending on whether a base\nfile is available or not. How does mergetool.$tool.cmd handle the cases?\n\n> * Disabled by default, since option is most likely\n> never set\n> * bc3 and vscode tested\n> \n> Signed-off-by: deboeto \n\nSome remarks on the commit message:\n\n- The Signed-off-by line has legal consequences. Therefore, we require\nthat authors use their genuine name, not an alias. Also, the From line\nmust match the Signed-off-by line.\n\n- Please have a look at the commit messages in the code base. The\nformatting presented here is very unusual. Please write in full\nsentences including punctuation, and use paragraphs where needed.\n\n- Please state the problem that is being solved (in present tense). This\nshould motivate the change, i.e., provide a convincing argument why the\nchange is needed. Then state what the solution is in imperative mood,\nthat is, an instruction to the code to change in such and such way. Use\nexamples to clarify how the new feature can be used.\n\n> ---\n> git gui: add directly calling merge tool from gitconfig\n> \n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1773%2FToBoMi%2Fadd_merge_tool_from_config_file-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1773/ToBoMi/add_merge_tool_from_config_file-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1773\n> \n> Documentation/config/gui.txt | 4 ++++\n> git-gui/lib/mergetool.tcl | 11 +++++++++--\n> 2 files changed, 13 insertions(+), 2 deletions(-)\n> \n> diff --git a/Documentation/config/gui.txt b/Documentation/config/gui.txt\n> index 171be774d24..e63d0b46e7c 100644\n> --- a/Documentation/config/gui.txt\n> +++ b/Documentation/config/gui.txt\n> @@ -55,3 +55,7 @@ gui.blamehistoryctx::\n> \tlinkgit:gitk[1] for the selected commit, when the `Show History\n> \tContext` menu item is invoked from 'git gui blame'. If this\n> \tvariable is set to zero, the whole history is shown.\n> +\n> +gui.mergeToolFromConfig::\n> +\tIf true, allow to call the merge tool configured in gitconfig\n> +\tin git gui directly.\n> \\ No newline at end of file\n\nUnfortunately, Documentation/config/gui.txt is not part of the Git GUI\nrepository. Any changes to the documentation must be submitted as\nseparate patch.\n\nPlease be careful not to introduce an incomplete last lines. Take note\nof \"No newline at end of file\". It should not be there.\n\n> diff --git a/git-gui/lib/mergetool.tcl b/git-gui/lib/mergetool.tcl\n> index e688b016ef6..fbd0889612a 100644\n> --- a/git-gui/lib/mergetool.tcl\n> +++ b/git-gui/lib/mergetool.tcl\n> @@ -272,8 +272,15 @@ proc merge_resolve_tool2 {} {\n> \t\t}\n> \t}\n> \tdefault {\n> -\t\terror_popup [mc \"Unsupported merge tool '%s'\" $tool]\n> -\t\treturn\n> +\t\tif {[is_config_true gui.mergetoolfromconfig]} {\n> +\t\t\tset path [get_config mergetool.$tool.path]\n\nAt this point, the value assigned to $path here is already available in\n$merge_tool_path.\n\n> +\t\t\tset cmdline_config [get_config mergetool.$tool.cmd]\n> +\t\t\tset cmdline_substituted [subst -nobackslashes -nocommands $cmdline_config]\n> +\t\t\tset cmdline [lreplace $cmdline_substituted 0 0 $path]\n\nI haven't yet taken the time to study what these lines do (I am far from\nfluent in Tcl) and have no opinion, yet.\n\n> +\t\t} else {\n> +\t\t\terror_popup [mc \"Unsupported merge tool '%s'\" $tool]\n> +\t\t\treturn\n> +\t\t}\n> \t}\n> \t}\n> \n> \n> base-commit: b9849e4f7631d80f146d159bf7b60263b3205632\n\n-- Hannes\n\n"},{"id":"501716","messageId":"AS2PR08MB8288FB2EA2B108A5EBAC86D7E1942@AS2PR08MB8288.eurprd08.prod.outlook.com","threadId":"61971","inReplyTo":"57d00f50-c652-4357-bf9b-02b93f99cfb5@kdbg.org","subject":"AW: [PATCH] git gui: add directly calling merge tool from gitconfig","fromName":"tobias.boesch@miele.com","fromEmail":"tobias.boesch@miele.com","sentAt":"2024-08-27T12:51:34Z","receivedAt":"2024-08-27T12:52:02Z","isPatch":true,"sender":{"key":"tobias.boesch@miele.com","avatar":"https://avatars.githubusercontent.com/u/64197724?v=4"},"body":"> -----Ursprüngliche Nachricht-----\n> Von: Johannes Sixt \n> Gesendet: Samstag, 24. August 2024 15:38\n> An: Boesch, Tobias \n> Cc: git@vger.kernel.org; ToBoMi via GitGitGadget \n> Betreff: Re: [PATCH] git gui: add directly calling merge tool from gitconfig\n>\n\nThanks for the review.\n\n> Am 19.08.24 um 13:29 schrieb ToBoMi via GitGitGadget:\n> > From: deboeto \n> >\n> > * git Gui can open a merge tool when conflicts are\n> > detected. The merge tools that are allowed to\n> > call have to be hard coded into git Gui\n> > althgough there are configuration options for\n> > merge tools git in the git config. Git calls\n> > the configured merge tools directly from the\n> > config while git Gui doesn't.\n> > * git Gui can now call the tool configured in the\n> > gitconfig directly.\n> > * Can be enabled through setting\n> > gui.mergeToolFromConfig\n>\n> Can we do better than having a new configuration variable? Let's say you have\n> configured merge.tool=vscode. This tool is not supported, but you have\n> configured mergetool.vscode.cmd suitably. Can we not use the latter\n> configuration variable unconditionally?\n>\n\nI think that would work. I'll change the patch to use the mergetool.cmd variable\nas the trigger for using the configured merge tool.\nIf the mergetool configuration option is set to a supported tool\n(merge.tool = vscode) it will cause git gui to use the supported tool (hard coded\ninto the source code - as before this change).\nIf the mergetool configuration option is set to an UNsupported tool and\nmergetool.cmd is set for the chosen mergetool it will use the command from\nthat option.\n\nPatch will be submitted soon.\n\n> Likewise, say, you have configured merge.tool=bc3. This one *is* supported.\n> What could go wrong if mergetool.bc3.cmd is used instead of the built-in\n> command line? The behavior would change for users that configured\n> mergetool.$tool.cmd for a supported tool. But would it change for the worse?\n>\n\nI tested this together with the change of the activating option mentioned before.\nWith that in mind I cannot create a case were the mergetool.cmd is used on a\nsupported tool, because mergetool.cmd is only used for UNsupported tools.\n1. Assuming the merge.tool is set to a supported tool:\n - In this case the supported tool is used, no matter if the mergetool.cmd\n is set or not\n2. Assuming the merge.tool is set to an UNsupported tool:\n - Then the variable IS evaluated\n - If it is set to an invalid command or a wrong mergetool.path is given,\n Git gui will complain as before this change that the command is not\n Found in PATH\n\n> BTW, the code builds different command lines depending on whether a base\n> file is available or not. How does mergetool.$tool.cmd handle the cases?\n>\n\nCurrently it doesn't.\nI don't know if it should, because I guess that git also has no other possibility\nthan to call this command for a merge unconditionally - even when the base file\nname is empty.\nI haven't had such a case that I can remember. How can it be triggered?\nDoesn't all merges have a common ancestor as long as the histories are related?\n\n> > * Disabled by default, since option is most likely\n> > never set\n> > * bc3 and vscode tested\n> >\n> > Signed-off-by: deboeto \n>\n> Some remarks on the commit message:\n>\n> - The Signed-off-by line has legal consequences. Therefore, we require that\n> authors use their genuine name, not an alias. Also, the From line must match\n> the Signed-off-by line.\n>\n> - Please have a look at the commit messages in the code base. The formatting\n> presented here is very unusual. Please write in full sentences including\n> punctuation, and use paragraphs where needed.\n>\n> - Please state the problem that is being solved (in present tense). This should\n> motivate the change, i.e., provide a convincing argument why the change is\n> needed. Then state what the solution is in imperative mood, that is, an\n> instruction to the code to change in such and such way. Use examples to\n> clarify how the new feature can be used.\n\nCorrected - please review again.\n\n>\n> > ---\n> > git gui: add directly calling merge tool from gitconfig\n> >\n> > Published-As:\n> > https://github.com/gitgitgadget/git/releases/tag/pr-\n> 1773%2FToBoMi%2Fad\n> > d_merge_tool_from_config_file-v1\n> > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git\n> > pr-1773/ToBoMi/add_merge_tool_from_config_file-v1\n> > Pull-Request: https://github.com/gitgitgadget/git/pull/1773\n> >\n> > Documentation/config/gui.txt | 4 ++++\n> > git-gui/lib/mergetool.tcl | 11 +++++++++--\n> > 2 files changed, 13 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/Documentation/config/gui.txt\n> > b/Documentation/config/gui.txt index 171be774d24..e63d0b46e7c\n> 100644\n> > --- a/Documentation/config/gui.txt\n> > +++ b/Documentation/config/gui.txt\n> > @@ -55,3 +55,7 @@ gui.blamehistoryctx::\n> > linkgit:gitk[1] for the selected commit, when the `Show History\n> > Context` menu item is invoked from 'git gui blame'. If this\n> > variable is set to zero, the whole history is shown.\n> > +\n> > +gui.mergeToolFromConfig::\n> > + If true, allow to call the merge tool configured in gitconfig\n> > + in git gui directly.\n> > \\ No newline at end of file\n>\n> Unfortunately, Documentation/config/gui.txt is not part of the Git GUI\n> repository. Any changes to the documentation must be submitted as separate\n> patch.\n>\n> Please be careful not to introduce an incomplete last lines. Take note of \"No\n> newline at end of file\". It should not be there.\n>\n> > diff --git a/git-gui/lib/mergetool.tcl b/git-gui/lib/mergetool.tcl\n> > index e688b016ef6..fbd0889612a 100644\n> > --- a/git-gui/lib/mergetool.tcl\n> > +++ b/git-gui/lib/mergetool.tcl\n> > @@ -272,8 +272,15 @@ proc merge_resolve_tool2 {} {\n> > }\n> > }\n> > default {\n> > - error_popup [mc \"Unsupported merge tool '%s'\" $tool]\n> > - return\n> > + if {[is_config_true gui.mergetoolfromconfig]} {\n> > + set path [get_config mergetool.$tool.path]\n>\n> At this point, the value assigned to $path here is already available in\n> $merge_tool_path.\n>\n> > + set cmdline_config [get_config mergetool.$tool.cmd]\n> > + set cmdline_substituted [subst -nobackslashes -\n> nocommands $cmdline_config]\n> > + set cmdline [lreplace $cmdline_substituted 0 0 $path]\n>\n> I haven't yet taken the time to study what these lines do (I am far from fluent\n> in Tcl) and have no opinion, yet.\n>\n> > + } else {\n> > + error_popup [mc \"Unsupported merge tool '%s'\"\n> $tool]\n> > + return\n> > + }\n> > }\n> > }\n> >\n> >\n> > base-commit: b9849e4f7631d80f146d159bf7b60263b3205632\n>\n> -- Hannes\n\n\n\n-------------------------------------------------------------------------------------------------\nimperial-Werke oHG, Sitz Bünde, Registergericht Bad Oeynhausen - HRA 4825\n"},{"id":"501717","messageId":"AS2PR08MB8288FC89C781619E82C2A96BE1942@AS2PR08MB8288.eurprd08.prod.outlook.com","threadId":"61971","inReplyTo":"57d00f50-c652-4357-bf9b-02b93f99cfb5@kdbg.org","subject":"AW: [PATCH] git gui: add directly calling merge tool from gitconfig","fromName":"tobias.boesch@miele.com","fromEmail":"tobias.boesch@miele.com","sentAt":"2024-08-27T13:53:13Z","receivedAt":"2024-08-27T13:54:03Z","isPatch":true,"sender":{"key":"tobias.boesch@miele.com","avatar":"https://avatars.githubusercontent.com/u/64197724?v=4"},"body":"> -----Ursprüngliche Nachricht-----\n> Von: Johannes Sixt \n> Gesendet: Samstag, 24. August 2024 15:38\n> An: Boesch, Tobias \n> Cc: git@vger.kernel.org; ToBoMi via GitGitGadget \n> Betreff: Re: [PATCH] git gui: add directly calling merge tool from gitconfig\n>\n\nContinuing my incomplete last reply:\n\n> Am 19.08.24 um 13:29 schrieb ToBoMi via GitGitGadget:\n> > From: deboeto \n> >\n> > * git Gui can open a merge tool when conflicts are\n> > detected. The merge tools that are allowed to\n> > call have to be hard coded into git Gui\n> > althgough there are configuration options for\n> > merge tools git in the git config. Git calls\n> > the configured merge tools directly from the\n> > config while git Gui doesn't.\n> > * git Gui can now call the tool configured in the\n> > gitconfig directly.\n> > * Can be enabled through setting\n> > gui.mergeToolFromConfig\n>\n> Can we do better than having a new configuration variable? Let's say you have\n> configured merge.tool=vscode. This tool is not supported, but you have\n> configured mergetool.vscode.cmd suitably. Can we not use the latter\n> configuration variable unconditionally?\n>\n> Likewise, say, you have configured merge.tool=bc3. This one *is* supported.\n> What could go wrong if mergetool.bc3.cmd is used instead of the built-in\n> command line? The behavior would change for users that configured\n> mergetool.$tool.cmd for a supported tool. But would it change for the worse?\n>\n> BTW, the code builds different command lines depending on whether a base\n> file is available or not. How does mergetool.$tool.cmd handle the cases?\n>\n> > * Disabled by default, since option is most likely\n> > never set\n> > * bc3 and vscode tested\n> >\n> > Signed-off-by: deboeto \n>\n> Some remarks on the commit message:\n>\n> - The Signed-off-by line has legal consequences. Therefore, we require that\n> authors use their genuine name, not an alias. Also, the From line must match\n> the Signed-off-by line.\n>\n> - Please have a look at the commit messages in the code base. The formatting\n> presented here is very unusual. Please write in full sentences including\n> punctuation, and use paragraphs where needed.\n>\n> - Please state the problem that is being solved (in present tense). This should\n> motivate the change, i.e., provide a convincing argument why the change is\n> needed. Then state what the solution is in imperative mood, that is, an\n> instruction to the code to change in such and such way. Use examples to\n> clarify how the new feature can be used.\n>\n> > ---\n> > git gui: add directly calling merge tool from gitconfig\n> >\n> > Published-As:\n> > https://github.com/gitgitgadget/git/releases/tag/pr-\n> 1773%2FToBoMi%2Fad\n> > d_merge_tool_from_config_file-v1\n> > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git\n> > pr-1773/ToBoMi/add_merge_tool_from_config_file-v1\n> > Pull-Request: https://github.com/gitgitgadget/git/pull/1773\n> >\n> > Documentation/config/gui.txt | 4 ++++\n> > git-gui/lib/mergetool.tcl | 11 +++++++++--\n> > 2 files changed, 13 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/Documentation/config/gui.txt\n> > b/Documentation/config/gui.txt index 171be774d24..e63d0b46e7c\n> 100644\n> > --- a/Documentation/config/gui.txt\n> > +++ b/Documentation/config/gui.txt\n> > @@ -55,3 +55,7 @@ gui.blamehistoryctx::\n> > linkgit:gitk[1] for the selected commit, when the `Show History\n> > Context` menu item is invoked from 'git gui blame'. If this\n> > variable is set to zero, the whole history is shown.\n> > +\n> > +gui.mergeToolFromConfig::\n> > + If true, allow to call the merge tool configured in gitconfig\n> > + in git gui directly.\n> > \\ No newline at end of file\n>\n> Unfortunately, Documentation/config/gui.txt is not part of the Git GUI\n> repository. Any changes to the documentation must be submitted as separate\n> patch.\n>\n\nConfiguration option will be removed in the next patch version. Therefore the\ndocumentation change is no longer needed.\n\n> Please be careful not to introduce an incomplete last lines. Take note of \"No\n> newline at end of file\". It should not be there.\n>\n\nSee above.\n\n> > diff --git a/git-gui/lib/mergetool.tcl b/git-gui/lib/mergetool.tcl\n> > index e688b016ef6..fbd0889612a 100644\n> > --- a/git-gui/lib/mergetool.tcl\n> > +++ b/git-gui/lib/mergetool.tcl\n> > @@ -272,8 +272,15 @@ proc merge_resolve_tool2 {} {\n> > }\n> > }\n> > default {\n> > - error_popup [mc \"Unsupported merge tool '%s'\" $tool]\n> > - return\n> > + if {[is_config_true gui.mergetoolfromconfig]} {\n> > + set path [get_config mergetool.$tool.path]\n>\n> At this point, the value assigned to $path here is already available in\n> $merge_tool_path.\n>\n\nTrue - corrected in the next patch.\n\n> > + set cmdline_config [get_config mergetool.$tool.cmd]\n> > + set cmdline_substituted [subst -nobackslashes -\n> nocommands $cmdline_config]\n> > + set cmdline [lreplace $cmdline_substituted 0 0 $path]\n>\n> I haven't yet taken the time to study what these lines do (I am far from fluent\n> in Tcl) and have no opinion, yet.\n>\n\nThey replace the variables one can put into mergetool.cmd like $REMOTE or\n$LOCAL.\nWithout this substitution command they are not replaced with the real file\npaths.\nSee this example for vscode:\ncmd = \\\"Code.exe\\\" --wait --merge \\\"$LOCAL\\\" \\\"$REMOTE\\\" \\\"$BASE\\\" \\\"$MERGED\\\"\n\n> > + } else {\n> > + error_popup [mc \"Unsupported merge tool '%s'\"\n> $tool]\n> > + return\n> > + }\n> > }\n> > }\n> >\n> >\n> > base-commit: b9849e4f7631d80f146d159bf7b60263b3205632\n>\n> -- Hannes\n\n\n\n-------------------------------------------------------------------------------------------------\nimperial-Werke oHG, Sitz Bünde, Registergericht Bad Oeynhausen - HRA 4825\n"},{"id":"501757","messageId":"pull.1773.v2.git.1724833917245.gitgitgadget@gmail.com","threadId":"61971","inReplyTo":"pull.1773.git.1724066944786.gitgitgadget@gmail.com","subject":"[PATCH v2] git gui: add directly calling merge tool from gitconfig","fromName":"ToBoMi via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-08-28T08:31:56Z","receivedAt":"2024-08-28T08:32:02Z","isPatch":true,"sender":{"key":"name:ToBoMi","avatar":null},"body":"From: deboeto \n\ngit gui can open a merge tool when conflicts are\ndetected (Right click in the diff of the file with\nconflicts).\nThe merge tools that are allowed to\nuse are hard coded into git gui.\n\nIf one wants to add a new merge tool it has to be\nadded to git gui through a source code change.\nThis is not convenient in comparison to how it\nworks in git (without gui).\n\ngit itself has configuration options for a merge tools\npath and command in the git config.\nNew merge tools can be set up there without a\nsource code change.\n\nThose options are used only by pure git in\ncontrast to git gui. git calls the configured\nmerge tools directly from the config while git\nGui doesn't.\n\nWith this change git gui can call merge tools\nconfigured in the gitconfig directly without a\nchange in git gui source code.\nIt needs a configured merge.tool and a configured\nmergetool.cmd config entry.\n\ngitconfig example:\n[merge]\n\ttool = vscode\n[mergetool \"vscode\"]\n\tpath = the/path/to/Code.exe\n\tcmd = \\\"Code.exe\\\" --wait --merge \\\"$LOCAL\\\" \\\"$REMOTE\\\" \\\"$BASE\\\" \\\"$MERGED\\\"\n\nWithout the mergetool.cmd configuration and an\nunsupported merge.tool entry, git gui behaves\nmainly as before this change and informs the user\nabout an unsupported merge tool, but now also\nshows a hint to add a config entry for the tool\nin gitconfig.\n\nIf a wrong mergetool.cmd is configured by accident\nit is beeing handled by git gui already. In this\ncase git gui informs the user that the merge tool\ncouldn't be opened. This behavior is preserved by\nthis change and should not change.\n\nBeyond compare 3 and Visual Studio code were\ntested as manually configured merge tools.\n\nSigned-off-by: Tobias Boesch \n---\n git gui: add directly calling merge tool from gitconfig\n \n cc: Johannes Sixt j6t@kdbg.org\n \n Changes since v1:\n \n * Used existing option mergetool.cmd in gitconfig to trigger the direct\n call of the merge tool configured in the config instead adding a new\n option mergeToolFromConfig\n * Removed assignment of merge tool path to a variable and reused the\n already existing one: merget_tool_path\n * Changed formatting of the commit message\n * Added more context and an examples to the commit message\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1773%2FToBoMi%2Fadd_merge_tool_from_config_file-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1773/ToBoMi/add_merge_tool_from_config_file-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1773\n\nRange-diff vs v1:\n\n 1: 59e8f454a70 ! 1: e77d6dec6c5 git gui: add directly calling merge tool from gitconfig\n @@ Metadata\n ## Commit message ##\n git gui: add directly calling merge tool from gitconfig\n \n - * git Gui can open a merge tool when conflicts are\n - detected. The merge tools that are allowed to\n - call have to be hard coded into git Gui\n - althgough there are configuration options for\n - merge tools git in the git config. Git calls\n - the configured merge tools directly from the\n - config while git Gui doesn't.\n - * git Gui can now call the tool configured in the\n - gitconfig directly.\n - * Can be enabled through setting\n - gui.mergeToolFromConfig\n - * Disabled by default, since option is most likely\n - never set\n - * bc3 and vscode tested\n -\n - Signed-off-by: deboeto \n -\n - ## Documentation/config/gui.txt ##\n -@@ Documentation/config/gui.txt: gui.blamehistoryctx::\n - \tlinkgit:gitk[1] for the selected commit, when the `Show History\n - \tContext` menu item is invoked from 'git gui blame'. If this\n - \tvariable is set to zero, the whole history is shown.\n -+\n -+gui.mergeToolFromConfig::\n -+\tIf true, allow to call the merge tool configured in gitconfig\n -+\tin git gui directly.\n - \\ No newline at end of file\n + git gui can open a merge tool when conflicts are\n + detected (Right click in the diff of the file with\n + conflicts).\n + The merge tools that are allowed to\n + use are hard coded into git gui.\n +\n + If one wants to add a new merge tool it has to be\n + added to git gui through a source code change.\n + This is not convenient in comparison to how it\n + works in git (without gui).\n +\n + git itself has configuration options for a merge tools\n + path and command in the git config.\n + New merge tools can be set up there without a\n + source code change.\n +\n + Those options are used only by pure git in\n + contrast to git gui. git calls the configured\n + merge tools directly from the config while git\n + Gui doesn't.\n +\n + With this change git gui can call merge tools\n + configured in the gitconfig directly without a\n + change in git gui source code.\n + It needs a configured merge.tool and a configured\n + mergetool.cmd config entry.\n +\n + gitconfig example:\n + [merge]\n + tool = vscode\n + [mergetool \"vscode\"]\n + path = the/path/to/Code.exe\n + cmd = \\\"Code.exe\\\" --wait --merge \\\"$LOCAL\\\" \\\"$REMOTE\\\" \\\"$BASE\\\" \\\"$MERGED\\\"\n +\n + Without the mergetool.cmd configuration and an\n + unsupported merge.tool entry, git gui behaves\n + mainly as before this change and informs the user\n + about an unsupported merge tool, but now also\n + shows a hint to add a config entry for the tool\n + in gitconfig.\n +\n + If a wrong mergetool.cmd is configured by accident\n + it is beeing handled by git gui already. In this\n + case git gui informs the user that the merge tool\n + couldn't be opened. This behavior is preserved by\n + this change and should not change.\n +\n + Beyond compare 3 and Visual Studio code were\n + tested as manually configured merge tools.\n +\n + Signed-off-by: Tobias Boesch \n \n ## git-gui/lib/mergetool.tcl ##\n @@ git-gui/lib/mergetool.tcl: proc merge_resolve_tool2 {} {\n @@ git-gui/lib/mergetool.tcl: proc merge_resolve_tool2 {} {\n \tdefault {\n -\t\terror_popup [mc \"Unsupported merge tool '%s'\" $tool]\n -\t\treturn\n -+\t\tif {[is_config_true gui.mergetoolfromconfig]} {\n -+\t\t\tset path [get_config mergetool.$tool.path]\n -+\t\t\tset cmdline_config [get_config mergetool.$tool.cmd]\n -+\t\t\tset cmdline_substituted [subst -nobackslashes -nocommands $cmdline_config]\n -+\t\t\tset cmdline [lreplace $cmdline_substituted 0 0 $path]\n ++\t\tset tool_cmd [get_config mergetool.$tool.cmd]\n ++\t\tif {$tool_cmd ne {}} {\n ++\t\t\tset tool_cmd_file_vars_resolved [subst -nobackslashes -nocommands $tool_cmd]\n ++\t\t\tset cmdline [lreplace $tool_cmd_file_vars_resolved 0 0 $merge_tool_path]\n +\t\t} else {\n -+\t\t\terror_popup [mc \"Unsupported merge tool '%s'\" $tool]\n ++\t\t\terror_popup [mc \"Unsupported merge tool '%s'. Is the tool command and path configured properly in gitconfig?\" $tool]\n +\t\t\treturn\n +\t\t}\n \t}\n\n\n git-gui/lib/mergetool.tcl | 10 ++++++++--\n 1 file changed, 8 insertions(+), 2 deletions(-)\n\ndiff --git a/git-gui/lib/mergetool.tcl b/git-gui/lib/mergetool.tcl\nindex e688b016ef6..4c4e8f47bb0 100644\n--- a/git-gui/lib/mergetool.tcl\n+++ b/git-gui/lib/mergetool.tcl\n@@ -272,8 +272,14 @@ proc merge_resolve_tool2 {} {\n \t\t}\n \t}\n \tdefault {\n-\t\terror_popup [mc \"Unsupported merge tool '%s'\" $tool]\n-\t\treturn\n+\t\tset tool_cmd [get_config mergetool.$tool.cmd]\n+\t\tif {$tool_cmd ne {}} {\n+\t\t\tset tool_cmd_file_vars_resolved [subst -nobackslashes -nocommands $tool_cmd]\n+\t\t\tset cmdline [lreplace $tool_cmd_file_vars_resolved 0 0 $merge_tool_path]\n+\t\t} else {\n+\t\t\terror_popup [mc \"Unsupported merge tool '%s'. Is the tool command and path configured properly in gitconfig?\" $tool]\n+\t\t\treturn\n+\t\t}\n \t}\n \t}\n \n\nbase-commit: 159f2d50e75c17382c9f4eb7cbda671a6fa612d1\n-- \ngitgitgadget\n"},{"id":"501783","messageId":"xmqq7cc01sow.fsf@gitster.g","threadId":"61971","inReplyTo":"pull.1773.v2.git.1724833917245.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] git gui: add directly calling merge tool from gitconfig","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-08-28T17:08:15Z","receivedAt":"2024-08-28T17:08:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"ToBoMi via GitGitGadget\" writes:\n\n> From: deboeto \n\nUse the same ident (human readable name plus e-mail address) you\nhave on your Signed-off-by: line below for this \"From: \" in-body\nheader.\n\n> git gui can open a merge tool when conflicts are\n> detected (Right click in the diff of the file with\n> conflicts).\n> The merge tools that are allowed to\n> use are hard coded into git gui.\n>\n> If one wants to add a new merge tool it has to be\n> added to git gui through a source code change.\n> This is not convenient in comparison to how it\n> works in git (without gui).\n>\n> git itself has configuration options for a merge tools\n> path and command in the git config.\n> New merge tools can be set up there without a\n> source code change.\n\nEven if you configure an unknown tool, it would not get any benefit\nfrom what git-{diff,merge}tool--lib.sh gives the known diff/merge\nbackends, would it? Instead of a more thorough support for known\ntools done in setup_tool(), an unknown tool would be handled by\nsetup_user_tool() in git-mergetool-lib.sh which gives somewhat\ndegraded support.\n\nSo \"can be set up without\" may be true, but giving an impression\nthat a tool that is set up like so would work just like a known tool\nis misleading.\n\nBy the way, we do ask contributors to avoid overly long lines,\n50-col limt is a bit overly short and makes the resulting text\nharder to read than necessary.\n\n> Those options are used only by pure git in\n> contrast to git gui. git calls the configured\n> merge tools directly from the config while git\n> Gui doesn't.\n>\n> With this change git gui can call merge tools\n> configured in the gitconfig directly without a\n> change in git gui source code.\n> It needs a configured merge.tool and a configured\n> mergetool.cmd config entry.\n\nOK.\n\n> gitconfig example:\n> [merge]\n> \ttool = vscode\n> [mergetool \"vscode\"]\n> \tpath = the/path/to/Code.exe\n> \tcmd = \\\"Code.exe\\\" --wait --merge \\\"$LOCAL\\\" \\\"$REMOTE\\\" \\\"$BASE\\\" \\\"$MERGED\\\"\n>\n> Without the mergetool.cmd configuration and an\n> unsupported merge.tool entry, git gui behaves\n> mainly as before this change and informs the user\n> about an unsupported merge tool, but now also\n> shows a hint to add a config entry for the tool\n> in gitconfig.\n>\n> If a wrong mergetool.cmd is configured by accident\n> it is beeing handled by git gui already. In this\n\n\"is beeing\" -> \"is being\", but \"it gets handled by Git GUI already\"\nshould be sufficient.\n\n> case git gui informs the user that the merge tool\n> couldn't be opened. This behavior is preserved by\n> this change and should not change.\n>\n> Beyond compare 3 and Visual Studio code were\n> tested as manually configured merge tools.\n\nQuote proper nouns for readability? E.g. \n\n \"Beyond Compare 3\" and \"Visual Studio Code\" were ...\n\nThanks.\n"},{"id":"501952","messageId":"61b9b041-97cf-47ac-84ef-1467aba873e3@kdbg.org","threadId":"61971","inReplyTo":"pull.1773.v2.git.1724833917245.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] git gui: add directly calling merge tool from gitconfig","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2024-08-31T13:51:55Z","receivedAt":"2024-08-31T13:52:15Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 28.08.24 um 10:31 schrieb ToBoMi via GitGitGadget:\n> From: deboeto \n> \n> git gui can open a merge tool when conflicts are\n> detected (Right click in the diff of the file with\n> conflicts).\n> The merge tools that are allowed to\n> use are hard coded into git gui.\n> \n> If one wants to add a new merge tool it has to be\n> added to git gui through a source code change.\n> This is not convenient in comparison to how it\n> works in git (without gui).\n> \n> git itself has configuration options for a merge tools\n> path and command in the git config.\n> New merge tools can be set up there without a\n> source code change.\n> \n> Those options are used only by pure git in\n> contrast to git gui. git calls the configured\n> merge tools directly from the config while git\n> Gui doesn't.\n> \n> With this change git gui can call merge tools\n> configured in the gitconfig directly without a\n> change in git gui source code.\n> It needs a configured merge.tool and a configured\n> mergetool.cmd config entry.\n\nOK.\n\n> gitconfig example:\n> [merge]\n> \ttool = vscode\n> [mergetool \"vscode\"]\n> \tpath = the/path/to/Code.exe\n> \tcmd = \\\"Code.exe\\\" --wait --merge \\\"$LOCAL\\\" \\\"$REMOTE\\\" \\\"$BASE\\\" \\\"$MERGED\\\"\n\nI found it annoying that I had to configure .path in addition to .cmd.\nTypically, you would put the correct path into the .cmd configuration.\nIn fact, `git mergetool` works without .path and fails when .cmd does\nnot contain the correct path.\n\n> Without the mergetool.cmd configuration and an\n> unsupported merge.tool entry, git gui behaves\n> mainly as before this change and informs the user\n> about an unsupported merge tool, but now also\n> shows a hint to add a config entry for the tool\n> in gitconfig.\n\nGood.\n\nWhile testing I configured meld incorrectly once and got no feedback\nwhatsoever, but I would not attribute this to this patch.\n\nThere is no such thing called \"gitconfig\". Just strike \"in gitconfig\".\n\n> If a wrong mergetool.cmd is configured by accident\n> it is beeing handled by git gui already. In this\n> case git gui informs the user that the merge tool\n> couldn't be opened. This behavior is preserved by\n> this change and should not change.\n\nGood.\n\n> \n> Beyond compare 3 and Visual Studio code were\n> tested as manually configured merge tools.\n> \n> Signed-off-by: Tobias Boesch \n\nYou updated this line, but not the From: line. Would you mind\nconfiguring your user.name and then `git commit --amend --reset-author`?\n\n> git-gui/lib/mergetool.tcl | 10 ++++++++--\n> 1 file changed, 8 insertions(+), 2 deletions(-)\n> \n> diff --git a/git-gui/lib/mergetool.tcl b/git-gui/lib/mergetool.tcl\n> index e688b016ef6..4c4e8f47bb0 100644\n> --- a/git-gui/lib/mergetool.tcl\n> +++ b/git-gui/lib/mergetool.tcl\n> @@ -272,8 +272,14 @@ proc merge_resolve_tool2 {} {\n> \t\t}\n> \t}\n> \tdefault {\n> -\t\terror_popup [mc \"Unsupported merge tool '%s'\" $tool]\n> -\t\treturn\n> +\t\tset tool_cmd [get_config mergetool.$tool.cmd]\n> +\t\tif {$tool_cmd ne {}} {\n> +\t\t\tset tool_cmd_file_vars_resolved [subst -nobackslashes -nocommands $tool_cmd]\n\nI just learnt that a string value containing double-quotes is broken\ninto a list in the expected way (keeps quoted parts together as a single\nelement). However, this form of substitution replaces variable values\nwith arbitrary text without taking into account that the original string\nis actually a list. Should we not break the string into a list first,\nand apply the substitution on the list elements?\n\nIf there is a straight-forward way to do this (say, an obvious two-liner\nat most), we should do it. Otherwise, I can live with this solution for\nnow because it requires file names with double-quotes to break the\nexpected list nature.\n\nThere is another thing, though, that I would not want to take as\nlightly: The -nocommands modifier of `subst` does not live up to its\npromises, and it is even the documented behavior: command substitutions\nin array indexes are still executed. Consider this configuration:\n\n[merge]\n tool = evil\n[mergetool \"evil\"]\n cmd = meld \\\"$REMOTE([exit])\\\"\n\nGuess what happens when I run the merge tool? It exits Git GUI!\n\nI suggest to reject any configuration that contains an opening bracket\n'[' or anything else that introduces a command execution.\n\n> +\t\t\tset cmdline [lreplace $tool_cmd_file_vars_resolved 0 0 $merge_tool_path]\n> +\t\t} else {\n> +\t\t\terror_popup [mc \"Unsupported merge tool '%s'. Is the tool command and path configured properly in gitconfig?\" $tool]\n\nCan we not have a more helpful text? How about\n\n\t\t\terror_popup [mc \"Unsupported merge tool '%s'.\n\nSee the git-config manual page how to configure mergetool.%s.cmd\nsuitably.\" $tool $tool]\n\n> +\t\t\treturn\n> +\t\t}\n> \t}\n> \t}\n> \n\n-- Hannes\n\n"},{"id":"502198","messageId":"AS2PR08MB8288F9C7DFE0FC563D81A30DE19D2@AS2PR08MB8288.eurprd08.prod.outlook.com","threadId":"61971","inReplyTo":"xmqq7cc01sow.fsf@gitster.g","subject":"AW: [PATCH v2] git gui: add directly calling merge tool from gitconfig","fromName":"tobias.boesch@miele.com","fromEmail":"tobias.boesch@miele.com","sentAt":"2024-09-05T08:09:00Z","receivedAt":"2024-09-05T08:10:02Z","isPatch":true,"sender":{"key":"tobias.boesch@miele.com","avatar":"https://avatars.githubusercontent.com/u/64197724?v=4"},"body":"Thanks for he review.\n\n> -----Ursprüngliche Nachricht-----\n> Von: Junio C Hamano \n> Gesendet: Mittwoch, 28. August 2024 19:08\n> An: ToBoMi via GitGitGadget \n> Cc: git@vger.kernel.org; Boesch, Tobias \n> Betreff: Re: [PATCH v2] git gui: add directly calling merge tool from gitconfig\n>\n> \"ToBoMi via GitGitGadget\" writes:\n>\n> > From: deboeto \n>\n> Use the same ident (human readable name plus e-mail address) you have on\n> your Signed-off-by: line below for this \"From: \" in-body header.\n>\n> > git gui can open a merge tool when conflicts are detected (Right click\n> > in the diff of the file with conflicts).\n> > The merge tools that are allowed to\n> > use are hard coded into git gui.\n> >\n> > If one wants to add a new merge tool it has to be added to git gui\n> > through a source code change.\n> > This is not convenient in comparison to how it works in git (without\n> > gui).\n> >\n> > git itself has configuration options for a merge tools path and\n> > command in the git config.\n> > New merge tools can be set up there without a source code change.\n>\n> Even if you configure an unknown tool, it would not get any benefit from\n> what git-{diff,merge}tool--lib.sh gives the known diff/merge backends, would\n> it? Instead of a more thorough support for known tools done in setup_tool(),\n> an unknown tool would be handled by\n> setup_user_tool() in git-mergetool-lib.sh which gives somewhat degraded\n> support.\n>\n\nThat might be. I don't know about this handling.\nWould it be a problem to not have this handling for unsupported tools? Since the concept of supported tools is not removed by this patch, tools can still be added as supported, to get this thorough handling.\nI personally prefer to have an unsupported tool, that I can configure and use right now and add official support for it later, instead of having some well-supported tools which exclude the tool I want to use right now and no option to add it quickly.\nThat is why I didn't add support for the tool I want to use right now. I wanted it to be more universal, so that every tool I can configure will work immediately.\n\n> So \"can be set up without\" may be true, but giving an impression that a tool\n> that is set up like so would work just like a known tool is misleading.\n>\n\nI don't want this patch to give that impression. How can this be avoided from your point of view?\n\nThe degraded functionality for unsupported tools could be mentioned in the message for an unsupported tool introduced enhanced in this patch. It could tell the user that the current tool is not supported, but that it can be setup with degraded support in the config.\nAn updated message will be part of the next patch.\n\n> By the way, we do ask contributors to avoid overly long lines, 50-col limt is a\n> bit overly short and makes the resulting text harder to read than necessary.\n>\n> > Those options are used only by pure git in contrast to git gui. git\n> > calls the configured merge tools directly from the config while git\n> > Gui doesn't.\n> >\n> > With this change git gui can call merge tools configured in the\n> > gitconfig directly without a change in git gui source code.\n> > It needs a configured merge.tool and a configured mergetool.cmd config\n> > entry.\n>\n> OK.\n>\n> > gitconfig example:\n> > [merge]\n> > tool = vscode\n> > [mergetool \"vscode\"]\n> > path = the/path/to/Code.exe\n> > cmd = \\\"Code.exe\\\" --wait --merge \\\"$LOCAL\\\" \\\"$REMOTE\\\"\n> \\\"$BASE\\\" \\\"$MERGED\\\"\n> >\n> > Without the mergetool.cmd configuration and an unsupported merge.tool\n> > entry, git gui behaves mainly as before this change and informs the\n> > user about an unsupported merge tool, but now also shows a hint to add\n> > a config entry for the tool in gitconfig.\n> >\n> > If a wrong mergetool.cmd is configured by accident it is beeing\n> > handled by git gui already. In this\n>\n> \"is beeing\" -> \"is being\", but \"it gets handled by Git GUI already\"\n> should be sufficient.\n>\n> > case git gui informs the user that the merge tool couldn't be opened.\n> > This behavior is preserved by this change and should not change.\n> >\n> > Beyond compare 3 and Visual Studio code were tested as manually\n> > configured merge tools.\n>\n> Quote proper nouns for readability? E.g.\n>\n> \"Beyond Compare 3\" and \"Visual Studio Code\" were ...\n>\n> Thanks.\n\nI'll correct the minor suggestions in the next patch version.\n\n\n-------------------------------------------------------------------------------------------------\nimperial-Werke oHG, Sitz Bünde, Registergericht Bad Oeynhausen - HRA 4825\n"},{"id":"502306","messageId":"AS2PR08MB828842126285586C19028FB5E19E2@AS2PR08MB8288.eurprd08.prod.outlook.com","threadId":"61971","inReplyTo":"61b9b041-97cf-47ac-84ef-1467aba873e3@kdbg.org","subject":"AW: [PATCH v2] git gui: add directly calling merge tool from gitconfig","fromName":"tobias.boesch@miele.com","fromEmail":"tobias.boesch@miele.com","sentAt":"2024-09-06T06:32:34Z","receivedAt":"2024-09-06T06:33:02Z","isPatch":true,"sender":{"key":"tobias.boesch@miele.com","avatar":"https://avatars.githubusercontent.com/u/64197724?v=4"},"body":"> -----Ursprüngliche Nachricht-----\n> Von: Johannes Sixt \n> Gesendet: Samstag, 31. August 2024 15:52\n> An: Boesch, Tobias \n> Cc: ToBoMi via GitGitGadget ; git@vger.kernel.org\n> Betreff: Re: [PATCH v2] git gui: add directly calling merge tool from gitconfig\n>\n> Am 28.08.24 um 10:31 schrieb ToBoMi via GitGitGadget:\n> > From: deboeto \n> >\n> > git gui can open a merge tool when conflicts are detected (Right click\n> > in the diff of the file with conflicts).\n> > The merge tools that are allowed to\n> > use are hard coded into git gui.\n> >\n> > If one wants to add a new merge tool it has to be added to git gui\n> > through a source code change.\n> > This is not convenient in comparison to how it works in git (without\n> > gui).\n> >\n> > git itself has configuration options for a merge tools path and\n> > command in the git config.\n> > New merge tools can be set up there without a source code change.\n> >\n> > Those options are used only by pure git in contrast to git gui. git\n> > calls the configured merge tools directly from the config while git\n> > Gui doesn't.\n> >\n> > With this change git gui can call merge tools configured in the\n> > gitconfig directly without a change in git gui source code.\n> > It needs a configured merge.tool and a configured mergetool.cmd config\n> > entry.\n>\n> OK.\n>\n> > gitconfig example:\n> > [merge]\n> > tool = vscode\n> > [mergetool \"vscode\"]\n> > path = the/path/to/Code.exe\n> > cmd = \\\"Code.exe\\\" --wait --merge \\\"$LOCAL\\\" \\\"$REMOTE\\\"\n> \\\"$BASE\\\" \\\"$MERGED\\\"\n>\n> I found it annoying that I had to configure .path in addition to .cmd.\n> Typically, you would put the correct path into the .cmd configuration.\n> In fact, `git mergetool` works without .path and fails when .cmd does not\n> contain the correct path.\n>\n\nI changed it to only use the mergetool.cmd and updated the configuration hint that mentions the configuration variables so that users know to only specify the cmd variable.\n\n> > Without the mergetool.cmd configuration and an unsupported merge.tool\n> > entry, git gui behaves mainly as before this change and informs the\n> > user about an unsupported merge tool, but now also shows a hint to add\n> > a config entry for the tool in gitconfig.\n>\n> Good.\n>\n> While testing I configured meld incorrectly once and got no feedback\n> whatsoever, but I would not attribute this to this patch.\n>\n\nThat's odd. I tested this again by setting merge.tool to \"meld\" and configured mergetool.cmd to \"some wrong path\". When starting the mergetool I got a popup saying that meld was not found in path.\n\n> There is no such thing called \"gitconfig\". Just strike \"in gitconfig\".\n>\n> > If a wrong mergetool.cmd is configured by accident it is beeing\n> > handled by git gui already. In this case git gui informs the user that\n> > the merge tool couldn't be opened. This behavior is preserved by this\n> > change and should not change.\n>\n> Good.\n>\n> >\n> > Beyond compare 3 and Visual Studio code were tested as manually\n> > configured merge tools.\n> >\n> > Signed-off-by: Tobias Boesch \n>\n> You updated this line, but not the From: line. Would you mind configuring\n> your user.name and then `git commit --amend --reset-author`?\n>\n> > git-gui/lib/mergetool.tcl | 10 ++++++++--\n> > 1 file changed, 8 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/git-gui/lib/mergetool.tcl b/git-gui/lib/mergetool.tcl\n> > index e688b016ef6..4c4e8f47bb0 100644\n> > --- a/git-gui/lib/mergetool.tcl\n> > +++ b/git-gui/lib/mergetool.tcl\n> > @@ -272,8 +272,14 @@ proc merge_resolve_tool2 {} {\n> > }\n> > }\n> > default {\n> > - error_popup [mc \"Unsupported merge tool '%s'\" $tool]\n> > - return\n> > + set tool_cmd [get_config mergetool.$tool.cmd]\n> > + if {$tool_cmd ne {}} {\n> > + set tool_cmd_file_vars_resolved [subst -nobackslashes\n> -nocommands\n> > +$tool_cmd]\n>\n> I just learnt that a string value containing double-quotes is broken into a list in\n> the expected way (keeps quoted parts together as a single element). However,\n> this form of substitution replaces variable values with arbitrary text without\n> taking into account that the original string is actually a list. Should we not\n> break the string into a list first, and apply the substitution on the list elements?\n>\n\nI can iterate directly on the input string as a list. Will be part of the next patch version.\n\n> If there is a straight-forward way to do this (say, an obvious two-liner at\n> most), we should do it. Otherwise, I can live with this solution for now\n> because it requires file names with double-quotes to break the expected list\n> nature.\n>\n> There is another thing, though, that I would not want to take as\n> lightly: The -nocommands modifier of `subst` does not live up to its promises,\n> and it is even the documented behavior: command substitutions in array\n> indexes are still executed. Consider this configuration:\n>\n> [merge]\n> tool = evil\n> [mergetool \"evil\"]\n> cmd = meld \\\"$REMOTE([exit])\\\"\n>\n> Guess what happens when I run the merge tool? It exits Git GUI!\n>\n> I suggest to reject any configuration that contains an opening bracket '[' or\n> anything else that introduces a command execution.\n>\n\nGood catch. I added code to prevent this in the next patch version.\nWhen a command sequence is detected (basically square brackets in the command) the user get a hint to avoid square brackets in the mergetool.cmd.\n\n> > + set cmdline [lreplace $tool_cmd_file_vars_resolved 0 0\n> $merge_tool_path]\n> > + } else {\n> > + error_popup [mc \"Unsupported merge tool '%s'. Is the\n> tool command\n> > +and path configured properly in gitconfig?\" $tool]\n>\n> Can we not have a more helpful text? How about\n>\n> error_popup [mc \"Unsupported merge tool '%s'.\n>\n> See the git-config manual page how to configure mergetool.%s.cmd suitably.\"\n> $tool $tool]\n>\n\nTrue. Updating the text.\n\n> > + return\n> > + }\n> > }\n> > }\n> >\n>\n> -- Hannes\n\n\nMinor suggestions will be fixed in the next patch.\n\n\n-------------------------------------------------------------------------------------------------\nimperial-Werke oHG, Sitz Bünde, Registergericht Bad Oeynhausen - HRA 4825\n"},{"id":"502309","messageId":"pull.1773.v3.git.1725607643479.gitgitgadget@gmail.com","threadId":"61971","inReplyTo":"pull.1773.v2.git.1724833917245.gitgitgadget@gmail.com","subject":"[PATCH v3] git gui: add directly calling merge tool from gitconfig","fromName":"ToBoMi via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-09-06T07:27:23Z","receivedAt":"2024-09-06T07:27:27Z","isPatch":true,"sender":{"key":"name:ToBoMi","avatar":null},"body":"From: Tobias Boesch \n\ngit gui can open a merge tool when conflicts are detected (Right click\nin the diff of the file with conflicts).\nThe merge tools that are allowed to use are hard coded into git gui.\n\nIf one wants to add a new merge tool it has to be added to git gui\nthrough a source code change.\nThis is not convenient in comparison to how it works in git (without gui).\n\ngit itself has configuration options for a merge tools path and command\nin the git config.\nNew merge tools can be set up there without a source code change.\n\nThose options are used only by pure git in contrast to git gui. git calls\nthe configured merge tools directly from the config while git Gui doesn't.\n\nWith this change git gui can call merge tools configured in the gitconfig\ndirectly without a change in git gui source code.\nIt needs a configured merge.tool and a configured mergetool.cmd config\nentry.\n\ngitconfig example:\n[merge]\n\ttool = vscode\n[mergetool \"vscode\"]\n\tpath = the/path/to/Code.exe\n\tcmd = \\\"Code.exe\\\" --wait --merge \\\"$LOCAL\\\" \\\"$REMOTE\\\" \\\"$BASE\\\" \\\"$MERGED\\\"\n\nWithout the mergetool.cmd configuration and an unsupported merge.tool\nentry, git gui behaves mainly as before this change and informs the user\nabout an unsupported merge tool. In addtition it also shows a hint to add\na config entry to use the tool as an unsupported tool with degraded\nsupport.\n\nIf a wrong mergetool.cmd is configured by accident, it gets handled\nby git gui already. In this case git gui informs the user that the merge\ntool couldn't be opened. This behavior is preserved by this change and\nshould not change.\n\n\"Beyond Compare 3\" and \"Visual Studio Code\" were tested as manually\nconfigured merge tools.\n\nSigned-off-by: Tobias Boesch \n---\n git gui: add directly calling merge tool from gitconfig\n \n cc: Johannes Sixt j6t@kdbg.org\n \n Changes since v1:\n \n * Used existing option mergetool.cmd in gitconfig to trigger the direct\n call of the merge tool configured in the config instead adding a new\n option mergeToolFromConfig\n * Removed assignment of merge tool path to a variable and reused the\n already existing one: merget_tool_path\n * Changed formatting of the commit message\n * Added more context and an examples to the commit message\n \n Changes since v2:\n \n * Changed commit ident\n * Added hint to add a mergetool as an unsupprted tool\n * Minor typos\n * Highlighted proper nouns in commit message\n * Only using mergetool.cmd now - not using mergetool.path anymore\n * Removed gitconfig term in user message\n * Changed lines length of commit message\n * tcl commands in mergetool.cmd are now detected and not executed\n anymore\n * mergetool.cmd string parts are now substituted as list, not as a\n whole string\n * Made a more clear user hint on how to configure an unsupported\n mergetool\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1773%2FToBoMi%2Fadd_merge_tool_from_config_file-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1773/ToBoMi/add_merge_tool_from_config_file-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/1773\n\nRange-diff vs v2:\n\n 1: e77d6dec6c5 ! 1: c8c0107ddc5 git gui: add directly calling merge tool from gitconfig\n @@\n ## Metadata ##\n -Author: deboeto \n +Author: Tobias Boesch \n \n ## Commit message ##\n git gui: add directly calling merge tool from gitconfig\n \n - git gui can open a merge tool when conflicts are\n - detected (Right click in the diff of the file with\n - conflicts).\n - The merge tools that are allowed to\n - use are hard coded into git gui.\n + git gui can open a merge tool when conflicts are detected (Right click\n + in the diff of the file with conflicts).\n + The merge tools that are allowed to use are hard coded into git gui.\n \n - If one wants to add a new merge tool it has to be\n - added to git gui through a source code change.\n - This is not convenient in comparison to how it\n - works in git (without gui).\n + If one wants to add a new merge tool it has to be added to git gui\n + through a source code change.\n + This is not convenient in comparison to how it works in git (without gui).\n \n - git itself has configuration options for a merge tools\n - path and command in the git config.\n - New merge tools can be set up there without a\n - source code change.\n + git itself has configuration options for a merge tools path and command\n + in the git config.\n + New merge tools can be set up there without a source code change.\n \n - Those options are used only by pure git in\n - contrast to git gui. git calls the configured\n - merge tools directly from the config while git\n - Gui doesn't.\n + Those options are used only by pure git in contrast to git gui. git calls\n + the configured merge tools directly from the config while git Gui doesn't.\n \n - With this change git gui can call merge tools\n - configured in the gitconfig directly without a\n - change in git gui source code.\n - It needs a configured merge.tool and a configured\n - mergetool.cmd config entry.\n + With this change git gui can call merge tools configured in the gitconfig\n + directly without a change in git gui source code.\n + It needs a configured merge.tool and a configured mergetool.cmd config\n + entry.\n \n gitconfig example:\n [merge]\n @@ Commit message\n path = the/path/to/Code.exe\n cmd = \\\"Code.exe\\\" --wait --merge \\\"$LOCAL\\\" \\\"$REMOTE\\\" \\\"$BASE\\\" \\\"$MERGED\\\"\n \n - Without the mergetool.cmd configuration and an\n - unsupported merge.tool entry, git gui behaves\n - mainly as before this change and informs the user\n - about an unsupported merge tool, but now also\n - shows a hint to add a config entry for the tool\n - in gitconfig.\n + Without the mergetool.cmd configuration and an unsupported merge.tool\n + entry, git gui behaves mainly as before this change and informs the user\n + about an unsupported merge tool. In addtition it also shows a hint to add\n + a config entry to use the tool as an unsupported tool with degraded\n + support.\n \n - If a wrong mergetool.cmd is configured by accident\n - it is beeing handled by git gui already. In this\n - case git gui informs the user that the merge tool\n - couldn't be opened. This behavior is preserved by\n - this change and should not change.\n + If a wrong mergetool.cmd is configured by accident, it gets handled\n + by git gui already. In this case git gui informs the user that the merge\n + tool couldn't be opened. This behavior is preserved by this change and\n + should not change.\n \n - Beyond compare 3 and Visual Studio code were\n - tested as manually configured merge tools.\n + \"Beyond Compare 3\" and \"Visual Studio Code\" were tested as manually\n + configured merge tools.\n \n Signed-off-by: Tobias Boesch \n \n @@ git-gui/lib/mergetool.tcl: proc merge_resolve_tool2 {} {\n -\t\treturn\n +\t\tset tool_cmd [get_config mergetool.$tool.cmd]\n +\t\tif {$tool_cmd ne {}} {\n -+\t\t\tset tool_cmd_file_vars_resolved [subst -nobackslashes -nocommands $tool_cmd]\n -+\t\t\tset cmdline [lreplace $tool_cmd_file_vars_resolved 0 0 $merge_tool_path]\n ++\t\t\tif {([string first {[} $tool_cmd] != -1) || ([string first {]} $tool_cmd] != -1)} {\n ++\t\t\t\terror_popup [mc \"Unable to process square brackets in mergetool.cmd configuration option.\\\n ++\t\t\t\t\t\t\t\tPlease remove the square brackets.\"]\n ++\t\t\t\treturn\n ++\t\t\t} else {\n ++\t\t\t\tforeach command_part $tool_cmd {\n ++\t\t\t\t\tlappend cmdline [subst -nobackslashes -nocommands $command_part]\n ++\t\t\t\t}\n ++\t\t\t}\n +\t\t} else {\n -+\t\t\terror_popup [mc \"Unsupported merge tool '%s'. Is the tool command and path configured properly in gitconfig?\" $tool]\n ++\t\t\terror_popup [mc \"Unsupported merge tool '%s'.\\n\n ++\t\t\t\t\t\t\tCurrently unsupported tools can be added and used as unsupported tools with degraded support\\\n ++\t\t\t\t\t\t\tby adding the command of the tool to the \\\"mergetool.cmd\\\" option in the config.\n ++\t\t\t\t\t\t\tSee the configuration documentation for more details.\" $tool]\n +\t\t\treturn\n +\t\t}\n \t}\n\n\n git-gui/lib/mergetool.tcl | 20 ++++++++++++++++++--\n 1 file changed, 18 insertions(+), 2 deletions(-)\n\ndiff --git a/git-gui/lib/mergetool.tcl b/git-gui/lib/mergetool.tcl\nindex e688b016ef6..ccbc1a46554 100644\n--- a/git-gui/lib/mergetool.tcl\n+++ b/git-gui/lib/mergetool.tcl\n@@ -272,8 +272,24 @@ proc merge_resolve_tool2 {} {\n \t\t}\n \t}\n \tdefault {\n-\t\terror_popup [mc \"Unsupported merge tool '%s'\" $tool]\n-\t\treturn\n+\t\tset tool_cmd [get_config mergetool.$tool.cmd]\n+\t\tif {$tool_cmd ne {}} {\n+\t\t\tif {([string first {[} $tool_cmd] != -1) || ([string first {]} $tool_cmd] != -1)} {\n+\t\t\t\terror_popup [mc \"Unable to process square brackets in mergetool.cmd configuration option.\\\n+\t\t\t\t\t\t\t\tPlease remove the square brackets.\"]\n+\t\t\t\treturn\n+\t\t\t} else {\n+\t\t\t\tforeach command_part $tool_cmd {\n+\t\t\t\t\tlappend cmdline [subst -nobackslashes -nocommands $command_part]\n+\t\t\t\t}\n+\t\t\t}\n+\t\t} else {\n+\t\t\terror_popup [mc \"Unsupported merge tool '%s'.\\n\n+\t\t\t\t\t\t\tCurrently unsupported tools can be added and used as unsupported tools with degraded support\\\n+\t\t\t\t\t\t\tby adding the command of the tool to the \\\"mergetool.cmd\\\" option in the config.\n+\t\t\t\t\t\t\tSee the configuration documentation for more details.\" $tool]\n+\t\t\treturn\n+\t\t}\n \t}\n \t}\n \n\nbase-commit: 2e7b89e038c0c888acf61f1b4ee5a43d4dd5e94c\n-- \ngitgitgadget\n"},{"id":"502341","messageId":"a8ed352a-bcb8-4e62-945a-ac5d6ff78841@kdbg.org","threadId":"61971","inReplyTo":"AS2PR08MB828842126285586C19028FB5E19E2@AS2PR08MB8288.eurprd08.prod.outlook.com","subject":"Re: [PATCH v2] git gui: add directly calling merge tool from gitconfig","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2024-09-06T17:43:44Z","receivedAt":"2024-09-06T17:43:50Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 06.09.24 um 08:32 schrieb tobias.boesch@miele.com:\n>> Von: Johannes Sixt \n>> While testing I configured meld incorrectly once and got no feedback\n>> whatsoever, but I would not attribute this to this patch.\n>>\n> \n> That's odd. I tested this again by setting merge.tool to \"meld\" and\n> configured mergetool.cmd to \"some wrong path\". When starting the\n> mergetool I got a popup saying that meld was not found in path.\nBut if the configuration is\n\n cmd = \"meld far too many arguments provided\"\n\nand 'meld' *is* in the path, then there is no feedback because meld can\nbe started successfully, but reports an error only on stdout or stderr,\nwhich is ignored by Git GUI. And the exit code seems to be ignored, too.\n\nBut this can be treated in a follow-up patch if necessary.\n\n-- Hannes\n\n"},{"id":"502409","messageId":"9c7475c8-a666-4033-a4b1-79819ba7717f@kdbg.org","threadId":"61971","inReplyTo":"pull.1773.v3.git.1725607643479.gitgitgadget@gmail.com","subject":"Re: [PATCH v3] git gui: add directly calling merge tool from gitconfig","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2024-09-08T12:21:24Z","receivedAt":"2024-09-08T12:21:35Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 06.09.24 um 09:27 schrieb ToBoMi via GitGitGadget:\n> From: Tobias Boesch \n> \n> git gui can open a merge tool when conflicts are detected (Right click\n> in the diff of the file with conflicts).\n> The merge tools that are allowed to use are hard coded into git gui.\n> \n> If one wants to add a new merge tool it has to be added to git gui\n> through a source code change.\n> This is not convenient in comparison to how it works in git (without gui).\n> \n> git itself has configuration options for a merge tools path and command\n> in the git config.\n> New merge tools can be set up there without a source code change.\n> \n> Those options are used only by pure git in contrast to git gui. git calls\n> the configured merge tools directly from the config while git Gui doesn't.\n> \n> With this change git gui can call merge tools configured in the gitconfig\n> directly without a change in git gui source code.\n> It needs a configured merge.tool and a configured mergetool.cmd config\n> entry.\n\nThe configuration is \"mergetool.$tool.cmd\"!\n\nPersonally, I would avoid the words \"gitconfig\" and \"config\" (here and\nin the rest of the commit message), neither of which are English.\n\"Configuration\" would be OK, IMO.\n\n> \n> gitconfig example:\n> [merge]\n> \ttool = vscode\n> [mergetool \"vscode\"]\n> \tpath = the/path/to/Code.exe\n> \tcmd = \\\"Code.exe\\\" --wait --merge \\\"$LOCAL\\\" \\\"$REMOTE\\\" \\\"$BASE\\\" \\\"$MERGED\\\"\n> \n> Without the mergetool.cmd configuration and an unsupported merge.tool\n> entry, git gui behaves mainly as before this change and informs the user\n> about an unsupported merge tool. In addtition it also shows a hint to add\n> a config entry to use the tool as an unsupported tool with degraded\n> support.\n> \n> If a wrong mergetool.cmd is configured by accident, it gets handled\n> by git gui already. In this case git gui informs the user that the merge\n> tool couldn't be opened. This behavior is preserved by this change and\n> should not change.\n> \n> \"Beyond Compare 3\" and \"Visual Studio Code\" were tested as manually\n> configured merge tools.\n> \n> Signed-off-by: Tobias Boesch \n> ---\n\n> git-gui/lib/mergetool.tcl | 20 ++++++++++++++++++--\n> 1 file changed, 18 insertions(+), 2 deletions(-)\n> \n> diff --git a/git-gui/lib/mergetool.tcl b/git-gui/lib/mergetool.tcl\n> index e688b016ef6..ccbc1a46554 100644\n> --- a/git-gui/lib/mergetool.tcl\n> +++ b/git-gui/lib/mergetool.tcl\n> @@ -272,8 +272,24 @@ proc merge_resolve_tool2 {} {\n> \t\t}\n> \t}\n> \tdefault {\n> -\t\terror_popup [mc \"Unsupported merge tool '%s'\" $tool]\n> -\t\treturn\n> +\t\tset tool_cmd [get_config mergetool.$tool.cmd]\n> +\t\tif {$tool_cmd ne {}} {\n> +\t\t\tif {([string first {[} $tool_cmd] != -1) || ([string first {]} $tool_cmd] != -1)} {\n> +\t\t\t\terror_popup [mc \"Unable to process square brackets in mergetool.cmd configuration option.\\\n> +\t\t\t\t\t\t\t\tPlease remove the square brackets.\"]\n> +\t\t\t\treturn\n\nCondition and error text are OK. But see below.\n\n> +\t\t\t} else {\n> +\t\t\t\tforeach command_part $tool_cmd {\n> +\t\t\t\t\tlappend cmdline [subst -nobackslashes -nocommands $command_part]\n> +\t\t\t\t}\n\nGood.\n\nI have seen a few examples in the Tcl manual with lappend in the loop\nbody, and it seems to be customary to set the list variable to an empty\nvalue before the loop, i.e.\n\n\t\t\t\tset cmdline {}\n\n> +\t\t\t}\n> +\t\t} else {\n> +\t\t\terror_popup [mc \"Unsupported merge tool '%s'.\\n\n> +\t\t\t\t\t\t\tCurrently unsupported tools can be added and used as unsupported tools with degraded support\\\n> +\t\t\t\t\t\t\tby adding the command of the tool to the \\\"mergetool.cmd\\\" option in the config.\n> +\t\t\t\t\t\t\tSee the configuration documentation for more details.\" $tool]\n\nThis error message needs a bit more work (some of this also applies to\nthe message above):\n\n- A tool is only unsupported as long as there is no usable\nconfiguration. Once mergetool.$tool.cmd is set to something we can\nhandle, calling the tool \"unsupported\" isn't appropriate, I would think.\nHow about\n\nUnsupported merge tool '%s'.\n\nTo use this tool, configure \"mergetool.%s.cmd\" as shown in the\ngit-config manual page.\n\n- The configuration variable that we use is not mergetool.cmd, but\nmergetool.$tool.cmd.\n\n- Continuation lines must not be indented. Indented text appears\nindented in the error message.\n\n- Watch out whether an explicit \\n is given, whether the line-break is\nescaped or not; all of this has meaning.\n\n- Looking at other multi-line error messages in git-gui.sh, the\nconvention is\n\n\tmc[\"First paragraph goes here.\n\nSecond paragraph. All of it is on one line in the source code.\n\nThird paragraph. No \\n appears anywhere.\"]\n\n> +\t\t\treturn\n> +\t\t}\n> \t}\n> \t}\n\nAs a matter of personal taste, I prefer to structure code with error\nexits like so (but it is totally acceptable if you disagree):\n\n if {check for error 1} {\n error msg1\n return\n }\n if {check for error 2} {\n error msg2\n return\n }\n regular case\n goes here\n without indentation\n\nNote that there are no else-branches. This reduces the indentation levels.\n\n-- Hannes\n\n"},{"id":"502620","messageId":"AS2PR08MB82886B804A9A24F45C6C6F34E19B2@AS2PR08MB8288.eurprd08.prod.outlook.com","threadId":"61971","inReplyTo":"9c7475c8-a666-4033-a4b1-79819ba7717f@kdbg.org","subject":"AW: [PATCH v3] git gui: add directly calling merge tool from gitconfig","fromName":"tobias.boesch@miele.com","fromEmail":"tobias.boesch@miele.com","sentAt":"2024-09-11T13:41:28Z","receivedAt":"2024-09-11T13:41:48Z","isPatch":true,"sender":{"key":"tobias.boesch@miele.com","avatar":"https://avatars.githubusercontent.com/u/64197724?v=4"},"body":"> -----Ursprüngliche Nachricht-----\n> Von: Johannes Sixt \n> Gesendet: Sonntag, 8. September 2024 14:21\n> An: Boesch, Tobias \n> Cc: git@vger.kernel.org; ToBoMi via GitGitGadget \n> Betreff: Re: [PATCH v3] git gui: add directly calling merge tool from gitconfig\n>\n> Am 06.09.24 um 09:27 schrieb ToBoMi via GitGitGadget:\n> > From: Tobias Boesch \n> >\n> > git gui can open a merge tool when conflicts are detected (Right click\n> > in the diff of the file with conflicts).\n> > The merge tools that are allowed to use are hard coded into git gui.\n> >\n> > If one wants to add a new merge tool it has to be added to git gui\n> > through a source code change.\n> > This is not convenient in comparison to how it works in git (without gui).\n> >\n> > git itself has configuration options for a merge tools path and\n> > command in the git config.\n> > New merge tools can be set up there without a source code change.\n> >\n> > Those options are used only by pure git in contrast to git gui. git\n> > calls the configured merge tools directly from the config while git Gui\n> doesn't.\n> >\n> > With this change git gui can call merge tools configured in the\n> > gitconfig directly without a change in git gui source code.\n> > It needs a configured merge.tool and a configured mergetool.cmd config\n> > entry.\n>\n> The configuration is \"mergetool.$tool.cmd\"!\n>\n\nRight - changed it to \"mergetool..cmd\", since this is a descriptive\ntext and not a script. I personally prefer a more human friendly writing.\n\n> Personally, I would avoid the words \"gitconfig\" and \"config\" (here and in the\n> rest of the commit message), neither of which are English.\n> \"Configuration\" would be OK, IMO.\n>\n\nWill be changed.\n\n> >\n> > gitconfig example:\n> > [merge]\n> > tool = vscode\n> > [mergetool \"vscode\"]\n> > path = the/path/to/Code.exe\n> > cmd = \\\"Code.exe\\\" --wait --merge \\\"$LOCAL\\\" \\\"$REMOTE\\\"\n> \\\"$BASE\\\" \\\"$MERGED\\\"\n> >\n> > Without the mergetool.cmd configuration and an unsupported merge.tool\n> > entry, git gui behaves mainly as before this change and informs the\n> > user about an unsupported merge tool. In addtition it also shows a\n> > hint to add a config entry to use the tool as an unsupported tool with\n> > degraded support.\n> >\n> > If a wrong mergetool.cmd is configured by accident, it gets handled by\n> > git gui already. In this case git gui informs the user that the merge\n> > tool couldn't be opened. This behavior is preserved by this change and\n> > should not change.\n> >\n> > \"Beyond Compare 3\" and \"Visual Studio Code\" were tested as manually\n> > configured merge tools.\n> >\n> > Signed-off-by: Tobias Boesch \n> > ---\n>\n> > git-gui/lib/mergetool.tcl | 20 ++++++++++++++++++--\n> > 1 file changed, 18 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/git-gui/lib/mergetool.tcl b/git-gui/lib/mergetool.tcl\n> > index e688b016ef6..ccbc1a46554 100644\n> > --- a/git-gui/lib/mergetool.tcl\n> > +++ b/git-gui/lib/mergetool.tcl\n> > @@ -272,8 +272,24 @@ proc merge_resolve_tool2 {} {\n> > }\n> > }\n> > default {\n> > - error_popup [mc \"Unsupported merge tool '%s'\" $tool]\n> > - return\n> > + set tool_cmd [get_config mergetool.$tool.cmd]\n> > + if {$tool_cmd ne {}} {\n> > + if {([string first {[} $tool_cmd] != -1) || ([string first {]}\n> $tool_cmd] != -1)} {\n> > + error_popup [mc \"Unable to process square\n> brackets in mergetool.cmd configuration option.\\\n> > + Please remove\n> the square brackets.\"]\n> > + return\n>\n> Condition and error text are OK. But see below.\n>\n> > + } else {\n> > + foreach command_part $tool_cmd {\n> > + lappend cmdline [subst -nobackslashes\n> -nocommands $command_part]\n> > + }\n>\n> Good.\n>\n> I have seen a few examples in the Tcl manual with lappend in the loop body,\n> and it seems to be customary to set the list variable to an empty value before\n> the loop, i.e.\n>\n> set cmdline {}\n>\n\nDone in next patch.\n\n> > + }\n> > + } else {\n> > + error_popup [mc \"Unsupported merge tool '%s'.\\n\n> > + Currently unsupported\n> tools can be added and used as unsupported tools with degraded support\\\n> > + by adding the\n> command of the tool to the \\\"mergetool.cmd\\\" option in the config.\n> > + See the configuration\n> documentation for more details.\" $tool]\n>\n> This error message needs a bit more work (some of this also applies to the\n> message above):\n>\n> - A tool is only unsupported as long as there is no usable configuration. Once\n> mergetool.$tool.cmd is set to something we can handle, calling the tool\n> \"unsupported\" isn't appropriate, I would think.\n\nJunio C Hamano suggested to mark the tools unsupported in his review.\nI'll change it back and remove the unsupported hint.\n\n> How about\n>\n> Unsupported merge tool '%s'.\n>\n> To use this tool, configure \"mergetool.%s.cmd\" as shown in the git-config\n> manual page.\n>\n\nWill be changed.\n\n> - The configuration variable that we use is not mergetool.cmd, but\n> mergetool.$tool.cmd.\n>\n> - Continuation lines must not be indented. Indented text appears indented in\n> the error message.\n>\n> - Watch out whether an explicit \\n is given, whether the line-break is escaped\n> or not; all of this has meaning.\n>\n> - Looking at other multi-line error messages in git-gui.sh, the convention is\n>\n> mc[\"First paragraph goes here.\n>\n> Second paragraph. All of it is on one line in the source code.\n>\n> Third paragraph. No \\n appears anywhere.\"]\n>\n\nI didn't want to have unindented text ion the source code, but I'll change it.\n\n> > + return\n> > + }\n> > }\n> > }\n>\n> As a matter of personal taste, I prefer to structure code with error exits like so\n> (but it is totally acceptable if you disagree):\n>\n> if {check for error 1} {\n> error msg1\n> return\n> }\n> if {check for error 2} {\n> error msg2\n> return\n> }\n> regular case\n> goes here\n> without indentation\n>\n> Note that there are no else-branches. This reduces the indentation levels.\n>\n\nI tried to set this up - it failed because the square brackets in the if condition suddenly get \"counted\" as command executions or in other words; the curly braces are ignored. I was unable to get around that.\n\n> -- Hannes\n\n\n\n-------------------------------------------------------------------------------------------------\nimperial-Werke oHG, Sitz Bünde, Registergericht Bad Oeynhausen - HRA 4825\n"},{"id":"502621","messageId":"pull.1773.v4.git.1726064619705.gitgitgadget@gmail.com","threadId":"61971","inReplyTo":"pull.1773.v3.git.1725607643479.gitgitgadget@gmail.com","subject":"[PATCH v4] git gui: add directly calling merge tool from configuration","fromName":"ToBoMi via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-09-11T14:23:39Z","receivedAt":"2024-09-11T14:23:44Z","isPatch":true,"sender":{"key":"name:ToBoMi","avatar":null},"body":"From: Tobias Boesch \n\ngit gui can open a merge tool when conflicts are detected (Right click\nin the diff of the file with conflicts).\nThe merge tools that are allowed to use are hard coded into git gui.\n\nIf one wants to add a new merge tool it has to be added to git gui\nthrough a source code change.\nThis is not convenient in comparison to how it works in git (without gui).\n\ngit itself has configuration options for a merge tools path and command\nin the git configuration.\nNew merge tools can be set up there without a source code change.\n\nThose options are used only by pure git in contrast to git gui. git calls\nthe configured merge tools directly from the configuration while git Gui\ndoesn't.\n\nWith this change git gui can call merge tools configured in the\nconfiguration directly without a change in git gui source code.\nIt needs a configured \"merge.tool\" and a configured\n\"mergetool..cmd\" configuration entry as shown in the\ngit-config manual page.\n\nConfiguration example:\n[merge]\n\ttool = vscode\n[mergetool \"vscode\"]\n\tpath = the/path/to/Code.exe\n\tcmd = \\\"Code.exe\\\" --wait --merge \\\"$LOCAL\\\" \\\"$REMOTE\\\" \\\"$BASE\\\" \\\"$MERGED\\\"\n\nWithout the \"mergetool.cmd\" configuration and an unsupported \"merge.tool\"\nentry, git gui behaves mainly as before this change and informs the user\nabout an unsupported merge tool. In addtition it also shows a hint to add\na configuration entry to use the tool as an unsupported tool with degraded\nsupport.\n\nIf a wrong \"mergetool.cmd\" is configured by accident, it gets handled\nby git gui already. In this case git gui informs the user that the merge\ntool couldn't be opened. This behavior is preserved by this change and\nshould not change.\n\n\"Beyond Compare 3\" and \"Visual Studio Code\" were tested as manually\nconfigured merge tools.\n\nSigned-off-by: Tobias Boesch \n---\n git gui: add directly calling merge tool from gitconfig\n \n cc: Johannes Sixt j6t@kdbg.org\n \n Changes since v1:\n \n * Used existing option mergetool.cmd in gitconfig to trigger the direct\n call of the merge tool configured in the config instead adding a new\n option mergeToolFromConfig\n * Removed assignment of merge tool path to a variable and reused the\n already existing one: merget_tool_path\n * Changed formatting of the commit message\n * Added more context and an examples to the commit message\n \n Changes since v2:\n \n * Changed commit ident\n * Added hint to add a mergetool as an unsupprted tool\n * Minor typos\n * Highlighted proper nouns in commit message\n * Only using mergetool.cmd now - not using mergetool.path anymore\n * Removed gitconfig term in user message\n * Changed lines length of commit message\n * tcl commands in mergetool.cmd are now detected and not executed\n anymore\n * mergetool.cmd string parts are now substituted as list, not as a\n whole string\n * Made a more clear user hint on how to configure an unsupported\n mergetool\n \n Changes since v3:\n \n * Corrected wrong configuration option in commit message\n * Replaced \"config\" and \"gitconfig\" with \"configuration\" in commit\n message\n * Initialised cmdline list before appending values in loop\n * Removed hint that unsupported tools have degraded support\n * Changed popup message formatting in the popup and in the source code\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1773%2FToBoMi%2Fadd_merge_tool_from_config_file-v4\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1773/ToBoMi/add_merge_tool_from_config_file-v4\nPull-Request: https://github.com/gitgitgadget/git/pull/1773\n\nRange-diff vs v3:\n\n 1: c8c0107ddc5 ! 1: b5db8997b76 git gui: add directly calling merge tool from gitconfig\n @@ Metadata\n Author: Tobias Boesch \n \n ## Commit message ##\n - git gui: add directly calling merge tool from gitconfig\n + git gui: add directly calling merge tool from configuration\n \n git gui can open a merge tool when conflicts are detected (Right click\n in the diff of the file with conflicts).\n @@ Commit message\n This is not convenient in comparison to how it works in git (without gui).\n \n git itself has configuration options for a merge tools path and command\n - in the git config.\n + in the git configuration.\n New merge tools can be set up there without a source code change.\n \n Those options are used only by pure git in contrast to git gui. git calls\n - the configured merge tools directly from the config while git Gui doesn't.\n + the configured merge tools directly from the configuration while git Gui\n + doesn't.\n \n - With this change git gui can call merge tools configured in the gitconfig\n - directly without a change in git gui source code.\n - It needs a configured merge.tool and a configured mergetool.cmd config\n - entry.\n + With this change git gui can call merge tools configured in the\n + configuration directly without a change in git gui source code.\n + It needs a configured \"merge.tool\" and a configured\n + \"mergetool..cmd\" configuration entry as shown in the\n + git-config manual page.\n \n - gitconfig example:\n + Configuration example:\n [merge]\n tool = vscode\n [mergetool \"vscode\"]\n path = the/path/to/Code.exe\n cmd = \\\"Code.exe\\\" --wait --merge \\\"$LOCAL\\\" \\\"$REMOTE\\\" \\\"$BASE\\\" \\\"$MERGED\\\"\n \n - Without the mergetool.cmd configuration and an unsupported merge.tool\n + Without the \"mergetool.cmd\" configuration and an unsupported \"merge.tool\"\n entry, git gui behaves mainly as before this change and informs the user\n about an unsupported merge tool. In addtition it also shows a hint to add\n - a config entry to use the tool as an unsupported tool with degraded\n + a configuration entry to use the tool as an unsupported tool with degraded\n support.\n \n - If a wrong mergetool.cmd is configured by accident, it gets handled\n + If a wrong \"mergetool.cmd\" is configured by accident, it gets handled\n by git gui already. In this case git gui informs the user that the merge\n tool couldn't be opened. This behavior is preserved by this change and\n should not change.\n @@ git-gui/lib/mergetool.tcl: proc merge_resolve_tool2 {} {\n +\t\tset tool_cmd [get_config mergetool.$tool.cmd]\n +\t\tif {$tool_cmd ne {}} {\n +\t\t\tif {([string first {[} $tool_cmd] != -1) || ([string first {]} $tool_cmd] != -1)} {\n -+\t\t\t\terror_popup [mc \"Unable to process square brackets in mergetool.cmd configuration option.\\\n -+\t\t\t\t\t\t\t\tPlease remove the square brackets.\"]\n ++\t\t\t\terror_popup [mc \"Unable to process square brackets in mergetool.$tool.cmd configuration option.\n ++\n ++Please remove the square brackets.\"]\n +\t\t\t\treturn\n +\t\t\t} else {\n ++\t\t\t\tset cmdline {}\n +\t\t\t\tforeach command_part $tool_cmd {\n +\t\t\t\t\tlappend cmdline [subst -nobackslashes -nocommands $command_part]\n +\t\t\t\t}\n +\t\t\t}\n +\t\t} else {\n -+\t\t\terror_popup [mc \"Unsupported merge tool '%s'.\\n\n -+\t\t\t\t\t\t\tCurrently unsupported tools can be added and used as unsupported tools with degraded support\\\n -+\t\t\t\t\t\t\tby adding the command of the tool to the \\\"mergetool.cmd\\\" option in the config.\n -+\t\t\t\t\t\t\tSee the configuration documentation for more details.\" $tool]\n ++\t\t\terror_popup [mc \"Unsupported merge tool '%s'.\n ++\t\t\t\t\t\t\t\n ++To use this tool, configure \\\"mergetool.%s.cmd\\\" as shown in the git-config\\\n ++manual page.\" $tool $tool]\n +\t\t\treturn\n +\t\t}\n \t}\n\n\n git-gui/lib/mergetool.tcl | 22 ++++++++++++++++++++--\n 1 file changed, 20 insertions(+), 2 deletions(-)\n\ndiff --git a/git-gui/lib/mergetool.tcl b/git-gui/lib/mergetool.tcl\nindex e688b016ef6..2afc875ea0a 100644\n--- a/git-gui/lib/mergetool.tcl\n+++ b/git-gui/lib/mergetool.tcl\n@@ -272,8 +272,26 @@ proc merge_resolve_tool2 {} {\n \t\t}\n \t}\n \tdefault {\n-\t\terror_popup [mc \"Unsupported merge tool '%s'\" $tool]\n-\t\treturn\n+\t\tset tool_cmd [get_config mergetool.$tool.cmd]\n+\t\tif {$tool_cmd ne {}} {\n+\t\t\tif {([string first {[} $tool_cmd] != -1) || ([string first {]} $tool_cmd] != -1)} {\n+\t\t\t\terror_popup [mc \"Unable to process square brackets in mergetool.$tool.cmd configuration option.\n+\n+Please remove the square brackets.\"]\n+\t\t\t\treturn\n+\t\t\t} else {\n+\t\t\t\tset cmdline {}\n+\t\t\t\tforeach command_part $tool_cmd {\n+\t\t\t\t\tlappend cmdline [subst -nobackslashes -nocommands $command_part]\n+\t\t\t\t}\n+\t\t\t}\n+\t\t} else {\n+\t\t\terror_popup [mc \"Unsupported merge tool '%s'.\n+\t\t\t\t\t\t\t\n+To use this tool, configure \\\"mergetool.%s.cmd\\\" as shown in the git-config\\\n+manual page.\" $tool $tool]\n+\t\t\treturn\n+\t\t}\n \t}\n \t}\n \n\nbase-commit: c5ee8f2d1c9d336e0a46139bd35236d4a0bb93ff\n-- \ngitgitgadget\n"},{"id":"502666","messageId":"pull.1773.v5.git.1726136277300.gitgitgadget@gmail.com","threadId":"61971","inReplyTo":"pull.1773.v4.git.1726064619705.gitgitgadget@gmail.com","subject":"[PATCH v5] git gui: add directly calling merge tool from configuration","fromName":"ToBoMi via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-09-12T10:17:57Z","receivedAt":"2024-09-12T10:18:02Z","isPatch":true,"sender":{"key":"name:ToBoMi","avatar":null},"body":"From: Tobias Boesch \n\ngit gui can open a merge tool when conflicts are detected (Right click\nin the diff of the file with conflicts).\nThe merge tools that are allowed to use are hard coded into git gui.\n\nIf one wants to add a new merge tool it has to be added to git gui\nthrough a source code change.\nThis is not convenient in comparison to how it works in git (without gui).\n\ngit itself has configuration options for a merge tools path and command\nin the git configuration.\nNew merge tools can be set up there without a source code change.\n\nThose options are used only by pure git in contrast to git gui. git calls\nthe configured merge tools directly from the configuration while git Gui\ndoesn't.\n\nWith this change git gui can call merge tools configured in the\nconfiguration directly without a change in git gui source code.\nIt needs a configured \"merge.tool\" and a configured\n\"mergetool..cmd\" configuration entry as shown in the\ngit-config manual page.\n\nConfiguration example:\n[merge]\n\ttool = vscode\n[mergetool \"vscode\"]\n\tpath = the/path/to/Code.exe\n\tcmd = \\\"Code.exe\\\" --wait --merge \\\"$LOCAL\\\" \\\"$REMOTE\\\" \\\"$BASE\\\" \\\"$MERGED\\\"\n\nWithout the \"mergetool.cmd\" configuration and an unsupported \"merge.tool\"\nentry, git gui behaves mainly as before this change and informs the user\nabout an unsupported merge tool. In addtition it also shows a hint to add\na configuration entry to use the tool as an unsupported tool with degraded\nsupport.\n\nIf a wrong \"mergetool.cmd\" is configured by accident, it gets handled\nby git gui already. In this case git gui informs the user that the merge\ntool couldn't be opened. This behavior is preserved by this change and\nshould not change.\n\n\"Beyond Compare 3\" and \"Visual Studio Code\" were tested as manually\nconfigured merge tools.\n\nSigned-off-by: Tobias Boesch \n---\n git gui: add directly calling merge tool from gitconfig\n \n cc: Johannes Sixt j6t@kdbg.org\n \n Changes since v1:\n \n * Used existing option mergetool.cmd in gitconfig to trigger the direct\n call of the merge tool configured in the config instead adding a new\n option mergeToolFromConfig\n * Removed assignment of merge tool path to a variable and reused the\n already existing one: merget_tool_path\n * Changed formatting of the commit message\n * Added more context and an examples to the commit message\n \n Changes since v2:\n \n * Changed commit ident\n * Added hint to add a mergetool as an unsupprted tool\n * Minor typos\n * Highlighted proper nouns in commit message\n * Only using mergetool.cmd now - not using mergetool.path anymore\n * Removed gitconfig term in user message\n * Changed lines length of commit message\n * tcl commands in mergetool.cmd are now detected and not executed\n anymore\n * mergetool.cmd string parts are now substituted as list, not as a\n whole string\n * Made a more clear user hint on how to configure an unsupported\n mergetool\n \n Changes since v3:\n \n * Corrected wrong configuration option in commit message\n * Replaced \"config\" and \"gitconfig\" with \"configuration\" in commit\n message\n * Initialised cmdline list before appending values in loop\n * Removed hint that unsupported tools have degraded support\n * Changed popup message formatting in the popup and in the source code\n \n Changes since v4:\n \n * Removed trailing whitespace\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1773%2FToBoMi%2Fadd_merge_tool_from_config_file-v5\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1773/ToBoMi/add_merge_tool_from_config_file-v5\nPull-Request: https://github.com/gitgitgadget/git/pull/1773\n\nRange-diff vs v4:\n\n 1: b5db8997b76 ! 1: c37db60c218 git gui: add directly calling merge tool from configuration\n @@ git-gui/lib/mergetool.tcl: proc merge_resolve_tool2 {} {\n +\t\t\t}\n +\t\t} else {\n +\t\t\terror_popup [mc \"Unsupported merge tool '%s'.\n -+\t\t\t\t\t\t\t\n ++\n +To use this tool, configure \\\"mergetool.%s.cmd\\\" as shown in the git-config\\\n +manual page.\" $tool $tool]\n +\t\t\treturn\n\n\n git-gui/lib/mergetool.tcl | 22 ++++++++++++++++++++--\n 1 file changed, 20 insertions(+), 2 deletions(-)\n\ndiff --git a/git-gui/lib/mergetool.tcl b/git-gui/lib/mergetool.tcl\nindex e688b016ef6..50ed530cbdb 100644\n--- a/git-gui/lib/mergetool.tcl\n+++ b/git-gui/lib/mergetool.tcl\n@@ -272,8 +272,26 @@ proc merge_resolve_tool2 {} {\n \t\t}\n \t}\n \tdefault {\n-\t\terror_popup [mc \"Unsupported merge tool '%s'\" $tool]\n-\t\treturn\n+\t\tset tool_cmd [get_config mergetool.$tool.cmd]\n+\t\tif {$tool_cmd ne {}} {\n+\t\t\tif {([string first {[} $tool_cmd] != -1) || ([string first {]} $tool_cmd] != -1)} {\n+\t\t\t\terror_popup [mc \"Unable to process square brackets in mergetool.$tool.cmd configuration option.\n+\n+Please remove the square brackets.\"]\n+\t\t\t\treturn\n+\t\t\t} else {\n+\t\t\t\tset cmdline {}\n+\t\t\t\tforeach command_part $tool_cmd {\n+\t\t\t\t\tlappend cmdline [subst -nobackslashes -nocommands $command_part]\n+\t\t\t\t}\n+\t\t\t}\n+\t\t} else {\n+\t\t\terror_popup [mc \"Unsupported merge tool '%s'.\n+\n+To use this tool, configure \\\"mergetool.%s.cmd\\\" as shown in the git-config\\\n+manual page.\" $tool $tool]\n+\t\t\treturn\n+\t\t}\n \t}\n \t}\n \n\nbase-commit: c5ee8f2d1c9d336e0a46139bd35236d4a0bb93ff\n-- \ngitgitgadget\n"},{"id":"502788","messageId":"2ee3a148-23eb-48cb-8e10-551437fb03d3@kdbg.org","threadId":"61971","inReplyTo":"pull.1773.v5.git.1726136277300.gitgitgadget@gmail.com","subject":"Re: [PATCH v5] git gui: add directly calling merge tool from configuration","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2024-09-14T13:32:41Z","receivedAt":"2024-09-14T14:06:33Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 12.09.24 um 12:17 schrieb ToBoMi via GitGitGadget:\n> Configuration example:\n> [merge]\n> \ttool = vscode\n> [mergetool \"vscode\"]\n> \tpath = the/path/to/Code.exe\n> \tcmd = \\\"Code.exe\\\" --wait --merge \\\"$LOCAL\\\" \\\"$REMOTE\\\" \\\"$BASE\\\" \\\"$MERGED\\\"\n\nThis example is not up-to-date anymore, is it?\n\nAlso, below are two cases where \"mergetool.cmd\" is mentioned\nincorrectly.\n\n> Without the \"mergetool.cmd\" configuration and an unsupported \"merge.tool\"\n> entry, git gui behaves mainly as before this change and informs the user\n> about an unsupported merge tool. In addtition it also shows a hint to add\n> a configuration entry to use the tool as an unsupported tool with degraded\n> support.\n> \n> If a wrong \"mergetool.cmd\" is configured by accident, it gets handled\n> by git gui already. In this case git gui informs the user that the merge\n> tool couldn't be opened. This behavior is preserved by this change and\n> should not change.\n\n> --- a/git-gui/lib/mergetool.tcl\n> +++ b/git-gui/lib/mergetool.tcl\n> @@ -272,8 +272,26 @@ proc merge_resolve_tool2 {} {\n> \t\t}\n> \t}\n> \tdefault {\n> -\t\terror_popup [mc \"Unsupported merge tool '%s'\" $tool]\n> -\t\treturn\n> +\t\tset tool_cmd [get_config mergetool.$tool.cmd]\n> +\t\tif {$tool_cmd ne {}} {\n> +\t\t\tif {([string first {[} $tool_cmd] != -1) || ([string first {]} $tool_cmd] != -1)} {\n> +\t\t\t\terror_popup [mc \"Unable to process square brackets in mergetool.$tool.cmd configuration option.\n\nThis $tool in the format string breaks [mc]. It must be %s and an\nargument. I'll fix this up while queuing.\n\n> +\n> +Please remove the square brackets.\"]\n> +\t\t\t\treturn\n> +\t\t\t} else {\n> +\t\t\t\tset cmdline {}\n> +\t\t\t\tforeach command_part $tool_cmd {\n> +\t\t\t\t\tlappend cmdline [subst -nobackslashes -nocommands $command_part]\n> +\t\t\t\t}\n> +\t\t\t}\n> +\t\t} else {\n> +\t\t\terror_popup [mc \"Unsupported merge tool '%s'.\n> +\n> +To use this tool, configure \\\"mergetool.%s.cmd\\\" as shown in the git-config\\> +manual page.\" $tool $tool]\n\nI am surprised that the backslash does not paste the two lines together\nwithout a space. \"git-config\" and \"manual\" do appear as separate words\nin the error message. Nevertheless, since I do not know how this pans\nout in the translation files, I will remove the line continuation and\nwrite all on one line.\n\n> +\t\t\treturn\n> +\t\t}\n> \t}\n> \t}\n\nThank you for your contribution! Below is the range-diff between this\nsubmission and the queued version.\n\n-- Hannes\n\n1: 03e92d6 ! 1: 8ff65c7 git gui: add directly calling merge tool from configuration\n @@ Commit message\n [merge]\n tool = vscode\n [mergetool \"vscode\"]\n - path = the/path/to/Code.exe\n - cmd = \\\"Code.exe\\\" --wait --merge \\\"$LOCAL\\\" \\\"$REMOTE\\\" \\\"$BASE\\\" \\\"$MERGED\\\"\n + cmd = \\\"the/path/to/Code.exe\\\" --wait --merge \\\"$LOCAL\\\" \\\"$REMOTE\\\" \\\"$BASE\\\" \\\"$MERGED\\\"\n \n - Without the \"mergetool.cmd\" configuration and an unsupported \"merge.tool\"\n - entry, git gui behaves mainly as before this change and informs the user\n - about an unsupported merge tool. In addtition it also shows a hint to add\n - a configuration entry to use the tool as an unsupported tool with degraded\n - support.\n + Without the \"mergetool..cmd\" entry and an unsupported\n + \"merge.tool\" entry, git gui behaves mainly as before this change and\n + informs the user about an unsupported merge tool. In addtition, it also\n + shows a hint to add a configuration entry to use the tool as an\n + unsupported tool with degraded support.\n \n - If a wrong \"mergetool.cmd\" is configured by accident, it gets handled\n - by git gui already. In this case git gui informs the user that the merge\n - tool couldn't be opened. This behavior is preserved by this change and\n - should not change.\n + If a wrong \"mergetool..cmd\" is configured by accident,\n + it gets handled by git gui already. In this case git gui informs the\n + user that the merge tool couldn't be opened. This behavior is preserved\n + by this change and should not change.\n \n \"Beyond Compare 3\" and \"Visual Studio Code\" were tested as manually\n configured merge tools.\n \n Signed-off-by: Tobias Boesch \n + Signed-off-by: Johannes Sixt \n \n ## lib/mergetool.tcl ##\n @@ lib/mergetool.tcl: proc merge_resolve_tool2 {} {\n @@ lib/mergetool.tcl: proc merge_resolve_tool2 {} {\n +\t\tset tool_cmd [get_config mergetool.$tool.cmd]\n +\t\tif {$tool_cmd ne {}} {\n +\t\t\tif {([string first {[} $tool_cmd] != -1) || ([string first {]} $tool_cmd] != -1)} {\n -+\t\t\t\terror_popup [mc \"Unable to process square brackets in mergetool.$tool.cmd configuration option.\n ++\t\t\t\terror_popup [mc \"Unable to process square brackets in \\\"mergetool.%s.cmd\\\" configuration option.\n +\n -+Please remove the square brackets.\"]\n ++Please remove the square brackets.\" $tool]\n +\t\t\t\treturn\n +\t\t\t} else {\n +\t\t\t\tset cmdline {}\n @@ lib/mergetool.tcl: proc merge_resolve_tool2 {} {\n +\t\t} else {\n +\t\t\terror_popup [mc \"Unsupported merge tool '%s'.\n +\n -+To use this tool, configure \\\"mergetool.%s.cmd\\\" as shown in the git-config\\\n -+manual page.\" $tool $tool]\n ++To use this tool, configure \\\"mergetool.%s.cmd\\\" as shown in the git-config manual page.\" $tool $tool]\n +\t\t\treturn\n +\t\t}\n \t}\n\n"},{"id":"502835","messageId":"AS2PR08MB8288A8DC805BF4FFB67B1709E1602@AS2PR08MB8288.eurprd08.prod.outlook.com","threadId":"61971","inReplyTo":"2ee3a148-23eb-48cb-8e10-551437fb03d3@kdbg.org","subject":"AW: [PATCH v5] git gui: add directly calling merge tool from configuration","fromName":"tobias.boesch@miele.com","fromEmail":"tobias.boesch@miele.com","sentAt":"2024-09-16T08:42:06Z","receivedAt":"2024-09-16T08:43:03Z","isPatch":true,"sender":{"key":"tobias.boesch@miele.com","avatar":"https://avatars.githubusercontent.com/u/64197724?v=4"},"body":"\n\n> -----Ursprüngliche Nachricht-----\n> Von: Johannes Sixt \n> Gesendet: Samstag, 14. September 2024 15:33\n> An: Boesch, Tobias \n> Cc: git@vger.kernel.org; ToBoMi via GitGitGadget \n> Betreff: Re: [PATCH v5] git gui: add directly calling merge tool from\n> configuration\n>\n> Am 12.09.24 um 12:17 schrieb ToBoMi via GitGitGadget:\n> > Configuration example:\n> > [merge]\n> > tool = vscode\n> > [mergetool \"vscode\"]\n> > path = the/path/to/Code.exe\n> > cmd = \\\"Code.exe\\\" --wait --merge \\\"$LOCAL\\\" \\\"$REMOTE\\\"\n> \\\"$BASE\\\" \\\"$MERGED\\\"\n>\n> This example is not up-to-date anymore, is it?\n>\n> Also, below are two cases where \"mergetool.cmd\" is mentioned incorrectly.\n>\n> > Without the \"mergetool.cmd\" configuration and an unsupported\n> \"merge.tool\"\n> > entry, git gui behaves mainly as before this change and informs the\n> > user about an unsupported merge tool. In addtition it also shows a\n> > hint to add a configuration entry to use the tool as an unsupported\n> > tool with degraded support.\n> >\n> > If a wrong \"mergetool.cmd\" is configured by accident, it gets handled\n> > by git gui already. In this case git gui informs the user that the\n> > merge tool couldn't be opened. This behavior is preserved by this\n> > change and should not change.\n>\n> > --- a/git-gui/lib/mergetool.tcl\n> > +++ b/git-gui/lib/mergetool.tcl\n> > @@ -272,8 +272,26 @@ proc merge_resolve_tool2 {} {\n> > }\n> > }\n> > default {\n> > - error_popup [mc \"Unsupported merge tool '%s'\" $tool]\n> > - return\n> > + set tool_cmd [get_config mergetool.$tool.cmd]\n> > + if {$tool_cmd ne {}} {\n> > + if {([string first {[} $tool_cmd] != -1) || ([string first {]}\n> $tool_cmd] != -1)} {\n> > + error_popup [mc \"Unable to process square\n> brackets in mergetool.$tool.cmd configuration option.\n>\n> This $tool in the format string breaks [mc]. It must be %s and an argument. I'll\n> fix this up while queuing.\n>\n> > +\n> > +Please remove the square brackets.\"]\n> > + return\n> > + } else {\n> > + set cmdline {}\n> > + foreach command_part $tool_cmd {\n> > + lappend cmdline [subst -nobackslashes\n> -nocommands $command_part]\n> > + }\n> > + }\n> > + } else {\n> > + error_popup [mc \"Unsupported merge tool '%s'.\n> > +\n> > +To use this tool, configure \\\"mergetool.%s.cmd\\\" as shown in the\n> > +git-config\\> +manual page.\" $tool $tool]\n>\n> I am surprised that the backslash does not paste the two lines together\n> without a space. \"git-config\" and \"manual\" do appear as separate words in the\n> error message. Nevertheless, since I do not know how this pans out in the\n> translation files, I will remove the line continuation and write all on one line.\n>\n\nTrue I also don't know why.\nYou could add a whitespace after the newline and have code matching the documentation of tcl:\n\n\"\\whiteSpace\nA single space character replaces the backslash, newline, and all spaces and tabs after the newline. [...]\"\nFrom https://www.tcl.tk/man/tcl8.7/TclCmd/Tcl.html#M24\n\nThat doesn't change the error message (stays good) in my tests and makes the code compliant to the tcl docs.\n\n> > + return\n> > + }\n> > }\n> > }\n>\n> Thank you for your contribution! Below is the range-diff between this\n> submission and the queued version.\n>\n\nThank you for fixing the issues left open and your patient review.\n(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)\n\n> -- Hannes\n>\n> 1: 03e92d6 ! 1: 8ff65c7 git gui: add directly calling merge tool from\n> configuration\n> @@ Commit message\n> [merge]\n> tool = vscode\n> [mergetool \"vscode\"]\n> - path = the/path/to/Code.exe\n> - cmd = \\\"Code.exe\\\" --wait --merge \\\"$LOCAL\\\" \\\"$REMOTE\\\"\n> \\\"$BASE\\\" \\\"$MERGED\\\"\n> + cmd = \\\"the/path/to/Code.exe\\\" --wait --merge \\\"$LOCAL\\\"\n> \\\"$REMOTE\\\" \\\"$BASE\\\" \\\"$MERGED\\\"\n>\n> - Without the \"mergetool.cmd\" configuration and an unsupported\n> \"merge.tool\"\n> - entry, git gui behaves mainly as before this change and informs the user\n> - about an unsupported merge tool. In addtition it also shows a hint to add\n> - a configuration entry to use the tool as an unsupported tool with\n> degraded\n> - support.\n> + Without the \"mergetool..cmd\" entry and an\n> unsupported\n> + \"merge.tool\" entry, git gui behaves mainly as before this change and\n> + informs the user about an unsupported merge tool. In addtition, it also\n> + shows a hint to add a configuration entry to use the tool as an\n> + unsupported tool with degraded support.\n>\n> - If a wrong \"mergetool.cmd\" is configured by accident, it gets handled\n> - by git gui already. In this case git gui informs the user that the merge\n> - tool couldn't be opened. This behavior is preserved by this change and\n> - should not change.\n> + If a wrong \"mergetool..cmd\" is configured by\n> accident,\n> + it gets handled by git gui already. In this case git gui informs the\n> + user that the merge tool couldn't be opened. This behavior is preserved\n> + by this change and should not change.\n>\n> \"Beyond Compare 3\" and \"Visual Studio Code\" were tested as manually\n> configured merge tools.\n>\n> Signed-off-by: Tobias Boesch \n> + Signed-off-by: Johannes Sixt \n>\n> ## lib/mergetool.tcl ##\n> @@ lib/mergetool.tcl: proc merge_resolve_tool2 {} {\n> @@ lib/mergetool.tcl: proc merge_resolve_tool2 {} {\n> + set tool_cmd [get_config mergetool.$tool.cmd]\n> + if {$tool_cmd ne {}} {\n> + if {([string first {[} $tool_cmd] != -1) || ([string first {]}\n> $tool_cmd] != -1)} {\n> -+ error_popup [mc \"Unable to process square\n> brackets in mergetool.$tool.cmd configuration option.\n> ++ error_popup [mc \"Unable to process square\n> brackets in \\\"mergetool.%s.cmd\\\" configuration option.\n> +\n> -+Please remove the square brackets.\"]\n> ++Please remove the square brackets.\" $tool]\n> + return\n> + } else {\n> + set cmdline {}\n> @@ lib/mergetool.tcl: proc merge_resolve_tool2 {} {\n> + } else {\n> + error_popup [mc \"Unsupported merge tool '%s'.\n> +\n> -+To use this tool, configure \\\"mergetool.%s.cmd\\\" as shown in the git-\n> config\\\n> -+manual page.\" $tool $tool]\n> ++To use this tool, configure \\\"mergetool.%s.cmd\\\" as shown in the git-\n> config manual page.\" $tool $tool]\n> + return\n> + }\n> }\n\n\n\n-------------------------------------------------------------------------------------------------\nimperial-Werke oHG, Sitz Bünde, Registergericht Bad Oeynhausen - HRA 4825\n"},{"id":"506803","messageId":"AS2PR08MB828858E352C40E3488B1F5D0E15C2@AS2PR08MB8288.eurprd08.prod.outlook.com","threadId":"61971","inReplyTo":"AS2PR08MB8288A8DC805BF4FFB67B1709E1602@AS2PR08MB8288.eurprd08.prod.outlook.com","subject":"AW: [PATCH v5] git gui: add directly calling merge tool from configuration","fromName":"tobias.boesch@miele.com","fromEmail":"tobias.boesch@miele.com","sentAt":"2024-11-07T14:16:06Z","receivedAt":"2024-11-07T14:17:02Z","isPatch":true,"sender":{"key":"tobias.boesch@miele.com","avatar":"https://avatars.githubusercontent.com/u/64197724?v=4"},"body":"> -----Ursprüngliche Nachricht-----\n> Von: Boesch, Tobias\n> Gesendet: Montag, 16. September 2024 10:42\n> An: Johannes Sixt \n> Cc: git@vger.kernel.org; ToBoMi via GitGitGadget \n> Betreff: AW: [PATCH v5] git gui: add directly calling merge tool from\n> configuration\n>\n>\n>\n> > -----Ursprüngliche Nachricht-----\n> > Von: Johannes Sixt \n> > Gesendet: Samstag, 14. September 2024 15:33\n> > An: Boesch, Tobias \n> > Cc: git@vger.kernel.org; ToBoMi via GitGitGadget\n> > \n> > Betreff: Re: [PATCH v5] git gui: add directly calling merge tool from\n> > configuration\n> >\n> > Am 12.09.24 um 12:17 schrieb ToBoMi via GitGitGadget:\n> > > Configuration example:\n> > > [merge]\n> > > tool = vscode\n> > > [mergetool \"vscode\"]\n> > > path = the/path/to/Code.exe\n> > > cmd = \\\"Code.exe\\\" --wait --merge \\\"$LOCAL\\\" \\\"$REMOTE\\\"\n> > \\\"$BASE\\\" \\\"$MERGED\\\"\n> >\n> > This example is not up-to-date anymore, is it?\n> >\n> > Also, below are two cases where \"mergetool.cmd\" is mentioned incorrectly.\n> >\n> > > Without the \"mergetool.cmd\" configuration and an unsupported\n> > \"merge.tool\"\n> > > entry, git gui behaves mainly as before this change and informs the\n> > > user about an unsupported merge tool. In addtition it also shows a\n> > > hint to add a configuration entry to use the tool as an unsupported\n> > > tool with degraded support.\n> > >\n> > > If a wrong \"mergetool.cmd\" is configured by accident, it gets\n> > > handled by git gui already. In this case git gui informs the user\n> > > that the merge tool couldn't be opened. This behavior is preserved\n> > > by this change and should not change.\n> >\n> > > --- a/git-gui/lib/mergetool.tcl\n> > > +++ b/git-gui/lib/mergetool.tcl\n> > > @@ -272,8 +272,26 @@ proc merge_resolve_tool2 {} {\n> > > }\n> > > }\n> > > default {\n> > > - error_popup [mc \"Unsupported merge tool '%s'\" $tool]\n> > > - return\n> > > + set tool_cmd [get_config mergetool.$tool.cmd]\n> > > + if {$tool_cmd ne {}} {\n> > > + if {([string first {[} $tool_cmd] != -1) || ([string first {]}\n> > $tool_cmd] != -1)} {\n> > > + error_popup [mc \"Unable to process square\n> > brackets in mergetool.$tool.cmd configuration option.\n> >\n> > This $tool in the format string breaks [mc]. It must be %s and an\n> > argument. I'll fix this up while queuing.\n> >\n> > > +\n> > > +Please remove the square brackets.\"]\n> > > + return\n> > > + } else {\n> > > + set cmdline {}\n> > > + foreach command_part $tool_cmd {\n> > > + lappend cmdline [subst -nobackslashes\n> > -nocommands $command_part]\n> > > + }\n> > > + }\n> > > + } else {\n> > > + error_popup [mc \"Unsupported merge tool '%s'.\n> > > +\n> > > +To use this tool, configure \\\"mergetool.%s.cmd\\\" as shown in the\n> > > +git-config\\> +manual page.\" $tool $tool]\n> >\n> > I am surprised that the backslash does not paste the two lines\n> > together without a space. \"git-config\" and \"manual\" do appear as\n> > separate words in the error message. Nevertheless, since I do not know\n> > how this pans out in the translation files, I will remove the line continuation\n> and write all on one line.\n> >\n>\n> True I also don't know why.\n> You could add a whitespace after the newline and have code matching the\n> documentation of tcl:\n>\n> \"\\whiteSpace\n> A single space character replaces the backslash, newline, and all spaces and\n> tabs after the newline. [...]\"\n> From https://www.tcl.tk/man/tcl8.7/TclCmd/Tcl.html#M24\n>\n> That doesn't change the error message (stays good) in my tests and makes the\n> code compliant to the tcl docs.\n>\n> > > + return\n> > > + }\n> > > }\n> > > }\n> >\n> > Thank you for your contribution! Below is the range-diff between this\n> > submission and the queued version.\n> >\n>\n> Thank you for fixing the issues left open and your patient review.\n> (Based on your comments and if there is no further notice - I assume that this\n> patch will be processed by your side without further submissions from my\n> side)\n\nI 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.\nThe review of this change is finished as far as I understand.\nThe 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\".\nSince 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.\n\nCan someone help me understanding this?\n\nTobias\n>\n> > -- Hannes\n> >\n> > 1: 03e92d6 ! 1: 8ff65c7 git gui: add directly calling merge tool\n> > from configuration\n> > @@ Commit message\n> > [merge]\n> > tool = vscode\n> > [mergetool \"vscode\"]\n> > - path = the/path/to/Code.exe\n> > - cmd = \\\"Code.exe\\\" --wait --merge \\\"$LOCAL\\\" \\\"$REMOTE\\\"\n> > \\\"$BASE\\\" \\\"$MERGED\\\"\n> > + cmd = \\\"the/path/to/Code.exe\\\" --wait --merge \\\"$LOCAL\\\"\n> > \\\"$REMOTE\\\" \\\"$BASE\\\" \\\"$MERGED\\\"\n> >\n> > - Without the \"mergetool.cmd\" configuration and an unsupported\n> > \"merge.tool\"\n> > - entry, git gui behaves mainly as before this change and informs the user\n> > - about an unsupported merge tool. In addtition it also shows a hint to\n> add\n> > - a configuration entry to use the tool as an unsupported tool with\n> > degraded\n> > - support.\n> > + Without the \"mergetool..cmd\" entry and an\n> > unsupported\n> > + \"merge.tool\" entry, git gui behaves mainly as before this change and\n> > + informs the user about an unsupported merge tool. In addtition, it also\n> > + shows a hint to add a configuration entry to use the tool as an\n> > + unsupported tool with degraded support.\n> >\n> > - If a wrong \"mergetool.cmd\" is configured by accident, it gets handled\n> > - by git gui already. In this case git gui informs the user that the merge\n> > - tool couldn't be opened. This behavior is preserved by this change and\n> > - should not change.\n> > + If a wrong \"mergetool..cmd\" is configured by\n> > accident,\n> > + it gets handled by git gui already. In this case git gui informs the\n> > + user that the merge tool couldn't be opened. This behavior is preserved\n> > + by this change and should not change.\n> >\n> > \"Beyond Compare 3\" and \"Visual Studio Code\" were tested as manually\n> > configured merge tools.\n> >\n> > Signed-off-by: Tobias Boesch \n> > + Signed-off-by: Johannes Sixt \n> >\n> > ## lib/mergetool.tcl ##\n> > @@ lib/mergetool.tcl: proc merge_resolve_tool2 {} {\n> > @@ lib/mergetool.tcl: proc merge_resolve_tool2 {} {\n> > + set tool_cmd [get_config mergetool.$tool.cmd]\n> > + if {$tool_cmd ne {}} {\n> > + if {([string first {[} $tool_cmd] != -1) || ([string first {]}\n> > $tool_cmd] != -1)} {\n> > -+ error_popup [mc \"Unable to process square\n> > brackets in mergetool.$tool.cmd configuration option.\n> > ++ error_popup [mc \"Unable to process square\n> > brackets in \\\"mergetool.%s.cmd\\\" configuration option.\n> > +\n> > -+Please remove the square brackets.\"]\n> > ++Please remove the square brackets.\" $tool]\n> > + return\n> > + } else {\n> > + set cmdline {}\n> > @@ lib/mergetool.tcl: proc merge_resolve_tool2 {} {\n> > + } else {\n> > + error_popup [mc \"Unsupported merge tool '%s'.\n> > +\n> > -+To use this tool, configure \\\"mergetool.%s.cmd\\\" as shown in the\n> > git- config\\\n> > -+manual page.\" $tool $tool]\n> > ++To use this tool, configure \\\"mergetool.%s.cmd\\\" as shown in the\n> > git- config manual page.\" $tool $tool]\n> > + return\n> > + }\n> > }\n\n\n\n-------------------------------------------------------------------------------------------------\nimperial-Werke oHG, Sitz Bünde, Registergericht Bad Oeynhausen - HRA 4825\n"},{"id":"506806","messageId":"8228ce55-0603-4cc2-9cea-d65a3caa5ef0@kdbg.org","threadId":"61971","inReplyTo":"AS2PR08MB828858E352C40E3488B1F5D0E15C2@AS2PR08MB8288.eurprd08.prod.outlook.com","subject":"Re: [PATCH v5] git gui: add directly calling merge tool from configuration","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2024-11-07T16:43:29Z","receivedAt":"2024-11-07T16:43:48Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 07.11.24 um 15:16 schrieb tobias.boesch@miele.com:\n> The documentation (https://git-scm.com/docs/ \n> MyFirstContribution#after-approval) says that my \"change will be \n> placed into seen fairly early on by the maintainer while it is still \n> in the review process\".\nThis document is only about contributions to git.git. Git-GUI has its\nown repository and workflow.\n\n> Since I cannot find it in seen or anywhere else, I wonder if there\n> is something wrong, if it just takes a little longer than I expected\n> it to be merged or if this change is merged somewhere else.\nThere is nothing wrong, except that it's on me now to submit a pull\nrequest to the Git maintainer. This should happen in the next days.\n\n-- Hannes\n\n"}]}