Request for review: replacing cdiff with colordiff

[Ville Skyttä]

On Fri, 2005-05-20 at 08:37 -0400, Matthew Miller wrote:
> On Fri, May 20, 2005 at 11:37:01AM +0200, Matthias Saou wrote:
> > - It says it's a wrapper to diff, but you don't require diffutils, is that
> >   normal?
> 
> It's not really a wrapper exactly -- it's just filter that adds colors. You
> could use it on patch files directly. This might be somewhat confusing,
> though -- it might not *hurt* to add this dependency. 

Ok, added.

> > - The obsoletes should contain the last known version or version-release of
> >   cdiff, especially since you provide cdiff.
> 
> Oh, I see that Ville has actually added a cdiff wrapper that acts exactly
> like cdiff, thus removing my earlier complaint.

Yes, that was mentioned in my initial message to this thread :)

> So in that case, it looks pretty good. I'm not so sure about the
> "plain=black" in the config file -- looks pretty ugly in my grey-on-black
> gnome terminal.

The most important thing about the config file is that default settings
in it doesn't make things _invisible_ with the most "usual" color
schemes.  (Non-)ugliness is secondary, and because the default color
scheme is black-on-white in Fedora, the default is geared towards such
setups.  See also the samples in the package's doc dir (there's one for
dark backgrounds), and note that one can configure the colors exactly
like one pleases in ~/.colordiffrc

But if you come up with a better compromise than plain=black for the
default config, I'm open to suggestions...

> Also, like the original cdiff, it doesn't handle wrapped lines properly in
> 'less -R' -- this might be a less bug, I'm not sure. Or maybe even a
> gnome-terminal one. Anyway, I suggest making that less -RS.

I think chopping lines with -S would be very much unexpected behaviour.
Although colorization doesn't always work for wrapped lines, it
_usually_ does just fine here (using Konsole, FWIW).  Chopping the lines
would avoid the colorization issue, but it would certainly _always_ hide
parts of the information when long lines are present and cause the need
to hit the right arrow which will certainly screw up the colorization.

Revised package with the versioned obsoletes and added diffutils dep:
http://cachalot.mine.nu/3/SRPMS.extras/colordiff-1.0.5-0.3.src.rpm
Good enough for inclusion now?