[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Applying outstanding patches [bug-grep]

From: Julian Foad
Subject: Re: Applying outstanding patches [bug-grep]
Date: Thu, 28 Apr 2005 17:27:06 +0100
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b) Gecko/20050217

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


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:


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.


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:


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


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


- 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;
            /* 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\""
@@ -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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]