Re: grep --color enhancement

[Pádraig Brady]

Charles Levert wrote:

>[ Here is a message sent via private email,
>  for the benefit of everyone who subscribes
>  to the bug-grep mailing list.  ]
>
>* On Monday 2005-12-12 at 10:12:01 +0000, Pádraig Brady wrote:
>  
>
>>Hi Charles,
>>    
>>
>
>Hi.
>Thanks for your interest in GNU grep.
>
>
>  
>
>>I notice you've been working on the grep --color code lately.
>>I've a small patch (against cvs) attached, which minimises
>>the number of "match colour start" and "match colour end"
>>sequences outputted.
>>
>>For e.g. the following will output 8 colour sequences:
>>echo "test" | grep --color=always '.'
>>With my patch there will only be 2.
>>    
>>
>
>This does output less octets in the end.
>
>At issue is whether anyone currently relies on
>every individual non-empty match being "marked
>up" by an SGR_START/SGR_END pair.
>  
>
Never thought of that. That could be used I
suppose by a GUI regular expression debugger
or something.

>We already elected a while back to not "mark
>up" empty matches, which are a possibility with
>patterns such as 'a*'.
>  
>
Right. Probably better leave markup
to a more specialised tool?

>
>  
>
>>Where this really is beneficial is for multi byte chars.
>>For e.g the following e.g. which highlights non ascii
>>characters now works correctly on my UTF8 terminal:
>>
>>echo "utf∞" | LANG=C grep --color=always -E '[^ -~]'
>>    
>>
>
>Yes, but note that this is a somewhat perverse
>combination of locales (an UTF-8 one for echo
>and C for grep).
>  
>
It is. I couldn't think of another way to get grep
to match non ascii data though.
It doesn't work unless LANG=C.

>>
>>p.s. I notice the end colour sequence is now ^[m rather than ^[0m
>>Is that correct?
>>    
>>
>
>Yes (^[[m with no 0), in addition to being
>followed by ^[[K unless the ne boolean
>GREP_COLORS capability is specified.
>  
>
cool. Hope all terminals support that.
FYI I added the "highlight today" functionality to cal
and used terminfo (if available), which I found very easy.
The cal code does the equivalent to:
tput smso ; date +%d; tput rmso

>For now, the default SGR substring remains
>'01;31' for matched text, although it as well
>might be changed to just '1;31'.
>  
>
Did that in the new attached patch, since one
if its aims is to minimise output.

>The "else" code path at the top is broken by
>this change, but this would be easy to fix.
>  
>
doh! Order fixed in the attached patch

>The "if (only_matching)" code path at the bottom
>doesn't need the test within MATCH_COLOR_END(),
>but it doesn't break anything.
>  
>
I was a bit worried about leaving an unterminated colour span a line.
Anyway, I've also removed this from the from the attached patch.

thanks,
Pádraig.

--- grep.c.orig 2005-12-09 17:37:35.000000000 +0000
+++ grep.c      2005-12-12 14:36:42.000000000 +0000
@@ -130,8 +130,8 @@
 /* The color strings used for matched text.
    The user can overwrite them using the deprecated
    environment variable GREP_COLOR or the new GREP_COLORS.  */
-static const char *selected_match_color = "01;31";     /* bold red */
-static const char *context_match_color  = "01;31";     /* bold red */
+static const char *selected_match_color = "1;31";      /* bold red */
+static const char *context_match_color  = "1;31";      /* bold red */
 
 /* Other colors.  Defaults look damn good.  */
 static const char *filename_color = "35";      /* magenta */
@@ -788,6 +788,19 @@
   const char *mid = NULL;
   char *buf;           /* XXX */
   const char *ibeg;    /* XXX */
+  int match_color_end_pending = 0;
+
+#define MATCH_COLOR_START() \
+  do { if (!match_color_end_pending) { \
+    PR_SGR_START_IF(match_color); \
+    match_color_end_pending = 1; }\
+  } while (0)
+
+#define MATCH_COLOR_END() \
+  do { if (match_color_end_pending) { \
+    PR_SGR_END_IF(match_color); \
+    match_color_end_pending = 0; }\
+  } while (0)
 
   if (match_icase)     /* XXX - None of the -i stuff should be here.  */
     {
@@ -839,17 +852,21 @@
                  cur = mid;
                  mid = NULL;
                }
-             fwrite (cur, sizeof (char), b - cur, stdout);
+             if (b - cur) {
+               MATCH_COLOR_END();
+               fwrite (cur, sizeof (char), b - cur, stdout);
+              }
            }
 
-         PR_SGR_START_IF(match_color);
+         MATCH_COLOR_START();
          fwrite (b, sizeof (char), match_size, stdout);
-         PR_SGR_END_IF(match_color);
-         if (only_matching)
+         if (only_matching) {
            fputs("\n", stdout);
+         }
        }
       cur = b + match_size;
     }
+    MATCH_COLOR_END();
 
   if (buf)
     free(buf); /* XXX */