Re: Feature request: be able to pass arguments to difftool command

From: David Aguilar <davvid@gmail.com>
Date: Sep 16, 2018, 06:28 UTC
Message-ID: <20180916062800.GA18517@gmail.com>
In-Reply-To: <20180829091838.2eee8a6a@pc09.procura.nl>

On Wed, Aug 29, 2018 at 09:18:38AM +0200, H.Merijn Brand wrote:

Show 50 quoted lines

> 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