There's something strange with how `git diff' parses its flags:
git diff --some-bogus-flag
--> complains as expected
git diff --follow
--> works but there's no mention of this in the man page, and it
doesn't look like it's doing anything
git diff -C -M
--> works as expected too
git diff -CM
--> doesn't say anything (but it does exit with an error code)
git diff -CM --I-can-write-anything here!
--> does the same
--
((lambda (x) (x x)) (lambda (x) (x x))) Eli Barzilay:
http://barzilay.org/ Maze is Life!
On Fri, May 07, 2010 at 03:41:56PM -0400, Eli Barzilay wrote:
There's something strange with how `git diff' parses its flags:
Yep.
git diff --some-bogus-flag
--> complains as expected
Yep.
git diff --follow
--> works but there's no mention of this in the man page, and it
doesn't look like it's doing anything
--follow is a "diff option" even though it is about revision
traversal. This is an artifact of the implementation, where
rename-following happens at a low level where we have only diff_options.
It should be possible to fix, though (parse the option at the revision
layer, but have it set the diff_options flag).
git diff -C -M
--> works as expected too
Yep.
git diff -CM
--> doesn't say anything (but it does exit with an error code)
The revision and diff parsers do not use parse_options, and thus don't
understand things like bundling. Even if they did, -C takes an optional
argument. In this case, the argument format is bogus, so it aborts, but
for some reason there is no error message (it is the same with -B, -M,
and -C).
git diff -CM --I-can-write-anything here!
--> does the same
Because we barfed at -CM already. It only looks funny because we didn't
bother to print a message.
Patches welcome.
-Peff
On Sat, May 08, 2010 at 12:06:02AM -0400, Jeff King wrote:
quoted
git diff --follow
--> works but there's no mention of this in the man page, and it
doesn't look like it's doing anything
--follow is a "diff option" even though it is about revision
traversal. This is an artifact of the implementation, where
rename-following happens at a low level where we have only diff_options.
It should be possible to fix, though (parse the option at the revision
layer, but have it set the diff_options flag).
Actually, that doesn't help. "git diff" actually uses setup_revisions,
so many revision traversal options are parsed and ignored. E.g.:
$ git diff --pretty=short
will happily run and do nothing with the --pretty=short bit, as we are
not actually showing commits.
quoted
git diff -CM
--> doesn't say anything (but it does exit with an error code)
The revision and diff parsers do not use parse_options, and thus don't
understand things like bundling. Even if they did, -C takes an optional
argument. In this case, the argument format is bogus, so it aborts, but
for some reason there is no error message (it is the same with -B, -M,
and -C).
quoted
git diff -CM --I-can-write-anything here!
--> does the same
Because we barfed at -CM already. It only looks funny because we didn't
bother to print a message.
Patches welcome.
Actually, this patch is really quite trivial. Here it is.
-- >8 --
Subject: [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>
---
Actually, --output seems to call die(). We could do that, too, and it
would give us the slightly nicer:
fatal: invalid argument to -C: M
Though I guess that is slightly less libified, setup_revisions just
exits anyway.
diff.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/diff.c b/diff.c
index e40c127..0c955be 100644
--- a/diff.c
+++ b/diff.c
@@ -2783,18 +2783,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.1.176.gcff095.dirty