path: root/builtin/grep.c
diff options
authorJeff King <>2017-02-14 06:05:55 (GMT)
committerJunio C Hamano <>2017-02-14 19:26:37 (GMT)
commitb5b81136da11638bdf1e336d9ec371b820fbf412 (patch)
tree70f6d17a0e5269ee89d70fdf30e1096fc94c40e7 /builtin/grep.c
parent20d6421cae23d33fa153a9251360298ff7f1ce0a (diff)
grep: fix "--" rev/pathspec disambiguation
If we see "git grep pattern rev -- file" then we apply the usual rev/pathspec disambiguation rules: any "rev" before the "--" must be a revision, and we do not need to apply the verify_non_filename() check. But there are two bugs here: 1. We keep a seen_dashdash flag to handle this case, but we set it in the same left-to-right pass over the arguments in which we parse "rev". So when we see "rev", we do not yet know that there is a "--", and we mistakenly complain if there is a matching file. We can fix this by making a preliminary pass over the arguments to find the "--", and only then checking the rev arguments. 2. If we can't resolve "rev" but there isn't a dashdash, that's OK. We treat it like a path, and complain later if it doesn't exist. But if there _is_ a dashdash, then we know it must be a rev, and should treat it as such, complaining if it does not resolve. The current code instead ignores it and tries to treat it like a path. This patch fixes both bugs, and tries to comment the parsing flow a bit better. It adds tests that cover the two bugs, but also some related situations (which already worked, but this confirms that our fixes did not break anything). Signed-off-by: Jeff King <> Signed-off-by: Junio C Hamano <>
Diffstat (limited to 'builtin/grep.c')
1 files changed, 24 insertions, 5 deletions
diff --git a/builtin/grep.c b/builtin/grep.c
index 461347a..e83b33b 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -1149,7 +1149,22 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
- /* Check revs and then paths */
+ /*
+ * We have to find "--" in a separate pass, because its presence
+ * influences how we will parse arguments that come before it.
+ */
+ for (i = 0; i < argc; i++) {
+ if (!strcmp(argv[i], "--")) {
+ seen_dashdash = 1;
+ break;
+ }
+ }
+ /*
+ * Resolve any rev arguments. If we have a dashdash, then everything up
+ * to it must resolve as a rev. If not, then we stop at the first
+ * non-rev and assume everything else is a path.
+ */
for (i = 0; i < argc; i++) {
const char *arg = argv[i];
unsigned char sha1[20];
@@ -1158,13 +1173,14 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
if (!strcmp(arg, "--")) {
- seen_dashdash = 1;
- /* Stop at the first non-rev */
- if (get_sha1_with_context(arg, 0, sha1, &oc))
+ if (get_sha1_with_context(arg, 0, sha1, &oc)) {
+ if (seen_dashdash)
+ die(_("unable to resolve revision: %s"), arg);
+ }
object = parse_object_or_die(sha1, arg);
if (!seen_dashdash)
@@ -1172,7 +1188,10 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
add_object_array_with_path(object, arg, &list, oc.mode, oc.path);
- /* The rest are paths */
+ /*
+ * Anything left over is presumed to be a path. But in the non-dashdash
+ * "do what I mean" case, we verify and complain when that isn't true.
+ */
if (!seen_dashdash) {
int j;
for (j = i; j < argc; j++)