On Tue, Dec 15, 2009 at 05:05:52PM -0800, Junio C Hamano wrote: > > 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. Yeah, I think textconv is similarly hard to find. We should probably have a pointer in "git-diff.txt" to the attributes documentation. I also think it would be much more obvious as "diff.*.external", but it is probably not worth changing at this point. > > .... 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. It does cover all three. We could do them separately, I guess, but I think that is just making things confusingly more inconsistent. > 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. Yeah, the only upside I can see is consistency. Which I do value, but it will be a hard sell to somebody whose setup has been broken. ;P > 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)? Will do, but it will probably be a few days as I'm about to do some holiday traveling. I'll do the textconv change with it as a series. -Peff -- 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