Re: [PATCH] grep: -r with no args now searches "."

[Paul Eggert]

On 01/03/12 00:17, Paolo Bonzini wrote:

> ... except people which use GREP_OPTIONS.

Good point.  Proposed patch enclosed below.

> We should first warn about this, and then change it after a year or so.

I'm not sure that'd be helpful.  Ordinary users won't see the warning.
Had we followed this procedure when making our recent incompatible change to
the behavior for "grep '[:space:]'", I expect that the year or so's delay
would not have helped appreciably.

> In addition, it seems to me that while this behavior would be fine for -r/-R,
> keeping a default of stdin would be better for --directories=recurse,
> which is currently a synonym.

Another possibility might be to enable the new behavior only for -R,
and to have a new option --directories=recurse-dot that corresponds
to -R.  This would be less likely to be a backwards-compatibility issue,
as it would limit the fallout only to users who put -R in their
GREP_OPTIONS.  This idea could be combined with the patch below, or applied
separately, and I'll code it up if there's interest.  A downside of
the only-if-'-R' approach is that it complicates the documentation / help
a bit more.

Anyway, rather than revert, how about the following patch?

>From 56b252075f85edfdaf6123fdd147a8c0682d1e50 Mon Sep 17 00:00:00 2001
From: Paul Eggert <address@hidden>
Date: Tue, 3 Jan 2012 14:42:28 -0800
Subject: [PATCH] grep: with no args, search "." only if command-line -r

* NEWS: Document this.
* doc/grep.texi (Environment Variables, grep Programs): Likewise.
* src/main.c (usage): Likewise.
(main): Implement this.
(prepend_default_options): Return a count of prepended options.
* tests/r-dot: Test the above.
---
 NEWS          |    6 ++++--
 doc/grep.texi |   13 ++++++++++---
 src/main.c    |   25 +++++++++++++++++--------
 tests/r-dot   |    7 +++++++
 4 files changed, 38 insertions(+), 13 deletions(-)

diff --git a/NEWS b/NEWS
index 0fd6039..b36cab7 100644
--- a/NEWS
+++ b/NEWS
@@ -35,8 +35,10 @@ GNU grep NEWS                                    -*- outline 
-*-
 
 ** New features
 
-  If no file operand is given, grep -r now searches the working directory.
-  Formerly it ignored the -r and searched standard input nonrecursively.
+  If no file operand is given, and a command-line -r or equivalent
+  option is given, grep now searches the working directory.  Formerly
+  grep ignored the -r and searched standard input nonrecursively.
+  An -r found in GREP_OPTIONS does not have this new effect.
 
   grep now supports color highlighting of matches on MS-Windows.
 
diff --git a/doc/grep.texi b/doc/grep.texi
index 5d779c1..78efb18 100644
--- a/doc/grep.texi
+++ b/doc/grep.texi
@@ -801,6 +801,13 @@ whitespace.
 A backslash escapes the next character, so it can be used to
 specify an option containing whitespace or a backslash.
 
+The @code{GREP_OPTIONS} value does not affect whether @command{grep}
+without file operands searches standard input or the working
+directory; that is affected only by command-line options.  For
+example, the command @samp{grep PAT} searches standard input and the
+command @samp{grep -r PAT} searches the working directory, regardless
+of whether @code{GREP_OPTIONS} contains @option{-r}.
+
 @item GREP_COLOR
 @vindex GREP_COLOR @r{environment variable}
 @cindex highlight markers
@@ -1014,9 +1021,9 @@ instead of strict equality with@ 2.
 for lines containing a match to the given pattern.
 By default, @command{grep} prints the matching lines.
 A file named @file{-} stands for standard input.
-If no input is specified, a recursive @command{grep}
-searches the working directory @file{.}, and a
-nonrecursive @command{grep} searches standard input.
+If no input is specified, @command{grep} searches the working
+directory @file{.} if given a command-line option specifying
+recursion; otherwise, @command{grep} searches standard input.
 There are four major variants of @command{grep},
 controlled by the following options.
 
diff --git a/src/main.c b/src/main.c
index a738050..51135ad 100644
--- a/src/main.c
+++ b/src/main.c
@@ -1522,8 +1522,8 @@ Context control:\n\
 \n"));
       fputs (_(after_options), stdout);
       printf (_("\
-When FILE is -, read standard input.  With no FILE, read '.' if recursive,\n\
-standard input if not.  If fewer than two FILEs are given, assume -h.\n\
+When FILE is -, read standard input.  With no FILE, read . if a command-line\n\
+-r is given, - otherwise.  If fewer than two FILEs are given, assume -h.\n\
 Exit status is 0 if any line is selected, 1 otherwise;\n\
 if any error occurs and -q is not given, the exit status is 2.\n"));
       printf (_("\nReport bugs to: %s\n"), PACKAGE_BUGREPORT);
@@ -1646,8 +1646,8 @@ prepend_args (char const *options, char *buf, char **argv)
 
 /* Prepend the whitespace-separated options in OPTIONS to the argument
    vector of a main program with argument count *PARGC and argument
-   vector *PARGV.  */
-static void
+   vector *PARGV.  Return the number of options prepended.  */
+static int
 prepend_default_options (char const *options, int *pargc, char ***pargv)
 {
   if (options && *options)
@@ -1663,7 +1663,10 @@ prepend_default_options (char const *options, int 
*pargc, char ***pargv)
       pp += prepend_args (options, buf, pp);
       while ((*pp++ = *argv++))
         continue;
+      return prepended;
     }
+
+  return 0;
 }
 
 /* Get the next non-digit option from ARGC and ARGV.
@@ -1807,7 +1810,8 @@ main (int argc, char **argv)
   char *keys;
   size_t keycc, oldcc, keyalloc;
   int with_filenames;
-  int opt, cc, status;
+  int opt, cc, status, prepended;
+  int prev_optind, last_recursive;
   int default_context;
   FILE *fp;
 
@@ -1843,10 +1847,12 @@ main (int argc, char **argv)
   exit_failure = EXIT_TROUBLE;
   atexit (close_stdout);
 
-  prepend_default_options (getenv ("GREP_OPTIONS"), &argc, &argv);
+  last_recursive = 0;
+  prepended = prepend_default_options (getenv ("GREP_OPTIONS"), &argc, &argv);
   setmatcher (NULL);
 
-  while ((opt = get_nondigit_option (argc, argv, &default_context)) != -1)
+  while (prev_optind = optind,
+         (opt = get_nondigit_option (argc, argv, &default_context)) != -1)
     switch (opt)
       {
       case 'A':
@@ -1936,6 +1942,8 @@ main (int argc, char **argv)
       case 'd':
         directories = XARGMATCH ("--directories", optarg,
                                  directories_args, directories_types);
+        if (directories == RECURSE_DIRECTORIES)
+          last_recursive = prev_optind;
         break;
 
       case 'e':
@@ -2024,6 +2032,7 @@ main (int argc, char **argv)
       case 'R':
       case 'r':
         directories = RECURSE_DIRECTORIES;
+        last_recursive = prev_optind;
         break;
 
       case 's':
@@ -2250,7 +2259,7 @@ main (int argc, char **argv)
         }
       while (++optind < argc);
     }
-  else if (directories == RECURSE_DIRECTORIES)
+  else if (directories == RECURSE_DIRECTORIES && prepended < last_recursive)
     {
       status = 1;
       if (stat (".", &stats_base.stat) == 0)
diff --git a/tests/r-dot b/tests/r-dot
index 73e593c..a1d5c0a 100755
--- a/tests/r-dot
+++ b/tests/r-dot
@@ -11,4 +11,11 @@ echo a:aaa > exp || framework_failure_
 (cd dir && grep -r aaa) > out || fail=1
 compare exp out || fail=1
 
+(cd dir && grep -r aaa < a) > out || fail=1
+compare exp out || fail=1
+
+echo aaa > exp || framework_failure_
+(cd dir && GREP_OPTIONS=-r grep aaa < a) > out || fail=1
+compare exp out || fail=1
+
 Exit $fail
-- 
1.7.6.4