Re: Giving command line parameter to textconv command?

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 16, 2009, 01:05 UTC
Message-ID: <7vljh3wwwf.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20091215164959.GA21322@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 8 quoted lines

>>  - diff.c::run_external_diff() that runs GIT_EXTERNAL_DIFF defines that
>>    the command has to take 7 parameters in a fixed order, and is not
>>    designed to permute its arguments like ll_ext_merge() does, but these
>>    days people don't use it directly (they use it indirectly via
>>    "difftool" wrapper), so it probably is not an issue.
>
> There is also diff.*.command, which I think people _do_ set manually (I
> used to, until I wrote textconv. :) ).

I had to spend fair amount of time to find where "diff.*.command" is described. We may want to update the documentation.

> .... I agree that people almost certainly have to write a shell-script
> wrapper anyway. But I wonder if we should pass it through the shell,
> just for the sake of consistency with the other variables (in
> particular, if textconv changes,

This covers GIT_EXTERNAL_DIFF, diff.external, and diff.<driver>.command trio, if I am not mistaken.

If we changed run_external_diff(), in practice nobody would notice, except for people who have installed the difftool helper in a directory with IFS in the path. That's one downside but I don't offhand see a use case where the change would make somebody vastly happier.

But maybe people will find good uses and we'll never know until we try. Care to roll a patch for that as well, to be queued for 1.7.0 (which will be the one after 1.6.6)?