Re: Applying outstanding patches [bug-grep]

[Julian Foad]

Tim Waugh wrote:
> 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

Sorry, I should have been more specific.  I have written a test script to 
reproduce it, and it works when I run the script by hand ("GREP=src/grep 
tests/foad1.sh"), but when the script is run from within "make check" the LANG 
environment variable does not get through from the calling line in my script to 
the function defined in the same script.  My attempt is attached.

> > 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!)

I did try it briefly, and I thought it made one of the tests in foad1.sh fail 
that otherwise passed, but perhaps I was too hasty.  Trying it again now, I'm 
finding here also that it differs depending on whether I run it from "make 
check" or alone.  I'll give it some more attention some time.

> 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);

I don't know, and haven't got time to review right now.  You probably wanted 
braces, at least.


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

Thanks.  I'll annotate the patch issue if you haven't.

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

Thanks.

- Julian

### LANG seems to be propagated into grep_test or not depending on whether the 
test script runs stand-alone or within the makefile.

Index: src/dfa.c
===================================================================
RCS file: /cvsroot/grep/grep/src/dfa.c,v
retrieving revision 1.34
diff -u -3 -p -d -r1.34 dfa.c
--- src/dfa.c   11 Apr 2005 22:36:32 -0000      1.34
+++ src/dfa.c   12 Apr 2005 19:43:11 -0000
@@ -632,7 +632,7 @@ parse_bracket_exp_mb ()
                      work_mbc->coll_elems[work_mbc->ncoll_elems++] = elem;
                    }
                }
-             wc = WEOF;
+             wc1 = wc = WEOF;
            }
          else
            /* We treat '[' as a normal character here.  */
Index: tests/foad1.sh
===================================================================
RCS file: /cvsroot/grep/grep/tests/foad1.sh,v
retrieving revision 1.3
diff -u -3 -p -d -r1.3 foad1.sh
--- tests/foad1.sh      27 Apr 2005 18:30:29 -0000      1.3
+++ tests/foad1.sh      27 Apr 2005 20:22:54 -0000
@@ -19,6 +19,7 @@ grep_test ()
   OUTPUT=`echo -n "$INPUT" | tr "/" "\n" | "$GREP" "$@" | tr "\n" "/"`
   if test "$OUTPUT" != "$EXPECT" || test "$VERBOSE" == "1"; then
     echo "Testing:  $GREP $@"
+    echo "  (LANG=$LANG)"
     echo "  input:  \"$INPUT\""
     echo "  output: \"$OUTPUT\""
   fi
@@ -65,4 +66,8 @@ grep_test "A/CX/B/C/" "A/B/C/" -wF -e A 
 grep_test "LIN7C 55327/" "" -wF -e 5327 -e 5532
 
 
+# Test character class erroneously matching a '[' character.
+LANG=de_DE.UTF-8 grep_test "[/" "" "[[:alpha:]]" -E
+
+
 exit $failures