All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kenny Lee Sin Cheong <kenny.lee28@gmail.com>
To: git@vger.kernel.org
Cc: gitster@pobox.com, Kenny Lee Sin Cheong <kenny.lee28@gmail.com>
Subject: [PATCH/RFC 3/4] Handle arg as revision first, then option.
Date: Mon, 30 Mar 2015 13:41:54 -0400	[thread overview]
Message-ID: <1427737315-7229-4-git-send-email-kenny.lee28@gmail.com> (raw)
In-Reply-To: <1427737315-7229-1-git-send-email-kenny.lee28@gmail.com>

Check the argument as a revision at first. If it fails, then tries to
check it as an option, and finally as a pathspec.

Returns -1 when we have an ambiguous revision range, such as
"master..next", to allow the argument to get checked as an option before
calling die() from verify_non_filename(). This is because we are
allowing "-" to be given in a revision range, but making the revision
check first. Otherwise, an ambiguous argument that starts with
"-" (let's say an option) would die even though its normal behaviour is
to silently return. Instead we check for ambiguity in a revision after
making sure that the argument cannot be parsed as an option.

This problem is discussed in:
http://article.gmane.org/gmane.comp.version-control.git/265672

Signed-off-by: Kenny Lee Sin Cheong <kenny.lee28@gmail.com>
---
 revision.c  | 61 +++++++++++++++++++++++++++++++++----------------------------
 sha1_name.c |  2 +-
 2 files changed, 34 insertions(+), 29 deletions(-)

diff --git a/revision.c b/revision.c
index 570945a..1ea290f 100644
--- a/revision.c
+++ b/revision.c
@@ -1516,7 +1516,10 @@ int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, unsi
 
 			if (!cant_be_filename) {
 				*dotdot = '.';
-				verify_non_filename(revs->prefix, arg);
+				if (is_inside_work_tree() && !is_inside_git_dir() &&
+				    check_filename(revs->prefix, arg)) {
+					return -1;
+				}
 			}
 
 			a_obj = parse_object(from_sha1);
@@ -2198,40 +2201,39 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 	read_from_stdin = 0;
 	for (left = i = 1; i < argc; i++) {
 		const char *arg = argv[i];
-		if (arg[0] == '-' && arg[1] && !starts_with(arg + 1, "..")) {
-			int opts;
-
-			opts = handle_revision_pseudo_opt(submodule,
-						revs, argc - i, argv + i,
-						&flags);
-			if (opts > 0) {
-				i += opts - 1;
-				continue;
-			}
+		if (handle_revision_arg(arg, revs, flags, revarg_opt)) {
+			if (arg[0] == '-' && arg[1] && !starts_with(arg + 1, "..")) {
+				int opts;
+
+				opts = handle_revision_pseudo_opt(submodule,
+								  revs, argc - i, argv + i,
+								  &flags);
+				if (opts > 0) {
+					i += opts - 1;
+					continue;
+				}
 
-			if (!strcmp(arg, "--stdin")) {
-				if (revs->disable_stdin) {
-					argv[left++] = arg;
+				if (!strcmp(arg, "--stdin")) {
+					if (revs->disable_stdin) {
+						argv[left++] = arg;
+						continue;
+					}
+					if (read_from_stdin++)
+						die("--stdin given twice?");
+					read_revisions_from_stdin(revs, &prune_data);
 					continue;
 				}
-				if (read_from_stdin++)
-					die("--stdin given twice?");
-				read_revisions_from_stdin(revs, &prune_data);
-				continue;
-			}
 
-			opts = handle_revision_opt(revs, argc - i, argv + i, &left, argv);
-			if (opts > 0) {
-				i += opts - 1;
+				opts = handle_revision_opt(revs, argc - i, argv + i, &left, argv);
+				if (opts > 0) {
+					i += opts - 1;
+					continue;
+				}
+				if (opts < 0)
+					exit(128);
 				continue;
 			}
-			if (opts < 0)
-				exit(128);
-			continue;
-		}
-
 
-		if (handle_revision_arg(arg, revs, flags, revarg_opt)) {
 			int j;
 			if (seen_dashdash || *arg == '^')
 				die("bad revision '%s'", arg);
@@ -2249,6 +2251,9 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 			break;
 		}
 		else
+			/* Make sure that a filename doesn't get interpreted as a revision */
+			if (!seen_dashdash)
+				verify_non_filename(revs->prefix, arg);
 			got_rev_arg = 1;
 	}
 
diff --git a/sha1_name.c b/sha1_name.c
index 7a621ba..b99b1dc 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -483,7 +483,7 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1,
 				break;
 			}
 		}
-	} else if (len == 1 && str[0] == '-') {
+	} else if (len == 1 && str[0] == '-' && !str[1]) {
 		nth_prior = 1;
 	}
 
-- 
2.3.3.203.g8ffb468.dirty

  parent reply	other threads:[~2015-03-30 17:42 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-30 17:41 [PATCH/RFC 0/4] Adding '-' notation as @{-1} (pu, d40f108) Kenny Lee Sin Cheong
2015-03-30 17:41 ` [PATCH/RFC 1/4] Add "-" as @{-1} support for the rev-parse command Kenny Lee Sin Cheong
2015-03-30 19:46   ` Junio C Hamano
2015-03-30 17:41 ` [PATCH/RFC 2/4] t1505: add tests for '-' notation in rev-parse Kenny Lee Sin Cheong
2015-03-31  4:55   ` Torsten Bögershausen
2015-03-30 17:41 ` Kenny Lee Sin Cheong [this message]
2015-03-30 17:41 ` [PATCH/RFC 4/4] t0102: add tests for '-' notation Kenny Lee Sin Cheong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1427737315-7229-4-git-send-email-kenny.lee28@gmail.com \
    --to=kenny.lee28@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.