Re: [patch] selected/context colors

[Charles Levert]

* On Monday 2005-11-14 at 12:58:20 +0000, Julian Foad wrote:
> Charles Levert wrote:
> >Here is the selected/context colors patch, as discussed.
> >Nothing else that was discussed is touched by this.
> >
> >I have attached a screen shot to be sure this
> >is what everybody understands and wants.
> 
> Thanks very much for the good, helpful screen shot.
> 
> >+    * src/grep.c (print_line_middle, prline): Change logic so that
> >+      lines with matches have their matched part properly handled ("mt"
> >+      color or --only-matching), whether -v or not -v.
> 
> Ah - no, I didn't expect to see matches highlighted where they appear in 
> context lines.  I think "grep -C3 -v s" means "show me the lines that don't 
> contain an "s", with a few lines of context around them."  I don't want all 
> the esses in the context lines to be highlighted; that's just a distraction 
> from the parts that I want to see.

Please confirm explicitly, however, that the
part about the main color for the whole line
being inverted in synchronization with the ':'
and '-' separators is what you want by default.

If we are to introduce new capabilities, I might
introduce a boolean one that, when set (i.e.,
not the default), doesn't do that inversion for
users who may want that.


> I can understand your point of view too,

It's not so much my point of view as it's what
2.5.1a used to do.  See below.

(Make sure to try 2.5.1a with different combinations
of options, to see what its behavior actually was.)


> and I'm not absolutely 100% stuck 
> on my current point of view, but I'd like to see some evidence, realistic 
> examples or reasoning to convince me that highlighting matches in context 
> lines is often useful and not often a distraction.

It can depend on the chosen color.  Interpret
them as visible bounds in between which is the
content you're looking for, just like a colored
or decorated window border.  (Some people use
window managers without them at all because they
find them a distraction or a waste of screen
real estate.)


> There will be some cases where it is definitely useful, and some cases 
> where it is definitely a distraction.  If we can't be sure that one way or 
> the other is both useful and safe (avoids annoying people) then we need to 
> have a colour configuration option for it, making four related capabilities:

Let rewrite each on a single line:

   Long conceptual name            Old   New
   --------------------            ---   ---
   selected-line                   ml=   sl=
   context-line                    cx=   cx=
   matched-text-in-selected-line   mt=   ms=
   matched-text-in-context-line    mt=   mc=
   reverse-behavior-on-v                 rv

We could keep mt= to set both ms= and mc=
in one shot.  rv is what I described above
(reverse == don't invert).  This isn't like the
discussion about command-line arguments; here,
things are overriden as we move to the right of
GREP_COLORS's value.


> In fact, I can't see any drawback to having that set of capabilities.
> 
> (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.

> "matched-text-in-selected-line" only, since no highlighting used to be done 
> on context lines.)

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.


> 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.

The other discussion is about prline() being
called at all for some lines, e.g., with -v
and -o.

I would like to fix what I did to prline() and
restore its correct functionality.  prline()
is mechanism, not policy.  The other thread is
about policy, not mechanism, and is valid too;
it's just that it's handled elsewhere.



It's interesting, though.  In both cases
(discussion threads), the things I have advocated
are what used to be in 2.5.1a (except for a bug
with needless line prefixes being printed without
a newline and with no content).  The things you
are advocating are a departure from what 2.5.1a
used to do.
More ironically, the things you are advocating in
the other thread are a consequence of something
I proposed in patch #3768 (but where I didn't
fully envision the consequences for -v).