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]
