[PATCH] diff: report bogus input to -C/-M/-B

We already detect invalid input to these functions, but we
simply exit with an error code, never saying anything as
simple as "your input was wrong". Let's fix that.

Before:

  $ git diff -CM
  $ echo $?
  128

After:

  $ git diff -CM
  error: invalid argument to -C: M
  $ echo $?
  128

There should be no problems with having diff_opt_parse print
to stderr, as there is already precedent in complaining
about bogus --color and --output arguments.

Signed-off-by: Jeff King <redacted>
---
More old patch potpourri. Original thread:

  http://article.gmane.org/gmane.comp.version-control.git/146570

I think this one just got overlooked.

 diff.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/diff.c b/diff.c
index 71efa8e..8487643 100644
--- a/diff.c
+++ b/diff.c

@@ -3142,18 +3142,18 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 	/* renames options */
 	else if (!prefixcmp(arg, "-B")) {
 		if ((options->break_opt = diff_scoreopt_parse(arg)) == -1)
-			return -1;
+			return error("invalid argument to -B: %s", arg+2);
 	}
 	else if (!prefixcmp(arg, "-M")) {
 		if ((options->rename_score = diff_scoreopt_parse(arg)) == -1)
-			return -1;
+			return error("invalid argument to -M: %s", arg+2);
 		options->detect_rename = DIFF_DETECT_RENAME;
 	}
 	else if (!prefixcmp(arg, "-C")) {
 		if (options->detect_rename == DIFF_DETECT_COPY)
 			DIFF_OPT_SET(options, FIND_COPIES_HARDER);
 		if ((options->rename_score = diff_scoreopt_parse(arg)) == -1)
-			return -1;
+			return error("invalid argument to -C: %s", arg+2);
 		options->detect_rename = DIFF_DETECT_COPY;
 	}
 	else if (!strcmp(arg, "--no-renames"))

-- 
1.7.3.1.235.gdd6c0

Jeff King wrote:

Before:

  $ git diff -CM
  $ echo $?
  128

After:

  $ git diff -CM
  error: invalid argument to -C: M
  $ echo $?
  128

Yes, please.

Who is it that exits(128) in this code path?  Are there other
functions it calls that might return an error without reporting it to
stderr?

On Thu, Oct 21, 2010 at 11:53:40AM -0500, Jonathan Nieder wrote:

Jeff King wrote:

quoted

Before:

  $ git diff -CM
  $ echo $?
  128

After:

  $ git diff -CM
  error: invalid argument to -C: M
  $ echo $?
  128

Yes, please.

Who is it that exits(128) in this code path?  Are there other
functions it calls that might return an error without reporting it to
stderr?

It's setup_revisions(). It only happens when calling
handle_revision_opt(), which either succeeds, calls error() for its
error cases, or chains to diff_opt_parse(). The latter uses error()
appropriately except for the -B/-M/-C cases.

So I think we're good after this patch is applied.

-Peff

On Oct 21, 2010, at 7:49 AM, Jeff King wrote:

We already detect invalid input to these functions, but we
simply exit with an error code, never saying anything as
simple as "your input was wrong". Let's fix that.

Before:

 $ git diff -CM
 $ echo $?
 128

After:

 $ git diff -CM
 error: invalid argument to -C: M
 $ echo $?
 128

There should be no problems with having diff_opt_parse print
to stderr, as there is already precedent in complaining
about bogus --color and --output arguments.

Thank you. I've had a note to myself to fix this for the past month, and still haven't gotten around to it. Glad to see that procrastinating pays off! ;)

-Kevin Ballard