Re: Bug #11022: Line wrapping causes GREP_COLOR background color to "smear"

[Julian Foad]

Charles Levert wrote:
> * On Tuesday 2005-06-14 at 00:44:56 +0100, Julian Foad wrote:
> > It would really help me and others to review it if you would provide a log
> > message that describes what the purpose of the patch is, and what changes 
> > it makes.
>
> For the patches I wrote, keep in mind that I can
> do both at once myself: apply them and write a
> complete ChangeLog entry.

But if you don't write the description of the patch until you commit it, then I 
won't be able to review your patch properly until after you have committed it, 
which would mean CVS would get cluttered up with lots of re-fixing and 
reverting and minor fixes and so on.  What I am asking is that everybody who 
provides a patch should provide a description of it at the same time, so that 
other people can understand why the patch exists and what it is intended to do. 
 Only with this information can people properly evaluate the patch.

> > You could condense that a bit for ChangeLog, but don't leave out the "why" 
> > parts.
>
> Do we really need to have both a ChangeLog
> entry and a long CVS commit message?

No, we don't.  I didn't mean to imply that we should duplicate the text.  I 
meant "Here's a description in the form that I'm used to; it could be a little 
bit briefer because ChangeLog entries are traditionally (very) brief."

I have just been looking through ChangeLog to find out what changes were made. 
 Most of the entries say just what source code was changed, and that is not 
enough information to be useful.  "cvs diff" can tell me what source code was 
changed; I want to know why it was changed, and what effect that change had on 
the overall operation of the software.

Therefore I want us to record more information about each change from now on.


Thanks for the info about handling tabs.

> > Should we also clear to EOL after 
> > changing colour at the start of a match, for symmetry?

You wrote in a subsequent message:
> I've thought about this one suggestion
> some more, and it makes sense actually.
> A new question would then be:  should it be
> eventually be possible to disable it using the
> same GREP_COLORS capability as for SGR_END,
> or should it have its own capability for that?

That's a question for a subsequent patch.  For now, for this patch, it appears 
that you agree that both the start and end sequences should clear to EOL.


Can you update the patch and post it here?  In fact, if you could split the 
patch even further, to factorise the printing into macros first, that would be 
even better.  The factorising is uncontroversial and can be committed straight 
away.  That would make the patch that fixes "smearing" very small and simple, 
which is a good thing.  (But if you want to keep the factorisation and the bug 
fix together in a single patch this time because it saves you some work, you 
can do.)

- Julian