On Wed, Jul 19, 2006 at 07:24:40PM +0100, Dave Ewart wrote: > > +exec "/usr/bin/diff",@ARGV unless -t STDOUT; > > It's not as simple as that, though: the above will fail if colordiff (or > 'diff' if aliased to colordiff) is used in a pipe. In other words, > this is an incomplete fix.
Uhm, isn't the whole point of the fix to correct just this issue? If the output isn't redirected, -t will be true and colordiff will continue. > In detail... > > i.e. (assuming 'alias diff=colordiff') > > diff file1 file2 - this will work and colour-highlight the output > diff file1 file2 > my.patch - this will *not* colour-highlight, as intended > > but > > someprog | diff > my.patch - this will give an error > > /usr/bin/diff: missing operand after `/usr/bin/diff' > /usr/bin/diff: Try `/usr/bin/diff --help' for more information. So it's working correctly: /usr/bin/diff /usr/bin/diff: missing operand after `/usr/bin/diff' /usr/bin/diff: Try `/usr/bin/diff --help' for more information. You need to always specify at least two arguments to diff, one of which can be "-". With or without colordiff. > Given this, and the fact that colordiff's entire raison d'ĂȘtre is to > colour highlight, my feeling is that a better option is this: rather than try > to make colordiff *avoid* highlighting in certain circumstances, this > limitation is explained in the man page. In this case, you can't recommend aliasing 'diff' to 'colordiff' then. > A documentation change might actually be a more thorough solution: it > will make the user know "what they're letting themselves in for" by > aliasing 'diff' to 'colordiff'. I would say there are two choices: 1) allow aliasing and then handle the case of !isatty(1) 2) tell people they _can't_ use the alias. Leaving this as documentation only would work, too -- but if someone makes the alias anyway, they will lose the moment they try to make a patch. Regards, -- 1KB // Microsoft corollary to Hanlon's razor: // Never attribute to stupidity what can be // adequately explained by malice. -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]