Re: [patch] selected/context colors

[Charles Levert]

* On Monday 2005-11-14 at 21:19:47 +0000, Julian Foad wrote:
> Charles Levert wrote:
> >(Make sure to try 2.5.1a with different combinations
> >of options, to see what its behavior actually was.)

Also make sure to try all combinations with some
output prefix, e.g., with -H.

> Testing v2.5.1 now, I find:
> 
> * The combination "-v -C" indeed highlighted matches within context lines.

Yes.


> * The combination "-o -v" didn't produce any output.

Indeed, by definition, there would never be any.


> and likewise "-o -C",

"-o -C" definitely produced output for
selected/matching lines, as it should.
It had a bug where it also produced
spurious prefixes with no newline for
context/non-matching lines.

> because the output from that 
> appears to be totally spurious (undesigned, accidental).

Not totally.
The selected/matching lines were good.

> Therefore I think 
> it is our job to forbid or define the meaning of these combinations.

And I think we never disagreed on those meanings,
for "-o -v" and "-o -C".


> Are there any other combinations I should try?

Yes:  "-o -v -C" (the one about which we disagreed so).

In 2.5.1a:

   -- For context/matching lines, it produced:

         prefix-matching_part\n

      If I understand your position so far
      correctly, this is the most prominent thing
      that you stand against.  I wanted to point
      out that it's what 2.5.1a did, but this is
      no proof that your position is untenable
      in the absolute, just that it's the one
      that's a departure from the stable release.

   -- For selected/non-matching lines, it produced:

         prefix:

      with no content or "\n".  What I argued
      so far is equivalent to stating that any
      "prefix:" (or "prefix-" for "-o -C" above)
      should be omitted completely when there
      is no content to report (based on whatever
      policy we choose for that content-printing
      decision).

Also try to notice where "--" line separator
were printed under all these option combinations.


> >>(A detail: the old "GREP_COLOR" option should override 
> >
> >The old GREP_COLOR does _not_ override
> >GREP_COLORS, to be explicit.  It can change
> >the default, but is itself overriden by the new
> >GREP_COLORS, which has priority.
> 
> Sorry - I knew that, but wrote it badly.  I meant "should affect", not 
> "should override".

Ok.

> >>"matched-text-in-selected-line" only, since no highlighting used to be 
> >>done on context lines.)
> 
> And I was wrong there.
> 
> >But I see what you mean.  Check it out for
> >yourself, the last official release did color
> >matched text, whether in selected lines or in
> >context lines.  To be compatible in behavior
> >in 2.5.1a, GREP_COLOR should provisionally
> >set _both_ matched-text-in-selected-line and
> >matched-text-in-context-line, before GREP_COLORS
> >gets a chance to set them individually.
> 
> Yes, that would be sensible.

And will be in my next proposed patch.
It's written; I was about to test it when I
started doing this reply.


> >>Secondly, the log entry indicates that this patch also changes the 
> >>behaviour of "-o".  We're discussing that in another thread and it 
> >>shouldn't be done until we've decided.
> >
> >This is a bug I introduced and that wasn't
> >there before.  It's about not processing things
> >in prline() when it's asked to.
> 
> Er... I'm not sure what bug this refers to and when "before" refers to, so 
> no comment yet.

Before is 2.5.1a, notably.
Before the prline() rework.


> Without a written spec. for the function, it is meaningless
[...]

Yes, that's been the case for many of the GNU
extensions to grep, wherever they reach in the
implementation.  We inherited the situation,
but it's what we're trying to remedy, including
in/by having this discussion.


[...]
> behaviours that were very broken.

I separate the spurious prefixes brokenness,
which is fixed, from other things which,
if they are broken, are so more in a
fundamental/conceptual but debatable way.