> -----Ursprüngliche Nachricht----- > Von: Johannes Sixt <j6t@xxxxxxxx> > Gesendet: Samstag, 24. August 2024 15:38 > An: Boesch, Tobias <tobias.boesch@xxxxxxxxx> > Cc: git@xxxxxxxxxxxxxxx; ToBoMi via GitGitGadget <gitgitgadget@xxxxxxxxx> > Betreff: Re: [PATCH] git gui: add directly calling merge tool from gitconfig > Continuing my incomplete last reply: > Am 19.08.24 um 13:29 schrieb ToBoMi via GitGitGadget: > > From: deboeto <tobias.boesch@xxxxxxxxx> > > > > * git Gui can open a merge tool when conflicts are > > detected. The merge tools that are allowed to > > call have to be hard coded into git Gui > > althgough there are configuration options for > > merge tools git in the git config. Git calls > > the configured merge tools directly from the > > config while git Gui doesn't. > > * git Gui can now call the tool configured in the > > gitconfig directly. > > * Can be enabled through setting > > gui.mergeToolFromConfig > > Can we do better than having a new configuration variable? Let's say you have > configured merge.tool=vscode. This tool is not supported, but you have > configured mergetool.vscode.cmd suitably. Can we not use the latter > configuration variable unconditionally? > > Likewise, say, you have configured merge.tool=bc3. This one *is* supported. > What could go wrong if mergetool.bc3.cmd is used instead of the built-in > command line? The behavior would change for users that configured > mergetool.$tool.cmd for a supported tool. But would it change for the worse? > > BTW, the code builds different command lines depending on whether a base > file is available or not. How does mergetool.$tool.cmd handle the cases? > > > * Disabled by default, since option is most likely > > never set > > * bc3 and vscode tested > > > > Signed-off-by: deboeto <tobias.boesch@xxxxxxxxx> > > Some remarks on the commit message: > > - The Signed-off-by line has legal consequences. Therefore, we require that > authors use their genuine name, not an alias. Also, the From line must match > the Signed-off-by line. > > - Please have a look at the commit messages in the code base. The formatting > presented here is very unusual. Please write in full sentences including > punctuation, and use paragraphs where needed. > > - Please state the problem that is being solved (in present tense). This should > motivate the change, i.e., provide a convincing argument why the change is > needed. Then state what the solution is in imperative mood, that is, an > instruction to the code to change in such and such way. Use examples to > clarify how the new feature can be used. > > > --- > > git gui: add directly calling merge tool from gitconfig > > > > Published-As: > > https://github.com/gitgitgadget/git/releases/tag/pr- > 1773%2FToBoMi%2Fad > > d_merge_tool_from_config_file-v1 > > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git > > pr-1773/ToBoMi/add_merge_tool_from_config_file-v1 > > Pull-Request: https://github.com/gitgitgadget/git/pull/1773 > > > > Documentation/config/gui.txt | 4 ++++ > > git-gui/lib/mergetool.tcl | 11 +++++++++-- > > 2 files changed, 13 insertions(+), 2 deletions(-) > > > > diff --git a/Documentation/config/gui.txt > > b/Documentation/config/gui.txt index 171be774d24..e63d0b46e7c > 100644 > > --- a/Documentation/config/gui.txt > > +++ b/Documentation/config/gui.txt > > @@ -55,3 +55,7 @@ gui.blamehistoryctx:: > > linkgit:gitk[1] for the selected commit, when the `Show History > > Context` menu item is invoked from 'git gui blame'. If this > > variable is set to zero, the whole history is shown. > > + > > +gui.mergeToolFromConfig:: > > + If true, allow to call the merge tool configured in gitconfig > > + in git gui directly. > > \ No newline at end of file > > Unfortunately, Documentation/config/gui.txt is not part of the Git GUI > repository. Any changes to the documentation must be submitted as separate > patch. > Configuration option will be removed in the next patch version. Therefore the documentation change is no longer needed. > Please be careful not to introduce an incomplete last lines. Take note of "No > newline at end of file". It should not be there. > See above. > > diff --git a/git-gui/lib/mergetool.tcl b/git-gui/lib/mergetool.tcl > > index e688b016ef6..fbd0889612a 100644 > > --- a/git-gui/lib/mergetool.tcl > > +++ b/git-gui/lib/mergetool.tcl > > @@ -272,8 +272,15 @@ proc merge_resolve_tool2 {} { > > } > > } > > default { > > - error_popup [mc "Unsupported merge tool '%s'" $tool] > > - return > > + if {[is_config_true gui.mergetoolfromconfig]} { > > + set path [get_config mergetool.$tool.path] > > At this point, the value assigned to $path here is already available in > $merge_tool_path. > True - corrected in the next patch. > > + set cmdline_config [get_config mergetool.$tool.cmd] > > + set cmdline_substituted [subst -nobackslashes - > nocommands $cmdline_config] > > + set cmdline [lreplace $cmdline_substituted 0 0 $path] > > I haven't yet taken the time to study what these lines do (I am far from fluent > in Tcl) and have no opinion, yet. > They replace the variables one can put into mergetool.cmd like $REMOTE or $LOCAL. Without this substitution command they are not replaced with the real file paths. See this example for vscode: cmd = \"Code.exe\" --wait --merge \"$LOCAL\" \"$REMOTE\" \"$BASE\" \"$MERGED\" > > + } else { > > + error_popup [mc "Unsupported merge tool '%s'" > $tool] > > + return > > + } > > } > > } > > > > > > base-commit: b9849e4f7631d80f146d159bf7b60263b3205632 > > -- Hannes ------------------------------------------------------------------------------------------------- imperial-Werke oHG, Sitz Bünde, Registergericht Bad Oeynhausen - HRA 4825