[PATCH 17/19] reset $sha1 $pathspec: require $sha1 only to be treeish
- From
Martin von Zweigbergk <martinvonz@gmail.com>
- Date
- Jan 9, 2013, 08:16 UTC
- Message-ID
- <1357719376-16406-18-git-send-email-martinvonz@gmail.com>
- In-Reply-To
- <1357719376-16406-1-git-send-email-martinvonz@gmail.com>
Resetting with paths does not update HEAD and there is nothing else that a commit should be needed for. Relax the argument parsing so only a tree is required.
The sha1 is only passed to read_from_tree(), which already only requires a tree.
The "rev" variable we pass to run_add_interactive() will resolve to a tree. This is fine since interactive_reset only needs the parameter to be a treeish and doesn't use it for display purposes. --- Is it correct that interactive_reset does not use the revision specifier for display purposes? Or, worse, that it requires it to be a commit in some cases? I tried it and didn't see any problem.
Can the two blocks of code that look up commit or tree be made to share more? I'm not very familiar with what functions are available. I think I tried keeping a separate "struct object *object" to be able to put the last three lines outside the blocks, but didn't like the result.
builtin/reset.c | 46 ++++++++++++++++++++++++++-------------------- t/t7102-reset.sh | 8 ++++++++ 2 files changed, 34 insertions(+), 20 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c index a2e69eb..4c223bd 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -177,9 +177,10 @@ const char **parse_args(int argc, const char **argv, const char *prefix, const c /* * Possible arguments are: * - * git reset [-opts] <rev> <paths>... - * git reset [-opts] <rev> -- <paths>... - * git reset [-opts] -- <paths>... + * git reset [-opts] [<rev>] + * git reset [-opts] <tree> [<paths>...] + * git reset [-opts] <tree> -- [<paths>...] + * git reset [-opts] -- [<paths>...] * git reset [-opts] <paths>... * * At this point, argv points immediately after [-opts]. @@ -194,11 +195,13 @@ const char **parse_args(int argc, const char **argv, const char *prefix, const c } /* * Otherwise, argv[0] could be either <rev> or <paths> and - * has to be unambiguous. + * has to be unambiguous. If there is a single argument, it + * can not be a tree */ - else if (!get_sha1_committish(argv[0], unused)) { + else if ((argc == 1 && !get_sha1_committish(argv[0], unused)) || + (argc > 1 && !get_sha1_treeish(argv[0], unused))) { /* - * Ok, argv[0] looks like a rev; it should not + * Ok, argv[0] looks like a commit/tree; it should not * be a filename. */ verify_non_filename(prefix, argv[0]); @@ -240,7 +243,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix) const char *rev; unsigned char sha1[20]; const char **pathspec = NULL; - struct commit *commit; + struct commit *commit = NULL; const struct option options[] = { OPT__QUIET(&quiet, N_("be quiet, only report errors")), OPT_SET_INT(0, "mixed", &reset_type, @@ -262,19 +265,22 @@ int cmd_reset(int argc, const char **argv, const char *prefix) PARSE_OPT_KEEP_DASHDASH); pathspec = parse_args(argc, argv, prefix, &rev); - if (get_sha1_committish(rev, sha1)) - die(_("Failed to resolve '%s' as a valid ref."), rev); - - /* - * NOTE: As "git reset $treeish -- $path" should be usable on - * any tree-ish, this is not strictly correct. We are not - * moving the HEAD to any commit; we are merely resetting the - * entries in the index to that of a treeish. - */ - commit = lookup_commit_reference(sha1); - if (!commit) - die(_("Could not parse object '%s'."), rev); - hashcpy(sha1, commit->object.sha1); + if (!pathspec) { + if (get_sha1_committish(rev, sha1)) + die(_("Failed to resolve '%s' as a valid revision."), rev); + commit = lookup_commit_reference(sha1); + if (!commit) + die(_("Could not parse object '%s'."), rev); + hashcpy(sha1, commit->object.sha1); + } else { + struct tree *tree; + if (get_sha1_treeish(rev, sha1)) + die(_("Failed to resolve '%s' as a valid tree."), rev); + tree = parse_tree_indirect(sha1); + if (!tree) + die(_("Could not parse object '%s'."), rev); + hashcpy(sha1, tree->object.sha1); + } if (patch_mode) { if (reset_type != NONE) diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh index 81b2570..1fa2a5f 100755 --- a/t/t7102-reset.sh +++ b/t/t7102-reset.sh @@ -497,4 +497,12 @@ test_expect_success 'disambiguation (4)' ' test ! -f secondfile ' +test_expect_success 'reset with paths accepts tree' ' + # for simpler tests, drop last commit containing added files + git reset --hard HEAD^ && + git reset HEAD^^{tree} -- . && + git diff --cached HEAD^ --exit-code && + git diff HEAD --exit-code +' + test_done
-- 1.8.1.rc3.331.g1ef2165