Charles Levert wrote:
Additional Item Attachment:
File name: gefp3.patch Size:32 KB
\"cvs diff -u\" patch version 3
<http://savannah.gnu.org/patch/download.php?item_id=4596&item_file_id=5447>
I've at last had a look through the code of this patch and try it out, and I
like it.
The only quibble I have is a very minor one with the help text describing
PATTERN. For fgrep and egrep, the indentation being like that of option
descriptions looks odd. For grep, it would be good to say what the default is,
although it didn't before. Instead of:
egrep (and similar for fgrep):
Usage: grep [OPTION]... PATTERN [FILE] ...
Search for PATTERN in each FILE or standard input.
Example: grep -i 'hello world' menu.h main.c
Regexp selection and interpretation:
PATTERN is an extended regular expression
-e, --regexp=PATTERN use PATTERN for matching
grep:
Usage: grep [OPTION]... PATTERN [FILE] ...
Search for PATTERN in each FILE or standard input.
Example: grep -i 'hello world' menu.h main.c
Regexp selection and interpretation:
-E, --extended-regexp PATTERN is an extended regular expression
-F, --fixed-strings PATTERN is a set of newline-separated strings
-G, --basic-regexp PATTERN is a basic regular expression
[...]
I'd suggest moving the description of PATTERN to the top, for all modes,
indicating the default value in the case of "grep":
egrep (and similar for fgrep):
Usage: grep [OPTION]... PATTERN [FILE] ...
Search for PATTERN in each FILE or standard input.
PATTERN is an extended regular expression.
Example: grep -i 'hello world' menu.h main.c
Regexp selection and interpretation:
-e, --regexp=PATTERN use PATTERN for matching
grep:
Usage: grep [OPTION]... PATTERN [FILE] ...
Search for PATTERN in each FILE or standard input.
PATTERN is, by default, a basic regular expression.
Example: grep -i 'hello world' menu.h main.c
Regexp selection and interpretation:
-E, --extended-regexp PATTERN is an extended regular expression
-F, --fixed-strings PATTERN is a set of newline-separated strings
-G, --basic-regexp PATTERN is a basic regular expression
[...]
But it would be fine to leave this change as a later tidy-up if you like.
Also - a caveat if you're unapplying and reapplying the patch - note that the
deletion of src/grepmat.c was not in the patch file.
Excellent work. I can't imagine anyone disagreeing with this way of
implementing grep/egrep/fgrep in any significant way. Please go ahead and
commit it.
- Julian
