git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "ToBoMi via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org,  ToBoMi <tobias.boesch@miele.com>
Subject: Re: [PATCH v2] git gui: add directly calling merge tool from gitconfig
Date: Wed, 28 Aug 2024 10:08:15 -0700	[thread overview]
Message-ID: <xmqq7cc01sow.fsf@gitster.g> (raw)
In-Reply-To: <pull.1773.v2.git.1724833917245.gitgitgadget@gmail.com> (ToBoMi via GitGitGadget's message of "Wed, 28 Aug 2024 08:31:56 +0000")

"ToBoMi via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: deboeto <tobias.boesch@miele.com>

Use the same ident (human readable name plus e-mail address) you
have on your Signed-off-by: line below for this "From: " in-body
header.

> git gui can open a merge tool when conflicts are
> detected (Right click in the diff of the file with
> conflicts).
> The merge tools that are allowed to
> use are hard coded into git gui.
>
> If one wants to add a new merge tool it has to be
> added to git gui through a source code change.
> This is not convenient in comparison to how it
> works in git (without gui).
>
> git itself has configuration options for a merge tools
> path and command in the git config.
> New merge tools can be set up there without a
> source code change.

Even if you configure an unknown tool, it would not get any benefit
from what git-{diff,merge}tool--lib.sh gives the known diff/merge
backends, would it?  Instead of a more thorough support for known
tools done in setup_tool(), an unknown tool would be handled by
setup_user_tool() in git-mergetool-lib.sh which gives somewhat
degraded support.

So "can be set up without" may be true, but giving an impression
that a tool that is set up like so would work just like a known tool
is misleading.

By the way, we do ask contributors to avoid overly long lines,
50-col limt is a bit overly short and makes the resulting text
harder to read than necessary.

> Those options are used only by pure git in
> contrast to git gui. git calls the configured
> merge tools directly from the config while git
> Gui doesn't.
>
> With this change git gui can call merge tools
> configured in the gitconfig directly without a
> change in git gui source code.
> It needs a configured merge.tool and a configured
> mergetool.cmd config entry.

OK.

> gitconfig example:
> [merge]
> 	tool = vscode
> [mergetool "vscode"]
> 	path = the/path/to/Code.exe
> 	cmd = \"Code.exe\" --wait --merge \"$LOCAL\" \"$REMOTE\" \"$BASE\" \"$MERGED\"
>
> Without the mergetool.cmd configuration and an
> unsupported merge.tool entry, git gui behaves
> mainly as before this change and informs the user
> about an unsupported merge tool, but now also
> shows a hint to add a config entry for the tool
> in gitconfig.
>
> If a wrong mergetool.cmd is configured by accident
> it is beeing handled by git gui already. In this

"is beeing" -> "is being", but "it gets handled by Git GUI already"
should be sufficient.

> case git gui informs the user that the merge tool
> couldn't be opened. This behavior is preserved by
> this change and should not change.
>
> Beyond compare 3 and Visual Studio code were
> tested as manually configured merge tools.

Quote proper nouns for readability?  E.g. 

    "Beyond Compare 3" and "Visual Studio Code" were ...

Thanks.

  reply	other threads:[~2024-08-28 17:08 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-19 11:29 [PATCH] git gui: add directly calling merge tool from gitconfig ToBoMi via GitGitGadget
2024-08-24 13:38 ` Johannes Sixt
2024-08-27 12:51   ` AW: " tobias.boesch
2024-08-27 13:53   ` tobias.boesch
2024-08-28  8:31 ` [PATCH v2] " ToBoMi via GitGitGadget
2024-08-28 17:08   ` Junio C Hamano [this message]
2024-09-05  8:09     ` AW: " tobias.boesch
2024-08-31 13:51   ` Johannes Sixt
2024-09-06  6:32     ` AW: " tobias.boesch
2024-09-06 17:43       ` Johannes Sixt
2024-09-06  7:27   ` [PATCH v3] " ToBoMi via GitGitGadget
2024-09-08 12:21     ` Johannes Sixt
2024-09-11 13:41       ` AW: " tobias.boesch
2024-09-11 14:23     ` [PATCH v4] git gui: add directly calling merge tool from configuration ToBoMi via GitGitGadget
2024-09-12 10:17       ` [PATCH v5] " ToBoMi via GitGitGadget
2024-09-14 13:32         ` Johannes Sixt
2024-09-16  8:42           ` AW: " tobias.boesch
2024-11-07 14:16             ` tobias.boesch
2024-11-07 16:43               ` Johannes Sixt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq7cc01sow.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=tobias.boesch@miele.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).