Jeff King <peff@xxxxxxxx> writes: >> - 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)? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html