Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should use exit code to distinguish between no results and an invocation error #1159

Closed
wincent opened this issue Jan 11, 2019 · 11 comments
Closed
Labels
bug A bug.

Comments

@wincent
Copy link

wincent commented Jan 11, 2019

What version of ripgrep are you using?

ripgrep 0.10.0
-SIMD -AVX (compiled)
+SIMD +AVX (runtime)

Describe your question, feature request, or bug.

I see that due to #948 ripgrep learnt how to distinguish errors (exit code 2) from no results (exit code 1), like Grep.

However, if you invoke rg with a non-existent flag, you'll still get exit code 1, which means that there is no way to programmatically distinguish between no results and a botched invocation.

rg --wat
error: Found argument '--wat' which wasn't expected, or isn't valid in this context

USAGE:

    rg [OPTIONS] PATTERN [PATH ...]
    rg [OPTIONS] [-e PATTERN ...] [-f PATTERNFILE ...] [PATH ...]
    rg [OPTIONS] --files [PATH ...]
    rg [OPTIONS] --type-list
    command | rg [OPTIONS] PATTERN

For more information try --help
zsh: exit 1     rg --wat

For comparison, grep exits with status 2 for unrecognized flags. ack exits with status 255 for unrecognized flags. ag suffers from the same problem, exiting with 1 indistinctly for bad invocations and for no results (issue: ggreer/the_silver_searcher#1298).

FWIW, I wasn't able to get rg (at least not v0.10.0) to return an exit status of 2, despite what #954 ostensibly does:

rg something /non-existent-directory
/non-existent-directory: No such file or directory (os error 2)
zsh: exit 1     rg something /non-existent-directory

Note that it's still exiting with status 1.

@BurntSushi BurntSushi added the bug A bug. label Jan 11, 2019
@BurntSushi
Copy link
Owner

BurntSushi commented Jan 11, 2019

With respect to invalid invocations, yes, that is indeed a bug. It's due to the fact that we let our arg parser exit the process for us if there's an incorrect argv. Instead, we should use App::get_matches_safe and handle the error ourselves. That should hopefully be a very simple change.

The latter part of your bug report is more interesting, and it's less clear whether there's anything to do. Certainly, ripgrep does not match grep's behavior, but that is not in and of itself a reason to change something. In particular, ripgrep only emits a 2 exit code if there was a failure that caused it to exit early, i.e., a catastrophic failure. A failure to read a file or directory is not catastrophic, and in particular, ripgrep continues searching. An example of a catastrophic failure is, say, the inability to parse a given pattern:

$ rg 'bad)'
regex parse error:
    bad)
       ^
error: unopened group

$ echo $?
2

Where grep differs on this point is that it seems to emit a 2 exit status if any error occurred at all, even if a match was found. For example:

$ grep foo does-not-exist UNLICENSE
grep: does-not-exist: No such file or directory

$ echo $?
2

$ grep the does-not-exist UNLICENSE
grep: does-not-exist: No such file or directory
UNLICENSE:This is free and unencumbered software released into the public domain.
UNLICENSE:distribute this software, either in source code form or as a compiled
UNLICENSE:In jurisdictions that recognize copyright laws, the author or authors
UNLICENSE:of this software dedicate any and all copyright interest in the
UNLICENSE:software to the public domain. We make this dedication for the benefit
UNLICENSE:of the public at large and to the detriment of our heirs and

$ echo $?
2

I don't really know what the "right" behavior is here. How do you want to use the exit status specifically?

@wincent
Copy link
Author

wincent commented Jan 11, 2019

I'm using ripgrep (or ag, or ack etc) in the Ferret Vim plug-in and I'd like to provide better feedback when the user does something wrong. Right now it will say "no matches found" even if they passed a bad path or a non-existent option.

@BurntSushi
Copy link
Owner

@wincent That doesn't quite cover all our bases though... What do you want to do when a non-catastrophic error occurs but a match was found? Do users see the error messages emitted by ripgrep?

@wincent
Copy link
Author

wincent commented Jan 11, 2019

I think making 1 what it means in grep makes sense (ie. no matches were found, but there were no errors and nothing obviously wrong that the user should "fix"). 2 to mean something went wrong, catastrophic or otherwise, irrespective of whether any (partial) results happened to get turned up on the way; if the tool really wants to, it can try to parse out the results, or the errors, or both.

@BurntSushi
Copy link
Owner

@wincent Thanks for the comment, but I don't think that really helps me make my decision. Could you explain more about the user experience instead?

@wincent
Copy link
Author

wincent commented Jan 11, 2019

Right now:

  • If there are no results, the user sees a "no results" message.
  • If the user passes --bogus-option they also see "no results" because Ferret responds that way to the exit code of 1.
  • If the user does rg foo non-existent they see "no results" because of the exit code of 1.
  • If the user does rg foo non-existent . they see the results from ".".

Ideally, I think they'd instead see:

  • Results if the invocation is correct and has results.
  • The "no results" message if the invocation is correct but there are no results.
  • An error message if the invocation is incorrect, even if there are partial results.

The first two cases are the common path and already work as expected. It's that last one which is rarer, but currently can't be handled well because it is ambiguous (Ferret would need to parse the error output. Empty output + exit code 1 means no results; non-empty output plus non-zero exit code means "something went wrong"; but it would be nice to tell this from the code without relying on a fragile parsing heuristic).

@BurntSushi
Copy link
Owner

OK, thanks for explaining that. I now have a more specific question.

The first two cases are the common path and already work as expected. It's that last one which is rarer, but currently can't be handled well because it is ambiguous (Ferret would need to parse the error output. Empty output + exit code 1 means no results; non-empty output plus non-zero exit code means "something went wrong"; but it would be nice to tell this from the code without relying on a fragile parsing heuristic).

What does Ferret do today with error messages that ripgrep writes to stderr? Does it show them to users?

Also, just to make sure we're in sync, GNU grep's exit status behavior is documented as:

EXIT STATUS
       Normally  the  exit  status  is 0 if a line is selected, 1 if no lines were selected, and 2 if an error
       occurred.  However, if the -q or --quiet or --silent is used and a line is selected, the exit status is
       0 even if an error occurred.

If ripgrep behaved like the above, would that resolve your concerns?

@wincent
Copy link
Author

wincent commented Jan 11, 2019

Currently Ferret "swallows" error output. It's visible only if the user runs :messages in Vim. If ripgrep behaved like grep that would make it easier to distinguish among the cases, although Ferret doesn't use the -q flag so that bit isn't relevant to its usage pattern (which is about getting the results, not checking whether results exist).

@BurntSushi
Copy link
Owner

Currently Ferret "swallows" error output. It's visible only if the user runs :messages in Vim.

It feels to me like this is a fundamental problem here. Even if I change ripgrep's approach to handling exit statuses, you still have problems like, "users can't tell whether they supplied an invalid regex pattern or whether a search was executed and couldn't find any hits but also errored on reading a particular file." In both of those cases, looking at the messages of stderr clarifies what happened. But otherwise, both instances would produce exactly the same exit status with nothing printed to stdout.

@wincent
Copy link
Author

wincent commented Jan 11, 2019

If Ripgrep exits with status 2 I will show the error output. Otherwise it goes to the Vim quickfix listing.

@BurntSushi
Copy link
Owner

Ah interesting, OK. I think I'm convinced then, and we can just implement grep's exit status semantics.

BurntSushi added a commit that referenced this issue Jan 26, 2019
Previously, we relied on clap to handle printing either an error
message, or --help/--version output, in addition to setting the exit
status code. Unfortunately, for --help/--version output, clap was
panicking if the write failed, which can happen in fairly common
scenarios via a broken pipe error. e.g., `rg -h | head`.

We fix this by using clap's "safe" API and doing the printing ourselves.
We also set the exit code to `2` when an invalid command has been given.

Fixes #1125 and partially addresses #1159
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A bug.
Projects
None yet
Development

No branches or pull requests

2 participants