Re: Applying outstanding patches [bug-grep]

[Tim Waugh]

On Thu, Apr 28, 2005 at 01:18:50PM +0100, Julian Foad wrote:

> bracket:
> 
> I'm happy with this fix but am having difficulty making a test script work 
> reliably.  As soon as I do, I'll check it in.

Really?  The how-to-reproduce section of bug #108484 is what I used
myself I think:

  https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=108484

> color:
> 
> This clears to end of line after each output.  I'm not sure that's the 
> right fix, but it's probably reasonable.  However, the implementation 
> appears wrong, missing the "only_matching" and "match_icase" cases.

Yes, agreed for "only_matching"---although I haven't been able to
trigger the problem when -o is used actually.  But not for
"match_icase", because:

> icolor:
> 
> This removes a block of code that is admittedly bad, but removing it seems 
> to reduce functionality.  Removing this code means that no insensitive 
> matches are coloured, whereas previously they would be coloured as long as 
> the pattern was specified in lower case.  Is this intended to be used in 
> conjunction with another patch that adds back better functionality?

The removed code is completely bogus and should never be used.  It is
never needed.  The (*execute) functions handle case insensitivity, and
so no special-casing is needed. (Try it!)

As far as the color patch, then, how about this?:

--- grep-2.5.1a/src/grep.c.color        2005-04-28 16:25:00.000000000 +0100
+++ grep-2.5.1a/src/grep.c      2005-04-28 16:38:42.000000000 +0100
@@ -551,10 +551,10 @@
            printf("\33[%sm", grep_color);
          fwrite(b, sizeof (char), match_size, stdout);
          if(color_option)
-           fputs("\33[00m", stdout);
+           fputs("\33[00m\33[K", stdout);
          fputs("\n", stdout);
          beg = b + match_size;
         }
       lastout = lim;
       if(line_buffered)
        fflush(stdout);
@@ -607,6 +608,7 @@
          fputs ("\33[00m", stdout);
          beg = b + match_size;
        }
+      fputs ("\33[K", stdout);
     }
   fwrite (beg, 1, lim - beg, stdout);
   if (ferror (stdout))


> oi:
> 
> OK, I know what this intends to fix, and I have regression tests (in 
> tests/foad1.sh).  The patch looks like it may depend on certain versions of 
> regex library functions.  Can you state the requirements for this patch to 
> work?  (e.g. What about "configure --with-included-regex"?)

I think the consensus with this patch had been that we'd leave it out
until we update the bundled regex.  The glibc CVS commit that added
RE_ICASE was on 2002-02-26.

> Yup, that was causing confusion.  Thanks for clarifying.  So, could you 
> give a concise description (a few lines) of each of those?  That is, what 
> do they do, and why, and is there any way of testing them?

(I've done that in the bug reports.)

> I apologise for my lack of knowledge of how Grep works.  Like it or not, 
> compared to me, you are the expert. :-)

Heh :-)

Tim.
*/

pgpu9qy7yxmg0.pgp
Description: PGP signature