[PATCH v4 1/2] reset: add '-' shorthand for '@{-1}'

[PATCH v4 1/2] reset: add '-' shorthand for '@{-1}'

[Sundararajan R]

Teaching reset the - shorthand involves checking if any file named '-' exists.
check_filename() is used to perform this check.

When the @{-1} branch does not exist then it can be safely assumed that the
user is referring to the file '-',if any. If this file exists then it is reset.
Otherwise, a bad flag error is shown.

But if the @{-1} branch exists then it becomes ambiguous without the explicit 
'--' disambiguation as to whether the user wants to reset the file '-' or if 
he wants to reset the working tree to the previous branch. Hence the program dies
with a message about the ambiguous argument.

When none of the above cases hold, - behaves like @{-1}.

Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Sundararajan R <dyoucme@gmail.com>
---
Corrected a minor style error.

 builtin/reset.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/builtin/reset.c b/builtin/reset.c
index 4c08ddc..80dd5d5 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -192,6 +192,8 @@ static void parse_args(struct pathspec *pathspec,
 {
 	const char *rev = "HEAD";
 	unsigned char unused[20];
+	int file_named_minus = 0;
+	int shorthand = 0;
 	/*
 	 * Possible arguments are:
 	 *
@@ -205,6 +207,12 @@ static void parse_args(struct pathspec *pathspec,
 	 */
 
 	if (argv[0]) {
+		if (!strcmp(argv[0], "-") && !argv[1]) {
+			argv[0] = "@{-1}";
+			shorthand = 1;
+			if (check_filename(prefix, "-"))
+				file_named_minus = 1;
+		}
 		if (!strcmp(argv[0], "--")) {
 			argv++; /* reset to HEAD, possibly with paths */
 		} else if (argv[1] && !strcmp(argv[1], "--")) {
@@ -222,11 +230,20 @@ static void parse_args(struct pathspec *pathspec,
 			 * Ok, argv[0] looks like a commit/tree; it should not
 			 * be a filename.
 			 */
-			verify_non_filename(prefix, argv[0]);
+			if (file_named_minus) {
+				die(_("ambiguous argument '-': both revision and filename\n"
+				"Use '--' to separate paths from revisions, like this:\n"
+				"'git <command> [<revision>...] -- [<file>...]'"));
+			}
+			else if (!shorthand) 
+				verify_non_filename(prefix, argv[0]);
 			rev = *argv++;
 		} else {
 			/* Otherwise we treat this as a filename */
-			verify_filename(prefix, argv[0], 1);
+			if (shorthand)
+				argv[0] = "-";
+			if (!file_named_minus)
+				verify_filename(prefix, argv[0], 1);
 		}
 	}
 	*rev_ret = rev;
-- 
2.1.0

[PATCH v4 2/2] reset: add tests for git reset -

[Sundararajan R]

The failure case which occurs on teaching git the '-' shorthand
is when there exists no branch pointed to by '@{-1}'. In this case, if there 
is a file named - in the working tree, the user can be unambiguously 
assumed to be referring to it while issuing this command.

The ambiguous case occurs when the @{-1} branch exists and file named '-' also
exists in the working tree. This are also treated as a failure case but here 
the user is given advice as to how he can proceed.

Another potentially tricky case is when the file '@{-1}' exists. In this case,
the command should succeed as the user hasn't mentioned the file '@{-1}' and can
be safely assumed to be referring to the @{-1} branch.

Add tests to check the handling of these cases.
Also add a test to verify that reset - behaves like reset @{-1} when none
of the above cases are true.

Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Helped-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
Signed-off-by: Sundararajan R <dyoucme@gmail.com>
---
Have made the edits suggested by Matthew and Kevin.

 t/t7102-reset.sh | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)

diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index 98bcfe2..a605c32 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -568,4 +568,78 @@ test_expect_success 'reset --mixed sets up work tree' '
 	test_cmp expect actual
 '
 
+test_expect_success 'reset - with no @{-1} branch should fail' '
+	test_when_finished rm -rf new &&
+	git init new &&
+	(
+		cd new &&
+		test_must_fail git reset - 2>../actual
+	) && 
+	test_i18ngrep "bad flag" actual 
+'
+
+test_expect_success 'reset - with no @{-1} branch and file named - should succeed' '
+	test_when_finished rm -rf new &&
+	git init new &&
+	(
+		cd new &&
+		echo "Hello" >- &&
+		git add - &&
+		git reset - >../actual 
+	) &&
+	test_must_be_empty actual
+'
+
+test_expect_success 'reset - with @{-1} branch and file named - should fail' '
+	test_when_finished rm -rf new &&
+	git init new &&
+	(
+		cd new && 
+		echo "Hello" >- &&
+		git add - &&
+		git commit -m "first_commit" &&
+		git checkout -b new_branch &&
+		>- &&
+		git add - &&
+		test_must_fail git reset - 2>../actual 
+	) &&
+	test_i18ngrep "ambiguous argument" actual 
+'
+
+test_expect_success 'reset - with @{-1} branch and file named @{-1} should succeed' '
+	test_when_finished rm -rf new &&
+	git init new &&
+	(
+		cd new && 
+		echo "Hello" >@{-1} &&
+		git add @{-1} &&
+		git commit -m "first_commit" &&
+		git checkout -b new_branch &&
+		>@{-1} &&
+		git add @{-1} &&
+		git reset - >../actual 
+	) &&
+	test_i18ngrep "Unstaged" actual 
+'
+
+test_expect_success 'reset - with @{-1} branch and no file named - should succeed' '
+	test_when_finished rm -rf new &&
+	git init new &&
+	(
+		cd new &&
+		echo "Hey" >new_file &&
+		git add new_file &&
+		git commit -m "first_commit" &&
+		git checkout -b new_branch &&
+		>new_file &&
+		git add new_file &&
+		git reset - &&
+		git status -uno --porcelain >actual &&
+		git add new_file &&
+		git reset @{-1} &&
+		git status -uno --porcelain >expected &&
+		test_cmp actual expected 
+	)
+'
+
 test_done
-- 
2.1.0