exit: tweak exit status logic

This changes how ripgrep emit exit status codes. In particular, any error
that occurs while searching will now cause ripgrep to emit a `2` exit
code, where as it previously would emit either a `0` or a `1` code based
on whether it matched or not. That is, ripgrep would only emit a `2` exit
code for a catastrophic error.

This tweak includes additional logic that GNU grep adheres to, which seems
like good sense. Namely, if -q/--quiet is given, and an error occurs and
a match occurs, then ripgrep will emit a `0` exit code.

Closes #1159

CHANGELOG.md

@@ -4,6 +4,13 @@ TODO.
 
 **BREAKING CHANGES**:
 
+* ripgrep has tweaked its exit status codes to be more like GNU grep's. Namely,
+  if a non-fatal error occurs during a search, then ripgrep will now always
+  emit a `2` exit status code, regardless of whether a match is found or not.
+  Previously, ripgrep would only emit a `2` exit status code for a catastrophic
+  error (e.g., regex syntax error). One exception to this is if ripgrep is run
+  with `-q/--quiet`. In that case, if an error occurs and a match is found,
+  then ripgrep will exit with a `0` exit status code.
 * The `avx-accel` feature of ripgrep has been removed since it is no longer
   necessary. All uses of AVX in ripgrep are now enabled automatically via
   runtime CPU feature detection. The `simd-accel` feature does remain
@@ -17,6 +24,8 @@ Feature enhancements:
   Add support for Brotli and Zstd to the `-z/--search-zip` flag.
 * [FEATURE #1138](https://github.com/BurntSushi/ripgrep/pull/1138):
   Add `--no-ignore-dot` flag for ignoring `.ignore` files.
+* [FEATURE #1159](https://github.com/BurntSushi/ripgrep/pull/1159):
+  ripgrep's exit status logic should now match GNU grep. See updated man page.
 * [FEATURE #1170](https://github.com/BurntSushi/ripgrep/pull/1170):
   Add `--ignore-file-case-insensitive` for case insensitive .ignore globs.
 

doc/rg.1.txt.tpl

@@ -90,7 +90,15 @@ EXIT STATUS
 -----------
 If ripgrep finds a match, then the exit status of the program is 0. If no match
 could be found, then the exit status is 1. If an error occurred, then the exit
-status is 2.
+status is always 2 unless ripgrep was run with the *--quiet* flag and a match
+was found. In summary:
+
+* `0` exit status occurs only when at least one match was found, and if
+  no error occurred, unless *--quiet* was given.
+* `1` exit status occurs only when no match was found and no error occurred.
+* `2` exit status occurs when an error occurred. This is true for both
+  catastrophic errors (e.g., a regex syntax error) and for soft errors (e.g.,
+  unable to read a file).
 
 
 CONFIGURATION FILES

src/args.rs

@@ -268,6 +268,11 @@ impl Args {
         Ok(builder.build(wtr))
     }
 
+    /// Returns true if and only if ripgrep should be "quiet."
+    pub fn quiet(&self) -> bool {
+        self.matches().is_present("quiet")
+    }
+
     /// Returns true if and only if the search should quit after finding the
     /// first match.
     pub fn quit_after_match(&self) -> Result<bool> {

src/main.rs

@@ -22,33 +22,37 @@ mod subject;
 type Result<T> = ::std::result::Result<T, Box<::std::error::Error>>;
 
 fn main() {
-    match Args::parse().and_then(try_main) {
-        Ok(true) => process::exit(0),
-        Ok(false) => process::exit(1),
-        Err(err) => {
-            eprintln!("{}", err);
-            process::exit(2);
-        }
+    if let Err(err) = Args::parse().and_then(try_main) {
+        eprintln!("{}", err);
+        process::exit(2);
     }
 }
 
-fn try_main(args: Args) -> Result<bool> {
+fn try_main(args: Args) -> Result<()> {
     use args::Command::*;
 
-    match args.command()? {
-        Search => search(args),
-        SearchParallel => search_parallel(args),
-        SearchNever => Ok(false),
-        Files => files(args),
-        FilesParallel => files_parallel(args),
-        Types => types(args),
+    let matched =
+        match args.command()? {
+            Search => search(&args),
+            SearchParallel => search_parallel(&args),
+            SearchNever => Ok(false),
+            Files => files(&args),
+            FilesParallel => files_parallel(&args),
+            Types => types(&args),
+        }?;
+    if matched && (args.quiet() || !messages::errored()) {
+        process::exit(0)
+    } else if messages::errored() {
+        process::exit(2)
+    } else {
+        process::exit(1)
     }
 }
 
 /// The top-level entry point for single-threaded search. This recursively
 /// steps through the file list (current directory by default) and searches
 /// each file sequentially.
-fn search(args: Args) -> Result<bool> {
+fn search(args: &Args) -> Result<bool> {
     let started_at = Instant::now();
     let quit_after_match = args.quit_after_match()?;
     let subject_builder = args.subject_builder();
@@ -68,7 +72,7 @@ fn search(args: Args) -> Result<bool> {
                 if err.kind() == io::ErrorKind::BrokenPipe {
                     break;
                 }
-                message!("{}: {}", subject.path().display(), err);
+                err_message!("{}: {}", subject.path().display(), err);
                 continue;
             }
         };
@@ -91,7 +95,7 @@ fn search(args: Args) -> Result<bool> {
 /// The top-level entry point for multi-threaded search. The parallelism is
 /// itself achieved by the recursive directory traversal. All we need to do is
 /// feed it a worker for performing a search on each file.
-fn search_parallel(args: Args) -> Result<bool> {
+fn search_parallel(args: &Args) -> Result<bool> {
     use std::sync::atomic::AtomicBool;
     use std::sync::atomic::Ordering::SeqCst;
 
@@ -127,7 +131,7 @@ fn search_parallel(args: Args) -> Result<bool> {
             let search_result = match searcher.search(&subject) {
                 Ok(search_result) => search_result,
                 Err(err) => {
-                    message!("{}: {}", subject.path().display(), err);
+                    err_message!("{}: {}", subject.path().display(), err);
                     return WalkState::Continue;
                 }
             };
@@ -144,7 +148,7 @@ fn search_parallel(args: Args) -> Result<bool> {
                     return WalkState::Quit;
                 }
                 // Otherwise, we continue on our merry way.
-                message!("{}: {}", subject.path().display(), err);
+                err_message!("{}: {}", subject.path().display(), err);
             }
             if matched.load(SeqCst) && quit_after_match {
                 WalkState::Quit
@@ -169,7 +173,7 @@ fn search_parallel(args: Args) -> Result<bool> {
 /// The top-level entry point for listing files without searching them. This
 /// recursively steps through the file list (current directory by default) and
 /// prints each path sequentially using a single thread.
-fn files(args: Args) -> Result<bool> {
+fn files(args: &Args) -> Result<bool> {
     let quit_after_match = args.quit_after_match()?;
     let subject_builder = args.subject_builder();
     let mut matched = false;
@@ -199,7 +203,7 @@ fn files(args: Args) -> Result<bool> {
 /// The top-level entry point for listing files without searching them. This
 /// recursively steps through the file list (current directory by default) and
 /// prints each path sequentially using multiple threads.
-fn files_parallel(args: Args) -> Result<bool> {
+fn files_parallel(args: &Args) -> Result<bool> {
     use std::sync::atomic::AtomicBool;
     use std::sync::atomic::Ordering::SeqCst;
     use std::sync::mpsc;
@@ -251,7 +255,7 @@ fn files_parallel(args: Args) -> Result<bool> {
 }
 
 /// The top-level entry point for --type-list.
-fn types(args: Args) -> Result<bool> {
+fn types(args: &Args) -> Result<bool> {
     let mut count = 0;
     let mut stdout = args.stdout();
     for def in args.type_defs()? {

src/messages.rs

@@ -2,7 +2,9 @@ use std::sync::atomic::{ATOMIC_BOOL_INIT, AtomicBool, Ordering};
 
 static MESSAGES: AtomicBool = ATOMIC_BOOL_INIT;
 static IGNORE_MESSAGES: AtomicBool = ATOMIC_BOOL_INIT;
+static ERRORED: AtomicBool = ATOMIC_BOOL_INIT;
 
+/// Emit a non-fatal error message, unless messages were disabled.
 #[macro_export]
 macro_rules! message {
     ($($tt:tt)*) => {
@@ -12,6 +14,18 @@ macro_rules! message {
     }
 }
 
+/// Like message, but sets ripgrep's "errored" flag, which controls the exit
+/// status.
+#[macro_export]
+macro_rules! err_message {
+    ($($tt:tt)*) => {
+        crate::messages::set_errored();
+        message!($($tt)*);
+    }
+}
+
+/// Emit a non-fatal ignore-related error message (like a parse error), unless
+/// ignore-messages were disabled.
 #[macro_export]
 macro_rules! ignore_message {
     ($($tt:tt)*) => {
@@ -48,3 +62,13 @@ pub fn ignore_messages() -> bool {
 pub fn set_ignore_messages(yes: bool) {
     IGNORE_MESSAGES.store(yes, Ordering::SeqCst)
 }
+
+/// Returns true if and only if ripgrep came across a non-fatal error.
+pub fn errored() -> bool {
+    ERRORED.load(Ordering::SeqCst)
+}
+
+/// Indicate that ripgrep has come across a non-fatal error.
+pub fn set_errored() {
+    ERRORED.store(true, Ordering::SeqCst);
+}

src/subject.rs

@@ -41,7 +41,7 @@ impl SubjectBuilder {
         match result {
             Ok(dent) => self.build(dent),
             Err(err) => {
-                message!("{}", err);
+                err_message!("{}", err);
                 None
             }
         }
@@ -127,9 +127,19 @@ impl Subject {
         self.dent.is_stdin()
     }
 
-    /// Returns true if and only if this subject points to a directory.
+    /// Returns true if and only if this subject points to a directory after
+    /// following symbolic links.
     fn is_dir(&self) -> bool {
-        self.dent.file_type().map_or(false, |ft| ft.is_dir())
+        let ft = match self.dent.file_type() {
+            None => return false,
+            Some(ft) => ft,
+        };
+        if ft.is_dir() {
+            return true;
+        }
+        // If this is a symlink, then we want to follow it to determine
+        // whether it's a directory or not.
+        self.dent.path_is_symlink() && self.dent.path().is_dir()
     }
 
     /// Returns true if and only if this subject points to a file.

tests/regression.rs

@@ -593,10 +593,47 @@ rgtest!(r1130, |dir: Dir, mut cmd: TestCommand| {
 });
 
 // See: https://github.com/BurntSushi/ripgrep/issues/1159
-rgtest!(r1159, |_: Dir, mut cmd: TestCommand| {
+rgtest!(r1159_invalid_flag, |_: Dir, mut cmd: TestCommand| {
     cmd.arg("--wat").assert_exit_code(2);
 });
 
+// See: https://github.com/BurntSushi/ripgrep/issues/1159
+rgtest!(r1159_exit_status, |dir: Dir, _: TestCommand| {
+    dir.create("foo", "test");
+
+    // search with a match gets 0 exit status.
+    let mut cmd = dir.command();
+    cmd.arg("test").assert_exit_code(0);
+
+    // search with --quiet and a match gets 0 exit status.
+    let mut cmd = dir.command();
+    cmd.arg("-q").arg("test").assert_exit_code(0);
+
+    // search with a match and an error gets 2 exit status.
+    let mut cmd = dir.command();
+    cmd.arg("test").arg("no-file").assert_exit_code(2);
+
+    // search with a match in --quiet mode and an error gets 0 exit status.
+    let mut cmd = dir.command();
+    cmd.arg("-q").arg("test").arg("foo").arg("no-file").assert_exit_code(0);
+
+    // search with no match gets 1 exit status.
+    let mut cmd = dir.command();
+    cmd.arg("nada").assert_exit_code(1);
+
+    // search with --quiet and no match gets 1 exit status.
+    let mut cmd = dir.command();
+    cmd.arg("-q").arg("nada").assert_exit_code(1);
+
+    // search with no match and an error gets 2 exit status.
+    let mut cmd = dir.command();
+    cmd.arg("nada").arg("no-file").assert_exit_code(2);
+
+    // search with no match in --quiet mode and an error gets 2 exit status.
+    let mut cmd = dir.command();
+    cmd.arg("-q").arg("nada").arg("foo").arg("no-file").assert_exit_code(2);
+});
+
 // See: https://github.com/BurntSushi/ripgrep/issues/1163
 rgtest!(r1163, |dir: Dir, mut cmd: TestCommand| {
     dir.create("bom.txt", "\u{FEFF}test123\ntest123");