Re* [PATCH] cmd_reset: don't trash uncommitted changes unless told to

[Junio C Hamano <gitster@xxxxxxxxx>]

"Avery Pennarun" <apenwarr@xxxxxxxxx> writes:

> On 6/25/08, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>> "Avery Pennarun" <apenwarr@xxxxxxxxx> writes:
>> >>   * You say "git checkout -- file" when you want to "check out the file
>>  >>    from the index";
>>  >
>>  > The real question here is the --.  Is it strictly needed?  It's
>>  > optional in things like git-diff, which just do their best to guess
>>  > what you mean if you don't use the --.
>>
>> No, I wrote -- only for clarity, because you can happen to have a branch
>>  whose name is the same as the file.  Otherwise you can safely omit it,
>>  just like git-diff and any other commands that follow the -- convention.
>
> Oops, I got mixed up.  Only git-reset requires the --.  Would it make
> sense to bring git-reset into line with everything else, then?

Ah, interesting.  It appears that the current "reset in C" inherited that
bug from the scripted version.  It works most of the time without --
except for one place.

	# prove that the work tree is clean...
	$ git reset --hard
	HEAD is now at 7b7f39e Fix use after free() in builtin-fetch
	$ git diff
        $ git diff --cached

	# what's different since HEAD^?
        $ git diff --name-only HEAD^
        builtin-fetch.c

        # reset the path
        $ git reset HEAD^ builtin-fetch.c
	builtin-fetch.c: needs update

	# prove that HEAD did not move
	$ git rev-parse HEAD
	7b7f39eae6ab0bbcc68d3c42a5b23595880e528f
	# prove that work tree did not change
        $ git diff HEAD
        # prove that index has old version
	$ git diff --cached HEAD^

Reset is about resetting the index and --hard option tells it to propagate
the change down to the work tree as well.

There is no "reset to the index", so "reset -- path" would be a redundant
way to spell "reset HEAD path" or "reset HEAD -- path" which is even more
redundant.

As long as builti-fetch.c is not a valid ref, you should be able to get
out of the above mess by any one of:

	$ git reset builtin-fetch.c
        $ git reset -- builtin-fetch.c
        $ git reset HEAD builtin-fetch.c

but the first one complains, saying builtin-fetch.c is not a valid ref.

This may help.

diff --git a/builtin-reset.c b/builtin-reset.c
index f34acb1..c7d60f5 100644
--- a/builtin-reset.c
+++ b/builtin-reset.c
@@ -194,9 +194,21 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 	reflog_action = args_to_str(argv);
 	setenv("GIT_REFLOG_ACTION", reflog_action, 0);
 
-	if (i < argc && strcmp(argv[i], "--"))
-		rev = argv[i++];
-
+	/*
+	 * Possible arguments are:
+	 *
+	 * git reset <rev> <paths>...
+	 * git reset <rev> -- <paths>...
+	 * git reset -- <paths>...
+	 * git reset <paths>...
+	 */
+	if (i < argc && strcmp(argv[i], "--")) {
+		/* could be "git reset <path>" */
+		if (get_sha1(argv[i+1], sha1))
+			;
+		else
+			rev = argv[i++];
+	}
 	if (get_sha1(rev, sha1))
 		die("Failed to resolve '%s' as a valid ref.", rev);
 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html