* [PATCH] documentation: git-mergetool updated to warn against builtin tools invocations
@ 2010-06-02 17:57 Sylvain Rabot
2010-06-02 23:38 ` Junio C Hamano
0 siblings, 1 reply; 6+ messages in thread
From: Sylvain Rabot @ 2010-06-02 17:57 UTC (permalink / raw)
To: git; +Cc: gitster, sylvain, Sylvain Rabot
For known tools such as meld, diffuse, p4merge, git-mergetool ignores the
mergetool.<tool>.* configurations. It took me a while to understand why my
mergetool.diffuse.cmd configuration was not taken care of.
This documentation update warns against this behavior and explains how
to setup a customized command line invocation for known diff tools.
Signed-off-by: Sylvain Rabot <srabot@steek.com>
---
Documentation/git-mergetool.txt | 15 +++++++++++++++
1 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index 55735fa..ac41d7c 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -45,6 +45,21 @@ Instead of running one of the known merge tool programs,
by specifying the command line to invoke in a configuration
variable `mergetool.<tool>.cmd`.
+
+Be aware that 'git mergetool' has some predefined invocation commands builtin
+for known diff tools like meld, diffuse, p4merge ... etc. It means that if
+you set the merge.tool configuration to one of these tools, the
+mergetool.<tool>.* configurations will not be taken care of. If you
+really want to customize the invocation of one of these tools,
+set `merge.tool` to "custom" or whatever you want and `mergetool.custom.cmd`
+to "/usr/bin/<tool> $LOCAL $MERGED $REMOTE"
++
+-------------
+[merge]
+ tool = diffuse-custom
+[mergetool "diffuse-custom"]
+ cmd = diffuse $LOCAL $MERGED $REMOTE
+-------------
++
When 'git mergetool' is invoked with this tool (either through the
`-t` or `--tool` option or the `merge.tool` configuration
variable) the configured command line will be invoked with `$BASE`
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] documentation: git-mergetool updated to warn against builtin tools invocations
2010-06-02 17:57 [PATCH] documentation: git-mergetool updated to warn against builtin tools invocations Sylvain Rabot
@ 2010-06-02 23:38 ` Junio C Hamano
2010-06-03 7:45 ` Michael J Gruber
2010-06-03 10:55 ` Sylvain Rabot
0 siblings, 2 replies; 6+ messages in thread
From: Junio C Hamano @ 2010-06-02 23:38 UTC (permalink / raw)
To: Sylvain Rabot; +Cc: git, sylvain, David Aguilar
Sylvain Rabot <srabot@steek.com> writes:
> +Be aware that 'git mergetool' has some predefined invocation commands builtin
> +for known diff tools like meld, diffuse, p4merge ... etc. It means that if
> +you set the merge.tool configuration to one of these tools, the
> +mergetool.<tool>.* configurations will not be taken care of. If you
> +really want to customize the invocation of one of these tools,
> +set `merge.tool` to "custom" or whatever you want and `mergetool.custom.cmd`
> +to "/usr/bin/<tool> $LOCAL $MERGED $REMOTE"
Two half-points and three points (that makes them four in total ;-):
o If I read the above without "It means that", it still makes sense;
o "If you really" can go without "really";
* I had to read "will not be taken care of" twice; "are ignored" is
probably easier to understand;
* The description and the example makes it sound as if the command line
has to have these three tokens in the given order, but the whole point
of this mechanism is that you can launch whatever external command with
a custom command line, so "and `mergetool.custom.cmd` to a command line
to invoke the command. $LOCAL $MERGED and $REMOTE on this command line
are substituted by ...." may be more reader-friendly.
* As a documentation update it is perfectly fine to describe this glitch,
but I wonder if we might want to lift this restriction (read: consider
this as a bug and fix it).
> ++
> +-------------
> +[merge]
> + tool = diffuse-custom
> +[mergetool "diffuse-custom"]
> + cmd = diffuse $LOCAL $MERGED $REMOTE
> +-------------
> ++
Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] documentation: git-mergetool updated to warn against builtin tools invocations
2010-06-02 23:38 ` Junio C Hamano
@ 2010-06-03 7:45 ` Michael J Gruber
2010-06-03 10:55 ` Sylvain Rabot
1 sibling, 0 replies; 6+ messages in thread
From: Michael J Gruber @ 2010-06-03 7:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Sylvain Rabot, git, sylvain, David Aguilar
Junio C Hamano venit, vidit, dixit 03.06.2010 01:38:
> Sylvain Rabot <srabot@steek.com> writes:
>
>> +Be aware that 'git mergetool' has some predefined invocation commands builtin
>> +for known diff tools like meld, diffuse, p4merge ... etc. It means that if
>> +you set the merge.tool configuration to one of these tools, the
>> +mergetool.<tool>.* configurations will not be taken care of. If you
>> +really want to customize the invocation of one of these tools,
>> +set `merge.tool` to "custom" or whatever you want and `mergetool.custom.cmd`
>> +to "/usr/bin/<tool> $LOCAL $MERGED $REMOTE"
>
> Two half-points and three points (that makes them four in total ;-):
>
> o If I read the above without "It means that", it still makes sense;
>
> o "If you really" can go without "really";
>
> * I had to read "will not be taken care of" twice; "are ignored" is
> probably easier to understand;
>
> * The description and the example makes it sound as if the command line
> has to have these three tokens in the given order, but the whole point
> of this mechanism is that you can launch whatever external command with
> a custom command line, so "and `mergetool.custom.cmd` to a command line
> to invoke the command. $LOCAL $MERGED and $REMOTE on this command line
> are substituted by ...." may be more reader-friendly.
>
> * As a documentation update it is perfectly fine to describe this glitch,
> but I wonder if we might want to lift this restriction (read: consider
> this as a bug and fix it).
Definitely. The current behaviour even means that when you have set up
fancytool, and then a new git-mergetool version suddenly starts to
support fancytool, possibly with different options, your existing
configuration breaks. I.e., even for currently unsupported tools you
have to use the procedure Silvain describes in order to protect against
updates!
Michael
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] documentation: git-mergetool updated to warn against builtin tools invocations
2010-06-02 23:38 ` Junio C Hamano
2010-06-03 7:45 ` Michael J Gruber
@ 2010-06-03 10:55 ` Sylvain Rabot
2010-06-04 21:29 ` Sylvain Rabot
1 sibling, 1 reply; 6+ messages in thread
From: Sylvain Rabot @ 2010-06-03 10:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Sylvain Rabot, git, David Aguilar
On Thu, Jun 3, 2010 at 01:38, Junio C Hamano <gitster@pobox.com> wrote:
> Sylvain Rabot <srabot@steek.com> writes:
>
>> +Be aware that 'git mergetool' has some predefined invocation commands builtin
>> +for known diff tools like meld, diffuse, p4merge ... etc. It means that if
>> +you set the merge.tool configuration to one of these tools, the
>> +mergetool.<tool>.* configurations will not be taken care of. If you
>> +really want to customize the invocation of one of these tools,
>> +set `merge.tool` to "custom" or whatever you want and `mergetool.custom.cmd`
>> +to "/usr/bin/<tool> $LOCAL $MERGED $REMOTE"
>
> Two half-points and three points (that makes them four in total ;-):
>
> o If I read the above without "It means that", it still makes sense;
Ok
>
> o "If you really" can go without "really";
Ok
>
> * I had to read "will not be taken care of" twice; "are ignored" is
> probably easier to understand;
Ok
>
> * The description and the example makes it sound as if the command line
> has to have these three tokens in the given order, but the whole point
> of this mechanism is that you can launch whatever external command with
> a custom command line, so "and `mergetool.custom.cmd` to a command line
> to invoke the command. $LOCAL $MERGED and $REMOTE on this command line
> are substituted by ...." may be more reader-friendly.
Ok
>
> * As a documentation update it is perfectly fine to describe this glitch,
> but I wonder if we might want to lift this restriction (read: consider
> this as a bug and fix it).
I agree, I wil take a look, I'm not a sh expert but I can try.
If I succeed I will send you a patch, otherwise I will send you a
corrected version of this patch.
>
>> ++
>> +-------------
>> +[merge]
>> + tool = diffuse-custom
>> +[mergetool "diffuse-custom"]
>> + cmd = diffuse $LOCAL $MERGED $REMOTE
>> +-------------
>> ++
>
> Thanks.
>
--
Sylvain
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] documentation: git-mergetool updated to warn against builtin tools invocations
2010-06-03 10:55 ` Sylvain Rabot
@ 2010-06-04 21:29 ` Sylvain Rabot
2010-06-07 12:50 ` Ramkumar Ramachandra
0 siblings, 1 reply; 6+ messages in thread
From: Sylvain Rabot @ 2010-06-04 21:29 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 2274 bytes --]
On Thu, 2010-06-03 at 12:55 +0200, Sylvain Rabot wrote:
> On Thu, Jun 3, 2010 at 01:38, Junio C Hamano <gitster@pobox.com> wrote:
> > Sylvain Rabot <srabot@steek.com> writes:
> >
> >> +Be aware that 'git mergetool' has some predefined invocation commands builtin
> >> +for known diff tools like meld, diffuse, p4merge ... etc. It means that if
> >> +you set the merge.tool configuration to one of these tools, the
> >> +mergetool.<tool>.* configurations will not be taken care of. If you
> >> +really want to customize the invocation of one of these tools,
> >> +set `merge.tool` to "custom" or whatever you want and `mergetool.custom.cmd`
> >> +to "/usr/bin/<tool> $LOCAL $MERGED $REMOTE"
> >
> > Two half-points and three points (that makes them four in total ;-):
> >
> > o If I read the above without "It means that", it still makes sense;
>
> Ok
>
> >
> > o "If you really" can go without "really";
>
> Ok
>
> >
> > * I had to read "will not be taken care of" twice; "are ignored" is
> > probably easier to understand;
>
> Ok
>
> >
> > * The description and the example makes it sound as if the command line
> > has to have these three tokens in the given order, but the whole point
> > of this mechanism is that you can launch whatever external command with
> > a custom command line, so "and `mergetool.custom.cmd` to a command line
> > to invoke the command. $LOCAL $MERGED and $REMOTE on this command line
> > are substituted by ...." may be more reader-friendly.
>
> Ok
>
> >
> > * As a documentation update it is perfectly fine to describe this glitch,
> > but I wonder if we might want to lift this restriction (read: consider
> > this as a bug and fix it).
>
> I agree, I wil take a look, I'm not a sh expert but I can try.
> If I succeed I will send you a patch, otherwise I will send you a
> corrected version of this patch.
I'm about to send a patch, before, can I assume that all users have
sed ?
>
> >
> >> ++
> >> +-------------
> >> +[merge]
> >> + tool = diffuse-custom
> >> +[mergetool "diffuse-custom"]
> >> + cmd = diffuse $LOCAL $MERGED $REMOTE
> >> +-------------
> >> ++
> >
> > Thanks.
> >
>
--
Sylvain Rabot <sylvain@abstraction.fr>
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] documentation: git-mergetool updated to warn against builtin tools invocations
2010-06-04 21:29 ` Sylvain Rabot
@ 2010-06-07 12:50 ` Ramkumar Ramachandra
0 siblings, 0 replies; 6+ messages in thread
From: Ramkumar Ramachandra @ 2010-06-07 12:50 UTC (permalink / raw)
To: sylvain; +Cc: git
Hi,
Sylvain Rabot wrote:
> I'm about to send a patch, before, can I assume that all users have
> sed ?
Yes. $(git grep sed *.sh) revels that sed is referred to in several
existing shell scripts already.
-- Ram
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-06-07 12:51 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-06-02 17:57 [PATCH] documentation: git-mergetool updated to warn against builtin tools invocations Sylvain Rabot
2010-06-02 23:38 ` Junio C Hamano
2010-06-03 7:45 ` Michael J Gruber
2010-06-03 10:55 ` Sylvain Rabot
2010-06-04 21:29 ` Sylvain Rabot
2010-06-07 12:50 ` Ramkumar Ramachandra
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).