* Feature request: be able to pass arguments to difftool command
@ 2018-08-28 15:57 H.Merijn Brand
2018-08-28 19:37 ` Junio C Hamano
0 siblings, 1 reply; 5+ messages in thread
From: H.Merijn Brand @ 2018-08-28 15:57 UTC (permalink / raw)
To: git
[-- Attachment #1.1: Type: text/plain, Size: 6956 bytes --]
In short, I want to be able to do
$ git difftool 5c5a -v2
where -v2 is passed to the tool invoked in the end. I'll elaborate on
why ...
Background.
Some changes are not obvious to spot. One of the best examples I found
is this one:
https://github.com/hexchat/hexchat/commit/5c5aacd9da7d45dfc1644b87de9e2379a1102d6a
$ git show 5c5a
commit 5c5aacd9da7d45dfc1644b87de9e2379a1102d6a
Author: Patrick Griffis <tingping@tingping.se>
Date: Wed Apr 4 19:21:53 2018 -0400
Fix another bad translation
diff --git a/po/el.po b/po/el.po
index 7d47ee1e..e54d62c1 100644
--- a/po/el.po
+++ b/po/el.po
@@ -1072,7 +1072,7 @@ msgstr "%C22*%O$t%C22$1%O: %C18$2%O κατά συνέχεια %C24$4%O από %C
#: src/common/textevents.h:13
msgid "%C22*%O$tCannot join %C22$1 %O(%C20You are banned%O)."
-msgstr "%C22*%O$tΑδυναμία εισαγωγής %C22$1 %O(%C20Είστε μπλοκαρισμένος%Ο)."
+msgstr "%C22*%O$tΑδυναμία εισαγωγής %C22$1 %O(%C20Είστε μπλοκαρισμένος%O)."
#: src/common/textevents.h:19
msgid "%C29*%O$tCapabilities acknowledged: %C29$2%O"
I bet none of you is able to spot the change in that line, even if
colored. I had a similar problem completely unrelated to git, and I
wrote the "ccdiff" tool, to do horizontally colored diff. As mail (when
used without HTML, as it ought to be used) is not apt to coloring, I
added an option to show markers under the changed text. I then
integrated the tool as much as possible into git
$ cpan App::ccdiff
$ git config --global diff.tool ccdiff
$ git config --global difftool.prompt false
$ git config --global difftool.ccdiff.cmd 'ccdiff --utf-8 -u $LOCAL $REMOTE'
$ git difftool 5c5a~..5c5a
--- /tmp/CXbTje_el.po Tue Aug 28 17:38:02 2018
+++ /tmp/8jrR7d_el.po Tue Aug 28 17:38:02 2018
1075,1075c1075,1075
#: src/common/textevents.h:13
msgid "%C22*%O$tCannot join %C22$1 %O(%C20You are banned%O)."
-msgstr "%C22*%O$tΑδυναμία εισαγωγής %C22$1 %O(%C20Είστε μπλοκαρισμένος%Ο)."
+msgstr "%C22*%O$tΑδυναμία εισαγωγής %C22$1 %O(%C20Είστε μπλοκαρισμένος%O)."
#: src/common/textevents.h:19
msgid "%C29*%O$tCapabilities acknowledged: %C29$2%O"
$ cat >~/bin/git-ccdiff <<EOC
#!/bin/sh
commit=$1
shift
if [ "$commit" = "" ]; then
git difftool
else
git difftool $commit~1..$commit
fi
EOF
$ chmod 755 ~/bin/git-ccdiff
$ git ccdiff 5c5a
--- /tmp/Cw7VDo_el.po Tue Aug 28 17:41:08 2018
+++ /tmp/yuTGro_el.po Tue Aug 28 17:41:08 2018
1075,1075c1075,1075
#: src/common/textevents.h:13
msgid "%C22*%O$tCannot join %C22$1 %O(%C20You are banned%O)."
-msgstr "%C22*%O$tΑδυναμία εισαγωγής %C22$1 %O(%C20Είστε μπλοκαρισμένος%Ο)."
+msgstr "%C22*%O$tΑδυναμία εισαγωγής %C22$1 %O(%C20Είστε μπλοκαρισμένος%O)."
#: src/common/textevents.h:19
msgid "%C29*%O$tCapabilities acknowledged: %C29$2%O"
So, with all parts in place, I want more info
$ git ccdiff 5c5a -m
No change. When I add -m to the cmd in ~/.gitconfig, like
[difftool "ccdiff"]
cmd = ccdiff --utf-8 -u -m $LOCAL $REMOTE
$ git ccdiff 5c5a
--- /tmp/fXkf4E_el.po Tue Aug 28 17:44:12 2018
+++ /tmp/P3mZ1E_el.po Tue Aug 28 17:44:12 2018
1075,1075c1075,1075
#: src/common/textevents.h:13
msgid "%C22*%O$tCannot join %C22$1 %O(%C20You are banned%O)."
-msgstr "%C22*%O$tΑδυναμία εισαγωγής %C22$1 %O(%C20Είστε μπλοκαρισμένος%Ο)."
- ▼
+msgstr "%C22*%O$tΑδυναμία εισαγωγής %C22$1 %O(%C20Είστε μπλοκαρισμένος%O)."
+ ▲
#: src/common/textevents.h:19
msgid "%C29*%O$tCapabilities acknowledged: %C29$2%O"
Nice, but I still cannot pass it from the command line, and in this
specific case I also want -v2 or -v3
cmd = ccdiff --utf-8 -u -m -v2 $LOCAL $REMOTE:
--- /tmp/dp0E5T_el.po Tue Aug 28 17:45:51 2018
+++ /tmp/za7m3T_el.po Tue Aug 28 17:45:51 2018
1075,1075c1075,1075
#: src/common/textevents.h:13
msgid "%C22*%O$tCannot join %C22$1 %O(%C20You are banned%O)."
-msgstr "%C22*%O$tΑδυναμία εισαγωγής %C22$1 %O(%C20Είστε μπλοκαρισμένος%↱Ο↰)."
- ▼
- -- verbose : GREEK CAPITAL LETTER OMICRON
+msgstr "%C22*%O$tΑδυναμία εισαγωγής %C22$1 %O(%C20Είστε μπλοκαρισμένος%↱O↰)."
+ ▲
+ -- verbose : LATIN CAPITAL LETTER O
#: src/common/textevents.h:19
msgid "%C29*%O$tCapabilities acknowledged: %C29$2%O"
cmd = ccdiff --utf-8 -u -m -v3 $LOCAL $REMOTE
(a screenshot is added to show the colors)
--- /tmp/4gPKTd_el.po Tue Aug 28 17:47:57 2018
+++ /tmp/IreaTe_el.po Tue Aug 28 17:47:57 2018
1075,1075c1075,1075
#: src/common/textevents.h:13
msgid "%C22*%O$tCannot join %C22$1 %O(%C20You are banned%O)."
-msgstr "%C22*%O$tΑδυναμία εισαγωγής %C22$1 %O(%C20Είστε μπλοκαρισμένος%↱Ο↰)."
- ▼
- -- verbose : GREEK CAPITAL LETTER OMICRON (U+00039F)
+msgstr "%C22*%O$tΑδυναμία εισαγωγής %C22$1 %O(%C20Είστε μπλοκαρισμένος%↱O↰)."
+ ▲
+ -- verbose : LATIN CAPITAL LETTER O (U+00004F)
#: src/common/textevents.h:19
msgid "%C29*%O$tCapabilities acknowledged: %C29$2%O"
The only workaround I could come up with (for now) is both ugly and
fragile: I keep the extra arguments in an environment variable and add
those back to the commandline after the command started. Very easy to
break though.
$ cat ~/bin/git-ccdiff
#!/pro/bin/perl
use 5.18.3;
use warnings;
my $commit;
if (@ARGV) {
$ARGV[0] =~ m/^-/ or $commit = shift;
}
@ARGV and $ENV{CCDIFF_OPTIONS} = "@ARGV";
my @git = qw( git difftool );
defined $commit and push @git, "$commit~1..$commit";
system @git;
So, my wish would be to have an option, possibly using -- to pass
additional command line arguments to git difftool, so that
$ git difftool $commit~1..$commit -- -m -v2
would pass the arguments after -- transparantly to ccdiff (in my case)
--
H.Merijn Brand http://tux.nl Perl Monger http://amsterdam.pm.org/
using perl5.00307 .. 5.29 porting perl5 on HP-UX, AIX, and openSUSE
http://mirrors.develooper.com/hpux/ http://www.test-smoke.org/
http://qa.perl.org http://www.goldmark.org/jeff/stupid-disclaimers/
[-- Attachment #1.2: 20180828174959.png --]
[-- Type: image/png, Size: 54385 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: Feature request: be able to pass arguments to difftool command
2018-08-28 15:57 Feature request: be able to pass arguments to difftool command H.Merijn Brand
@ 2018-08-28 19:37 ` Junio C Hamano
2018-08-29 7:18 ` H.Merijn Brand
0 siblings, 1 reply; 5+ messages in thread
From: Junio C Hamano @ 2018-08-28 19:37 UTC (permalink / raw)
To: H.Merijn Brand; +Cc: git
"H.Merijn Brand" <h.m.brand@xs4all.nl> writes:
> So, my wish would be to have an option, possibly using -- to pass
> additional command line arguments to git difftool, so that
>
> $ git difftool $commit~1..$commit -- -m -v2
>
> would pass the arguments after -- transparantly to ccdiff (in my case)
At the syntax level passing any option after "--" would be a no
starter, as I would imagine that "git difftool $revs -- $paths"
should still be supported.
At the concept level, however, I can see why such a feature would be
useful. Perhaps
$ git difftool --backend-option=-m --backend-option=-v2 HEAD
$ git mergetool --backend-option=--foo
with appropriate way(s) [*1*] to make it easier to type (and
implement) would be an acceptable avenue to pursue, I wonder?
[Footnote]
*1* There are various possible ways, not all of them are mutually
incompatible.
a. Give a short-form synonym, e.g. -X, to "--backend-option";
b. Assume that backend option always begins with a dash and add
one when missing, e.g. -Xm becomes --backend-option=-m
c. Allow giving multiple backend options on a single option and
split at whitespace, e.g. --backend-option="-m -v2"
d. Allow difftool.$toolname.opts configuration variable that is
multi-valued, so you can say
git -c difftool.ccdiff.opts=-v2 -c difftool.ccdiff.opts=-m difftool
(of course, not necessarily from the command line but the
point is you could configure it)
Some of these (e.g. b, c) may not be desirable, though.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Feature request: be able to pass arguments to difftool command
2018-08-28 19:37 ` Junio C Hamano
@ 2018-08-29 7:18 ` H.Merijn Brand
2018-09-16 6:28 ` David Aguilar
0 siblings, 1 reply; 5+ messages in thread
From: H.Merijn Brand @ 2018-08-29 7:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
[-- Attachment #1: Type: text/plain, Size: 3342 bytes --]
On Tue, 28 Aug 2018 12:37:40 -0700, Junio C Hamano <gitster@pobox.com>
wrote:
> "H.Merijn Brand" <h.m.brand@xs4all.nl> writes:
>
> > So, my wish would be to have an option, possibly using -- to pass
> > additional command line arguments to git difftool, so that
> >
> > $ git difftool $commit~1..$commit -- -m -v2
> >
> > would pass the arguments after -- transparantly to ccdiff (in my case)
>
> At the syntax level passing any option after "--" would be a no
> starter, as I would imagine that "git difftool $revs -- $paths"
> should still be supported.
>
> At the concept level, however, I can see why such a feature would be
> useful. Perhaps
>
> $ git difftool --backend-option=-m --backend-option=-v2 HEAD
> $ git mergetool --backend-option=--foo
This would mean I can just pass remaining arguments, like this?
--8<--- ~/bin/git-ccdiff
#!/usr/bin/env perl
use 5.18.3;
use warnings;
my $commit;
@ARGV && $ARGV[0] !~ m/^-/ and $commit = shift;
my @git = qw( git difftool );
defined $commit and push @git, "$commit~1..$commit";
system @git, @ARGV;
-->8---
> with appropriate way(s) [*1*] to make it easier to type (and
> implement) would be an acceptable avenue to pursue, I wonder?
I like it, as long as they are all separate options in the backend and
not available in one single variable that needs to be split
I can envision a configure variable like
backends.options.separator = U+2063
so the backend can safely split on that itself. But I also see this as
overly complex en over-engineering
> [Footnote]
>
> *1* There are various possible ways, not all of them are mutually
> incompatible.
>
> a. Give a short-form synonym, e.g. -X, to "--backend-option";
I like it
> b. Assume that backend option always begins with a dash and add
> one when missing, e.g. -Xm becomes --backend-option=-m
I guess not: there might be tools that do not work like that, e.g.
xfreerdp changed all their rememberable and logic options to the weird
stupid syntax they use now, including mixing -, -- and +
rdesktop -u user -p - -g 1280x1024 -a 16 -r clipboard:CLIPBOARD host
->
xfreerdp -u user --from-stdin -g 1280x1024 -a 16 --plugin clipbrd host
->
xfreerdp /u:user /from-stdin /size:1280x1024 /bpp:16 +clipboard /v:host
> c. Allow giving multiple backend options on a single option and
> split at whitespace, e.g. --backend-option="-m -v2"
That is the weak part in my workaround, as it will break on options like
--backend-option='--config="/path/to/My Configuration/My Application"'
> d. Allow difftool.$toolname.opts configuration variable that is
> multi-valued, so you can say
>
> git -c difftool.ccdiff.opts=-v2 -c difftool.ccdiff.opts=-m difftool
Hmm, maybe harder to explain, but why not
> (of course, not necessarily from the command line but the
> point is you could configure it)
>
> Some of these (e.g. b, c) may not be desirable, though.
--
H.Merijn Brand http://tux.nl Perl Monger http://amsterdam.pm.org/
using perl5.00307 .. 5.29 porting perl5 on HP-UX, AIX, and openSUSE
http://mirrors.develooper.com/hpux/ http://www.test-smoke.org/
http://qa.perl.org http://www.goldmark.org/jeff/stupid-disclaimers/
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Feature request: be able to pass arguments to difftool command
2018-08-29 7:18 ` H.Merijn Brand
@ 2018-09-16 6:28 ` David Aguilar
2018-09-17 16:44 ` Junio C Hamano
0 siblings, 1 reply; 5+ messages in thread
From: David Aguilar @ 2018-09-16 6:28 UTC (permalink / raw)
To: H.Merijn Brand; +Cc: Junio C Hamano, git
On Wed, Aug 29, 2018 at 09:18:38AM +0200, H.Merijn Brand wrote:
> On Tue, 28 Aug 2018 12:37:40 -0700, Junio C Hamano <gitster@pobox.com>
> wrote:
>
> > "H.Merijn Brand" <h.m.brand@xs4all.nl> writes:
> >
> > > So, my wish would be to have an option, possibly using -- to pass
> > > additional command line arguments to git difftool, so that
> > >
> > > $ git difftool $commit~1..$commit -- -m -v2
> > >
> > > would pass the arguments after -- transparantly to ccdiff (in my case)
> >
> > At the syntax level passing any option after "--" would be a no
> > starter, as I would imagine that "git difftool $revs -- $paths"
> > should still be supported.
> >
> > At the concept level, however, I can see why such a feature would be
> > useful. Perhaps
> >
> > $ git difftool --backend-option=-m --backend-option=-v2 HEAD
> > $ git mergetool --backend-option=--foo
>
> This would mean I can just pass remaining arguments, like this?
>
> --8<--- ~/bin/git-ccdiff
> #!/usr/bin/env perl
> use 5.18.3;
> use warnings;
>
> my $commit;
>
> @ARGV && $ARGV[0] !~ m/^-/ and $commit = shift;
>
> my @git = qw( git difftool );
> defined $commit and push @git, "$commit~1..$commit";
> system @git, @ARGV;
> -->8---
>
> > with appropriate way(s) [*1*] to make it easier to type (and
> > implement) would be an acceptable avenue to pursue, I wonder?
>
> I like it, as long as they are all separate options in the backend and
> not available in one single variable that needs to be split
>
> I can envision a configure variable like
>
> backends.options.separator = U+2063
>
> so the backend can safely split on that itself. But I also see this as
> overly complex en over-engineering
Personally, I think it'd be better to keep the tool simple.
While I do see the utility, it would be just as easy to configure a 2nd
and 3rd variant of the same difftool and use those as needed instead.
"git difftool -t ccdiff2" or "-t ccdiff3" is the simplest, and there's
nothing stopping the user from creating aliases to shorten it further.
We also already have, "git difftool -x / --extcmd"
for specifying a full-on external diff command.
> git -c difftool.ccdiff.opts=-v2 -c difftool.ccdiff.opts=-m difftool
For example, this seems simpler as:
git difftool -x 'ccdiff -v2 -m'
We already have two mechanisms for controlling the inner command that's
launched by difftool. IMO we don't need more.
My primary concerns with --backend-opts are as follows:
1. If we add a mechansim for passing -X/--backend-opts, then we
need to specify a new variable that users will need to be aware
of when creating custom commands. (sorry for stating the obvious)
2. All of the built-in commands would need to change to honor that
variable.
3. The documentation becomes more complex because someone that wants
to configure a bog-standard custom external tool now needs to
be aware of this extra external source of arguments.
#1 and #2 are primarily implementation concerns, but #3 suggests
to me that it's over-complicating things.
Furthermore, #2 is not really that simple.
What would the sciplet look like?
diff_cmd () {
"$merge_tool_path" $EXTRA_ARGS ...
}
That implies that we would need to shell quote stuff when
constructing $EXTRA_ARGS internally if we were to support multiple -X
arguments. That just made it a bit more complex.
IMO we should be working to simpliify, not make things more complex for
rare use cases. There's no reason the user can't just do:
V=2 git difftool
V=3 git difftool
... and let the inner script check for $V (or any other) variable.
While environment variables aren't great, this does seem like the right
place to use them.
Another option -- we already eval the configured command, so if the user
includes a variable ($ARGS) in their custom configuration then they can
specify extra flags today without needing to change the tool. ex:
[difftool "ccdiff"]
cmd = ccdiff $ARGS \"$LOCAL\" \"$REMOTE\"
ARGS='-v2 -m' git difftool HEAD~1..HEAD
Are these alternatives short and simple enough?
--
David
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Feature request: be able to pass arguments to difftool command
2018-09-16 6:28 ` David Aguilar
@ 2018-09-17 16:44 ` Junio C Hamano
0 siblings, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2018-09-17 16:44 UTC (permalink / raw)
To: David Aguilar; +Cc: H.Merijn Brand, git
David Aguilar <davvid@gmail.com> writes:
> While I do see the utility, it would be just as easy to configure a 2nd
> and 3rd variant of the same difftool and use those as needed instead.
>
> "git difftool -t ccdiff2" or "-t ccdiff3" is the simplest, and there's
> nothing stopping the user from creating aliases to shorten it further.
> ...
> We already have two mechanisms for controlling the inner command that's
> launched by difftool. IMO we don't need more.
OK, fair enough.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-09-17 16:44 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-28 15:57 Feature request: be able to pass arguments to difftool command H.Merijn Brand
2018-08-28 19:37 ` Junio C Hamano
2018-08-29 7:18 ` H.Merijn Brand
2018-09-16 6:28 ` David Aguilar
2018-09-17 16:44 ` Junio C Hamano
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).