[PATCH 1/2] revision: avoid reporting known options as unknown on error

[Jeff King <peff@peff.net>]

In parse_revision_opt(), we report "unknown option" when
handle_revision_opt() returns a negative value or zero. But these are
two different conditions: a negative value indicates a malformed option
(like a missing argument) and zero indicates an unknown option.

So malformed options produce nonsense like this:

  $ git shortlog --default
  error: bad --default argument
  error: unknown option `--default'
  usage: [...]

We should treat a negative return as a logic error which has already
been reported by handle_revision_opt(), and just show the regular usage
message.

Signed-off-by: Jeff King <peff@peff.net>
---
There's obviously a way to write this that makes the diff a little
shorter and doesn't repeat the usage_with_options() line. I think the
if/else cascade makes the mental model more clear, though.

 revision.c          | 5 ++++-
 t/t4201-shortlog.sh | 6 ++++++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/revision.c b/revision.c
index ee1df92d1d..f958d8c301 100644
--- a/revision.c
+++ b/revision.c
@@ -2771,7 +2771,10 @@ void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t *ctx,
 {
 	int n = handle_revision_opt(revs, ctx->argc, ctx->argv,
 				    &ctx->cpidx, ctx->out, NULL);
-	if (n <= 0) {
+	if (n < 0) {
+		/* handle_revision_opt() has already reported the error. */
+		usage_with_options(usagestr, options);
+	} else if (!n) {
 		error("unknown option `%s'", ctx->argv[0]);
 		usage_with_options(usagestr, options);
 	}
diff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh
index 023fbff546..4ba7f5aec6 100755
--- a/t/t4201-shortlog.sh
+++ b/t/t4201-shortlog.sh
@@ -436,4 +436,10 @@ test_expect_success 'stdin with multiple groups reports error' '
 	test_must_fail git shortlog --group=author --group=committer <log
 '
 
+test_expect_success 'invalid revision options are not reported as unknown' '
+	test_must_fail git shortlog --default 2>err &&
+	test_grep "bad --default argument" err &&
+	test_grep ! "unknown option" err
+'
+
 test_done
-- 
2.56.0.rc2.289.g137cf50cac