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

[Julian Foad]

Charles Levert wrote:
> There are also bugs such as
>
>    <https://savannah.gnu.org/bugs/?func=detailitem&item_id=11022>
>
> that could be closed right now as well because
> a complete solution is available to them.

And, in that patch issue:
> The framework introduced by the preprocessor macros is a good thing to have now 
> as moves the actual SGR strings in one place and it will be re-used many times 
> by a newer patch #3644.
>
> This includes a corresponding update to tests/foad1.sh as well.
>
> Only the ChangeLog entry is missing and will be added upon commit. 

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.

In this case the patch is simple enough that I can determine the two things
that it does by inspection, and write a log message like so (in the style used
by Apache and Subversion):

[[[
Fix bug #11022: Line wrapping causes GREP_COLOR background color to "smear",
by outputting a "clear to end of line" control sequence after colored output.
Factor out the printing of colour control sequences into macros.

* src/grep.c
  Add new macros for starting and ending colored output.
  Make the "end of colour" string also clear to the end of the line.
  (prline): Use the new macros.

* tests/foad1.sh
  Adjust the regression tests to expect the new codes.
]]]

You could condense that a bit for ChangeLog, but don't leave out the "why" 
parts.

In your patch you say:
> +/* Select Graphic Rendition (SGR) strings.  */
> +#define SGR_ARG "\33[%sm"
> +/* Also Erase in Line (EL) to Right by default.  */
> +#define SGR_END "\33[m\33[K"

I didn't really understand that last comment - especially the "by default".
Could you explain it or tell me if this would be a good replacement:

> /* Select Graphic Rendition (SGR) strings.  */
> /* Start colored output with a given color code. */
> #define SGR_ARG "\33[%sm"
> /* End colored output and erase to end of line.  */
> #define SGR_END "\33[m\33[K"

I'm not totally confident that this clearing to EOL is the right approach to 
the problem.  My concerns run along the lines of "Isn't this the terminal's 
fault, somehow?" and "What if people are depending on Grep to output just 
simple colour control sequences?"  What do you think?  I would be happier if I 
could see a precedent for this behaviour, perhaps in another GNU program.

Actually, I'm not sure I'm convinced by your "tabs" argument.  If, on a 
particular terminal, tabs fail to fill a background that is meant to be noraml 
colour (e.g. black), won't tabs also fail to fill parts of the matched text 
that are meant to be highlighted (e.g. red)?  Maybe the latter is not so ugly, 
but it's still wrong.  Should we also clear to EOL after changing colour at the 
start of a match, for symmetry?

Sorry I've so many questions for such an apparently simple patch.

Thanks.

- Julian