{"thread":{"id":"32568","subject":"[PATCH 02/19] reset $pathspec: exit with code 0 if successful","startedAt":"2013-01-09T08:15:57Z","lastAt":"2013-01-16T18:08:59Z","messageCount":68,"participants":["Martin von Zweigbergk","Matt Kraai","Jeff King","Junio C Hamano","Duy Nguyen"],"isPatch":true,"patchVersion":1,"patchTotal":19},"messages":[{"id":"206365","messageId":"1357719376-16406-1-git-send-email-martinvonz@gmail.com","threadId":"32568","inReplyTo":null,"subject":"[PATCH 00/19] reset improvements","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-09T08:15:57Z","receivedAt":"2013-01-09T08:15:57Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"This is kind of a re-roll of [1] (wow, apparently it took me almost\ntwo months to get done). The goal was, then and now, to teach \"git\nreset\" to work on an unborn branch and to not require a commit when a\ntree would do. This time, I also made some tangential improvements\nalong the way, mostly related to readability and performance.\n\nAs usual, the risker patches are towards the end. In particular, I\nfind it hard to evaluate how risky the last patch is. That last patch\nis responsible for much of the improvements in the timing table below,\nso it would be nice if it doesn't break things too badly (test pass,\nof course). The timings are best-of-five, wall time.\n\nCommand Before After\nreset (warm) 0.23 0.07\nreset -q (warm) 0.23 0.03\nreset . (warm) 0.09 0.07\nreset -q . (warm) 0.09 0.03\nreset --keep (warm) 0.31 0.29\nreset --keep -q (warm) 0.31 0.29\nreset (cold) 9.74 2.60\nreset -q (cold) 9.85 0.37\nreset . (cold) 2.66 2.51\nreset -q . (cold) 2.59 0.33\nreset --keep (cold) 7.58 7.52\nreset --keep -q (cold) 7.37 7.21\n\n\n\n [1] http://thread.gmane.org/gmane.comp.version-control.git/210568/focus=210855\n\nMartin von Zweigbergk (19):\n reset $pathspec: no need to discard index\n reset $pathspec: exit with code 0 if successful\n reset.c: pass pathspec around instead of (prefix, argv) pair\n reset: don't allow \"git reset -- $pathspec\" in bare repo\n reset.c: extract function for parsing arguments\n reset.c: remove unnecessary variable 'i'\n reset.c: extract function for updating {ORIG,}HEAD\n reset.c: share call to die_if_unmerged_cache()\n reset.c: replace switch by if-else\n reset --keep: only write index file once\n reset: avoid redundant error message\n reset.c: move update_index_refresh() call out of read_from_tree()\n reset.c: move lock, write and commit out of update_index_refresh()\n reset [--mixed]: don't write index file twice\n reset.c: finish entire cmd_reset() whether or not pathspec is given\n reset [--mixed] --quiet: don't refresh index\n reset $sha1 $pathspec: require $sha1 only to be treeish\n reset: allow reset on unborn branch\n reset [--mixed]: use diff-based reset whether or not pathspec was\n given\n\n builtin/reset.c | 281 +++++++++++++++++++----------------------\n t/t2013-checkout-submodule.sh | 2 +-\n t/t7102-reset.sh | 26 +++-\n t/t7106-reset-unborn-branch.sh | 52 ++++++++\n 4 files changed, 200 insertions(+), 161 deletions(-)\n create mode 100755 t/t7106-reset-unborn-branch.sh\n\n-- \n1.8.1.rc3.331.g1ef2165\n"},{"id":"206363","messageId":"1357719376-16406-2-git-send-email-martinvonz@gmail.com","threadId":"32568","inReplyTo":"1357719376-16406-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH 01/19] reset $pathspec: no need to discard index","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-09T08:15:58Z","receivedAt":"2013-01-09T08:15:58Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"Since 34110cd (Make 'unpack_trees()' have a separate source and\ndestination index, 2008-03-06), the index no longer gets clobbered by\ndo_diff_cache() and we can remove the code for discarding and\nre-reading it.\n\nThere are two paths to update_index_refresh() from cmd_reset(), but on\nboth paths, either read_cache() or read_cache_unmerged() will have\nbeen called, so the call to read_cache() in this method is redundant\n(although practically free).\n\nThis speeds up \"git reset -- .\" a little on the linux-2.6 repo (best\nof five, warm cache):\n\n Before After\nreal 0m0.093s 0m0.080s\nuser 0m0.040s 0m0.020s\nsys 0m0.050s 0m0.050s\n---\n builtin/reset.c | 16 +---------------\n 1 file changed, 1 insertion(+), 15 deletions(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 915cc9f..8cc7c72 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -126,9 +126,6 @@ static int update_index_refresh(int fd, struct lock_file *index_lock, int flags)\n \t\tfd = hold_locked_index(index_lock, 1);\n \t}\n \n-\tif (read_cache() < 0)\n-\t\treturn error(_(\"Could not read index\"));\n-\n \tresult = refresh_index(&the_index, (flags), NULL, NULL,\n \t\t\t _(\"Unstaged changes after reset:\")) ? 1 : 0;\n \tif (write_cache(fd, active_cache, active_nr) ||\n@@ -141,12 +138,6 @@ static void update_index_from_diff(struct diff_queue_struct *q,\n \t\tstruct diff_options *opt, void *data)\n {\n \tint i;\n-\tint *discard_flag = data;\n-\n-\t/* do_diff_cache() mangled the index */\n-\tdiscard_cache();\n-\t*discard_flag = 1;\n-\tread_cache();\n \n \tfor (i = 0; i < q->nr; i++) {\n \t\tstruct diff_filespec *one = q->queue[i]->one;\n@@ -179,17 +170,15 @@ static int read_from_tree(const char *prefix, const char **argv,\n \t\tunsigned char *tree_sha1, int refresh_flags)\n {\n \tstruct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n-\tint index_fd, index_was_discarded = 0;\n+\tint index_fd;\n \tstruct diff_options opt;\n \n \tmemset(&opt, 0, sizeof(opt));\n \tdiff_tree_setup_paths(get_pathspec(prefix, (const char **)argv), &opt);\n \topt.output_format = DIFF_FORMAT_CALLBACK;\n \topt.format_callback = update_index_from_diff;\n-\topt.format_callback_data = &index_was_discarded;\n \n \tindex_fd = hold_locked_index(lock, 1);\n-\tindex_was_discarded = 0;\n \tread_cache();\n \tif (do_diff_cache(tree_sha1, &opt))\n \t\treturn 1;\n@@ -197,9 +186,6 @@ static int read_from_tree(const char *prefix, const char **argv,\n \tdiff_flush(&opt);\n \tdiff_tree_release_paths(&opt);\n \n-\tif (!index_was_discarded)\n-\t\t/* The index is still clobbered from do_diff_cache() */\n-\t\tdiscard_cache();\n \treturn update_index_refresh(index_fd, lock, refresh_flags);\n }\n \n-- \n1.8.1.rc3.331.g1ef2165\n"},{"id":"206355","messageId":"1357719376-16406-3-git-send-email-martinvonz@gmail.com","threadId":"32568","inReplyTo":"1357719376-16406-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH 02/19] reset $pathspec: exit with code 0 if successful","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-09T08:15:59Z","receivedAt":"2013-01-09T08:15:59Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"\"git reset $pathspec\" currently exits with a non-zero exit code if the\nworktree is dirty after resetting, which is inconsistent with reset\nwithout pathspec, and it makes it harder to know whether the command\nreally failed. Change it to exit with code 0 regardless of whether the\nworktree is dirty so that non-zero indicates an error.\n\nThis makes the 4 \"disambiguation\" test cases in t7102 clearer since\nthey all used to \"fail\", 3 of which \"failed\" due to changes in the\nwork tree. Now only the ambiguous one fails.\n---\nI suppose this makes documenting the exit code unnecessary, since\n\"return zero iff successful\" is probably understood to be the default.\n\nThe variable in refresh_index() containing the value to be returned is\ncalled has_errors. I'm guessing I shouldn't take the name too\nseriously.\n\n builtin/reset.c | 8 +++-----\n t/t2013-checkout-submodule.sh | 2 +-\n t/t7102-reset.sh | 18 ++++++++++++------\n 3 files changed, 16 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 8cc7c72..65413d0 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -119,19 +119,17 @@ static void print_new_head_line(struct commit *commit)\n \n static int update_index_refresh(int fd, struct lock_file *index_lock, int flags)\n {\n-\tint result;\n-\n \tif (!index_lock) {\n \t\tindex_lock = xcalloc(1, sizeof(struct lock_file));\n \t\tfd = hold_locked_index(index_lock, 1);\n \t}\n \n-\tresult = refresh_index(&the_index, (flags), NULL, NULL,\n-\t\t\t _(\"Unstaged changes after reset:\")) ? 1 : 0;\n+\trefresh_index(&the_index, (flags), NULL, NULL,\n+\t\t _(\"Unstaged changes after reset:\"));\n \tif (write_cache(fd, active_cache, active_nr) ||\n \t\t\tcommit_locked_index(index_lock))\n \t\treturn error (\"Could not refresh index\");\n-\treturn result;\n+\treturn 0;\n }\n \n static void update_index_from_diff(struct diff_queue_struct *q,\ndiff --git a/t/t2013-checkout-submodule.sh b/t/t2013-checkout-submodule.sh\nindex 70edbb3..06b18f8 100755\n--- a/t/t2013-checkout-submodule.sh\n+++ b/t/t2013-checkout-submodule.sh\n@@ -23,7 +23,7 @@ test_expect_success '\"reset \" updates the index' '\n \tgit update-index --refresh &&\n \tgit diff-files --quiet &&\n \tgit diff-index --quiet --cached HEAD &&\n-\ttest_must_fail git reset HEAD^ submodule &&\n+\tgit reset HEAD^ submodule &&\n \ttest_must_fail git diff-files --quiet &&\n \tgit reset submodule &&\n \tgit diff-files --quiet\ndiff --git a/t/t7102-reset.sh b/t/t7102-reset.sh\nindex b096dc8..81b2570 100755\n--- a/t/t7102-reset.sh\n+++ b/t/t7102-reset.sh\n@@ -388,7 +388,8 @@ test_expect_success 'test --mixed ' '\n \techo 4 > file4 &&\n \techo 5 > file1 &&\n \tgit add file1 file3 file4 &&\n-\ttest_must_fail git reset HEAD -- file1 file2 file3 &&\n+\tgit reset HEAD -- file1 file2 file3 &&\n+\ttest_must_fail git diff --quiet &&\n \tgit diff > output &&\n \ttest_cmp output expect &&\n \tgit diff --cached > output &&\n@@ -402,7 +403,8 @@ test_expect_success 'test resetting the index at give paths' '\n \t>sub/file2 &&\n \tgit update-index --add sub/file1 sub/file2 &&\n \tT=$(git write-tree) &&\n-\ttest_must_fail git reset HEAD sub/file2 &&\n+\tgit reset HEAD sub/file2 &&\n+\ttest_must_fail git diff --quiet &&\n \tU=$(git write-tree) &&\n \techo \"$T\" &&\n \techo \"$U\" &&\n@@ -440,7 +442,8 @@ test_expect_success 'resetting specific path that is unmerged' '\n \t\techo \"100644 $F3 3\tfile2\"\n \t} | git update-index --index-info &&\n \tgit ls-files -u &&\n-\ttest_must_fail git reset HEAD file2 &&\n+\tgit reset HEAD file2 &&\n+\ttest_must_fail git diff --quiet &&\n \tgit diff-index --exit-code --cached HEAD\n '\n \n@@ -449,7 +452,8 @@ test_expect_success 'disambiguation (1)' '\n \tgit reset --hard &&\n \t>secondfile &&\n \tgit add secondfile &&\n-\ttest_must_fail git reset secondfile &&\n+\tgit reset secondfile &&\n+\ttest_must_fail git diff --quiet -- secondfile &&\n \ttest -z \"$(git diff --cached --name-only)\" &&\n \ttest -f secondfile &&\n \ttest ! -s secondfile\n@@ -474,7 +478,8 @@ test_expect_success 'disambiguation (3)' '\n \t>secondfile &&\n \tgit add secondfile &&\n \trm -f secondfile &&\n-\ttest_must_fail git reset HEAD secondfile &&\n+\tgit reset HEAD secondfile &&\n+\ttest_must_fail git diff --quiet &&\n \ttest -z \"$(git diff --cached --name-only)\" &&\n \ttest ! -f secondfile\n \n@@ -486,7 +491,8 @@ test_expect_success 'disambiguation (4)' '\n \t>secondfile &&\n \tgit add secondfile &&\n \trm -f secondfile &&\n-\ttest_must_fail git reset -- secondfile &&\n+\tgit reset -- secondfile &&\n+\ttest_must_fail git diff --quiet &&\n \ttest -z \"$(git diff --cached --name-only)\" &&\n \ttest ! -f secondfile\n '\n-- \n1.8.1.rc3.331.g1ef2165\n"},{"id":"206371","messageId":"1357719376-16406-4-git-send-email-martinvonz@gmail.com","threadId":"32568","inReplyTo":"1357719376-16406-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH 03/19] reset.c: pass pathspec around instead of (prefix, argv) pair","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-09T08:16:00Z","receivedAt":"2013-01-09T08:16:00Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"We use the path arguments in two places in reset.c: in\ninteractive_reset() and read_from_tree(). Both of these call\nget_pathspec(), so we pass the (prefix, arv) pair to both\nfunctions. Move the call to get_pathspec() out of these methods, for\ntwo reasons: 1) One argument is simpler than two. 2) It lets us use\nthe (arguably clearer) \"if (pathspec)\" in place of \"if (i < argc)\".\n---\nIf I understand correctly, this should be rebased on top of\nnd/parse-pathspec. Please let me know.\n\n builtin/reset.c | 27 ++++++++++-----------------\n 1 file changed, 10 insertions(+), 17 deletions(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 65413d0..045c960 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -153,26 +153,15 @@ static void update_index_from_diff(struct diff_queue_struct *q,\n \t}\n }\n \n-static int interactive_reset(const char *revision, const char **argv,\n-\t\t\t const char *prefix)\n-{\n-\tconst char **pathspec = NULL;\n-\n-\tif (*argv)\n-\t\tpathspec = get_pathspec(prefix, argv);\n-\n-\treturn run_add_interactive(revision, \"--patch=reset\", pathspec);\n-}\n-\n-static int read_from_tree(const char *prefix, const char **argv,\n-\t\tunsigned char *tree_sha1, int refresh_flags)\n+static int read_from_tree(const char **pathspec, unsigned char *tree_sha1,\n+\t\t\t int refresh_flags)\n {\n \tstruct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n \tint index_fd;\n \tstruct diff_options opt;\n \n \tmemset(&opt, 0, sizeof(opt));\n-\tdiff_tree_setup_paths(get_pathspec(prefix, (const char **)argv), &opt);\n+\tdiff_tree_setup_paths(pathspec, &opt);\n \topt.output_format = DIFF_FORMAT_CALLBACK;\n \topt.format_callback = update_index_from_diff;\n \n@@ -216,6 +205,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \tconst char *rev = \"HEAD\";\n \tunsigned char sha1[20], *orig = NULL, sha1_orig[20],\n \t\t\t\t*old_orig = NULL, sha1_old_orig[20];\n+\tconst char **pathspec = NULL;\n \tstruct commit *commit;\n \tstruct strbuf msg = STRBUF_INIT;\n \tconst struct option options[] = {\n@@ -287,22 +277,25 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\tdie(_(\"Could not parse object '%s'.\"), rev);\n \thashcpy(sha1, commit->object.sha1);\n \n+\tif (i < argc)\n+\t\tpathspec = get_pathspec(prefix, argv + i);\n+\n \tif (patch_mode) {\n \t\tif (reset_type != NONE)\n \t\t\tdie(_(\"--patch is incompatible with --{hard,mixed,soft}\"));\n-\t\treturn interactive_reset(rev, argv + i, prefix);\n+\t\treturn run_add_interactive(rev, \"--patch=reset\", pathspec);\n \t}\n \n \t/* git reset tree [--] paths... can be used to\n \t * load chosen paths from the tree into the index without\n \t * affecting the working tree nor HEAD. */\n-\tif (i < argc) {\n+\tif (pathspec) {\n \t\tif (reset_type == MIXED)\n \t\t\twarning(_(\"--mixed with paths is deprecated; use 'git reset -- ' instead.\"));\n \t\telse if (reset_type != NONE)\n \t\t\tdie(_(\"Cannot do %s reset with paths.\"),\n \t\t\t\t\t_(reset_type_names[reset_type]));\n-\t\treturn read_from_tree(prefix, argv + i, sha1,\n+\t\treturn read_from_tree(pathspec, sha1,\n \t\t\t\tquiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);\n \t}\n \tif (reset_type == NONE)\n-- \n1.8.1.rc3.331.g1ef2165\n"},{"id":"206357","messageId":"1357719376-16406-5-git-send-email-martinvonz@gmail.com","threadId":"32568","inReplyTo":"1357719376-16406-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH 04/19] reset: don't allow \"git reset -- $pathspec\" in bare repo","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-09T08:16:01Z","receivedAt":"2013-01-09T08:16:01Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"---\n builtin/reset.c | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 045c960..664fad9 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -295,8 +295,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\telse if (reset_type != NONE)\n \t\t\tdie(_(\"Cannot do %s reset with paths.\"),\n \t\t\t\t\t_(reset_type_names[reset_type]));\n-\t\treturn read_from_tree(pathspec, sha1,\n-\t\t\t\tquiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);\n \t}\n \tif (reset_type == NONE)\n \t\treset_type = MIXED; /* by default */\n@@ -308,6 +306,10 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\tdie(_(\"%s reset is not allowed in a bare repository\"),\n \t\t _(reset_type_names[reset_type]));\n \n+\tif (pathspec)\n+\t\treturn read_from_tree(pathspec, sha1,\n+\t\t\t\tquiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);\n+\n \t/* Soft reset does not touch the index file nor the working tree\n \t * at all, but requires them in a good order. Other resets reset\n \t * the index file to the tree object we are switching to. */\n-- \n1.8.1.rc3.331.g1ef2165\n"},{"id":"206360","messageId":"1357719376-16406-6-git-send-email-martinvonz@gmail.com","threadId":"32568","inReplyTo":"1357719376-16406-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH 05/19] reset.c: extract function for parsing arguments","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-09T08:16:02Z","receivedAt":"2013-01-09T08:16:02Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"Declutter cmd_reset() a bit by moving out the argument parsing to its\nown function.\n---\n builtin/reset.c | 71 ++++++++++++++++++++++++++++++---------------------------\n 1 file changed, 38 insertions(+), 33 deletions(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 664fad9..9473725 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -198,36 +198,10 @@ static void die_if_unmerged_cache(int reset_type)\n \n }\n \n-int cmd_reset(int argc, const char **argv, const char *prefix)\n-{\n-\tint i = 0, reset_type = NONE, update_ref_status = 0, quiet = 0;\n-\tint patch_mode = 0;\n+const char **parse_args(int argc, const char **argv, const char *prefix, const char **rev_ret) {\n+\tint i = 0;\n \tconst char *rev = \"HEAD\";\n-\tunsigned char sha1[20], *orig = NULL, sha1_orig[20],\n-\t\t\t\t*old_orig = NULL, sha1_old_orig[20];\n-\tconst char **pathspec = NULL;\n-\tstruct commit *commit;\n-\tstruct strbuf msg = STRBUF_INIT;\n-\tconst struct option options[] = {\n-\t\tOPT__QUIET(&quiet, N_(\"be quiet, only report errors\")),\n-\t\tOPT_SET_INT(0, \"mixed\", &reset_type,\n-\t\t\t\t\t\tN_(\"reset HEAD and index\"), MIXED),\n-\t\tOPT_SET_INT(0, \"soft\", &reset_type, N_(\"reset only HEAD\"), SOFT),\n-\t\tOPT_SET_INT(0, \"hard\", &reset_type,\n-\t\t\t\tN_(\"reset HEAD, index and working tree\"), HARD),\n-\t\tOPT_SET_INT(0, \"merge\", &reset_type,\n-\t\t\t\tN_(\"reset HEAD, index and working tree\"), MERGE),\n-\t\tOPT_SET_INT(0, \"keep\", &reset_type,\n-\t\t\t\tN_(\"reset HEAD but keep local changes\"), KEEP),\n-\t\tOPT_BOOLEAN('p', \"patch\", &patch_mode, N_(\"select hunks interactively\")),\n-\t\tOPT_END()\n-\t};\n-\n-\tgit_config(git_default_config, NULL);\n-\n-\targc = parse_options(argc, argv, prefix, options, git_reset_usage,\n-\t\t\t\t\t\tPARSE_OPT_KEEP_DASHDASH);\n-\n+\tunsigned char unused[20];\n \t/*\n \t * Possible arguments are:\n \t *\n@@ -250,7 +224,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t * Otherwise, argv[i] could be either or and\n \t\t * has to be unambiguous.\n \t\t */\n-\t\telse if (!get_sha1_committish(argv[i], sha1)) {\n+\t\telse if (!get_sha1_committish(argv[i], unused)) {\n \t\t\t/*\n \t\t\t * Ok, argv[i] looks like a rev; it should not\n \t\t\t * be a filename.\n@@ -262,6 +236,40 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t\tverify_filename(prefix, argv[i], 1);\n \t\t}\n \t}\n+\t*rev_ret = rev;\n+\treturn i < argc ? get_pathspec(prefix, argv + i) : NULL;\n+}\n+\n+int cmd_reset(int argc, const char **argv, const char *prefix)\n+{\n+\tint reset_type = NONE, update_ref_status = 0, quiet = 0;\n+\tint patch_mode = 0;\n+\tconst char *rev;\n+\tunsigned char sha1[20], *orig = NULL, sha1_orig[20],\n+\t\t\t\t*old_orig = NULL, sha1_old_orig[20];\n+\tconst char **pathspec = NULL;\n+\tstruct commit *commit;\n+\tstruct strbuf msg = STRBUF_INIT;\n+\tconst struct option options[] = {\n+\t\tOPT__QUIET(&quiet, N_(\"be quiet, only report errors\")),\n+\t\tOPT_SET_INT(0, \"mixed\", &reset_type,\n+\t\t\t\t\t\tN_(\"reset HEAD and index\"), MIXED),\n+\t\tOPT_SET_INT(0, \"soft\", &reset_type, N_(\"reset only HEAD\"), SOFT),\n+\t\tOPT_SET_INT(0, \"hard\", &reset_type,\n+\t\t\t\tN_(\"reset HEAD, index and working tree\"), HARD),\n+\t\tOPT_SET_INT(0, \"merge\", &reset_type,\n+\t\t\t\tN_(\"reset HEAD, index and working tree\"), MERGE),\n+\t\tOPT_SET_INT(0, \"keep\", &reset_type,\n+\t\t\t\tN_(\"reset HEAD but keep local changes\"), KEEP),\n+\t\tOPT_BOOLEAN('p', \"patch\", &patch_mode, N_(\"select hunks interactively\")),\n+\t\tOPT_END()\n+\t};\n+\n+\tgit_config(git_default_config, NULL);\n+\n+\targc = parse_options(argc, argv, prefix, options, git_reset_usage,\n+\t\t\t\t\t\tPARSE_OPT_KEEP_DASHDASH);\n+\tpathspec = parse_args(argc, argv, prefix, &rev);\n \n \tif (get_sha1_committish(rev, sha1))\n \t\tdie(_(\"Failed to resolve '%s' as a valid ref.\"), rev);\n@@ -277,9 +285,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\tdie(_(\"Could not parse object '%s'.\"), rev);\n \thashcpy(sha1, commit->object.sha1);\n \n-\tif (i < argc)\n-\t\tpathspec = get_pathspec(prefix, argv + i);\n-\n \tif (patch_mode) {\n \t\tif (reset_type != NONE)\n \t\t\tdie(_(\"--patch is incompatible with --{hard,mixed,soft}\"));\n-- \n1.8.1.rc3.331.g1ef2165\n"},{"id":"206370","messageId":"1357719376-16406-7-git-send-email-martinvonz@gmail.com","threadId":"32568","inReplyTo":"1357719376-16406-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH 06/19] reset.c: remove unnecessary variable 'i'","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-09T08:16:03Z","receivedAt":"2013-01-09T08:16:03Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"Throughout most of parse_args(), the variable 'i' remains at 0. In the\nremaining few cases, we can do pointer arithmentic on argv itself\ninstead.\n---\nThis is clearly mostly a matter of taste. The remainder of the series\ndoes not depend on it in any way.\n\n builtin/reset.c | 29 ++++++++++++++---------------\n 1 file changed, 14 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 9473725..68be05c 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -199,7 +199,6 @@ static void die_if_unmerged_cache(int reset_type)\n }\n \n const char **parse_args(int argc, const char **argv, const char *prefix, const char **rev_ret) {\n-\tint i = 0;\n \tconst char *rev = \"HEAD\";\n \tunsigned char unused[20];\n \t/*\n@@ -210,34 +209,34 @@ const char **parse_args(int argc, const char **argv, const char *prefix, const c\n \t * git reset [-opts] -- ...\n \t * git reset [-opts] ...\n \t *\n-\t * At this point, argv[i] points immediately after [-opts].\n+\t * At this point, argv points immediately after [-opts].\n \t */\n \n-\tif (i < argc) {\n-\t\tif (!strcmp(argv[i], \"--\")) {\n-\t\t\ti++; /* reset to HEAD, possibly with paths */\n-\t\t} else if (i + 1 < argc && !strcmp(argv[i+1], \"--\")) {\n-\t\t\trev = argv[i];\n-\t\t\ti += 2;\n+\tif (argc) {\n+\t\tif (!strcmp(argv[0], \"--\")) {\n+\t\t\targv++; /* reset to HEAD, possibly with paths */\n+\t\t} else if (argc > 1 && !strcmp(argv[1], \"--\")) {\n+\t\t\trev = argv[0];\n+\t\t\targv += 2;\n \t\t}\n \t\t/*\n-\t\t * Otherwise, argv[i] could be either or and\n+\t\t * Otherwise, argv[0] could be either or and\n \t\t * has to be unambiguous.\n \t\t */\n-\t\telse if (!get_sha1_committish(argv[i], unused)) {\n+\t\telse if (!get_sha1_committish(argv[0], unused)) {\n \t\t\t/*\n-\t\t\t * Ok, argv[i] looks like a rev; it should not\n+\t\t\t * Ok, argv[0] looks like a rev; it should not\n \t\t\t * be a filename.\n \t\t\t */\n-\t\t\tverify_non_filename(prefix, argv[i]);\n-\t\t\trev = argv[i++];\n+\t\t\tverify_non_filename(prefix, argv[0]);\n+\t\t\trev = *argv++;\n \t\t} else {\n \t\t\t/* Otherwise we treat this as a filename */\n-\t\t\tverify_filename(prefix, argv[i], 1);\n+\t\t\tverify_filename(prefix, argv[0], 1);\n \t\t}\n \t}\n \t*rev_ret = rev;\n-\treturn i < argc ? get_pathspec(prefix, argv + i) : NULL;\n+\treturn *argv ? get_pathspec(prefix, argv) : NULL;\n }\n \n int cmd_reset(int argc, const char **argv, const char *prefix)\n-- \n1.8.1.rc3.331.g1ef2165\n"},{"id":"206369","messageId":"1357719376-16406-8-git-send-email-martinvonz@gmail.com","threadId":"32568","inReplyTo":"1357719376-16406-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH 07/19] reset.c: extract function for updating {ORIG,}HEAD","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-09T08:16:04Z","receivedAt":"2013-01-09T08:16:04Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"By extracting the code for updating the HEAD and ORIG_HEAD symbolic\nreferences to a separate function, we declutter cmd_reset() a bit and\nwe make it clear that e.g. the four variables {,sha1_}{,old_}orig are\nonly used by this code.\n---\n builtin/reset.c | 39 +++++++++++++++++++++++----------------\n 1 file changed, 23 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 68be05c..4d556e7 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -239,16 +239,35 @@ const char **parse_args(int argc, const char **argv, const char *prefix, const c\n \treturn *argv ? get_pathspec(prefix, argv) : NULL;\n }\n \n+static int update_refs(const char *rev, const unsigned char *sha1) {\n+\tint update_ref_status;\n+\tstruct strbuf msg = STRBUF_INIT;\n+\tunsigned char *orig = NULL, sha1_orig[20],\n+\t\t*old_orig = NULL, sha1_old_orig[20];\n+\n+\tif (!get_sha1(\"ORIG_HEAD\", sha1_old_orig))\n+\t\told_orig = sha1_old_orig;\n+\tif (!get_sha1(\"HEAD\", sha1_orig)) {\n+\t\torig = sha1_orig;\n+\t\tset_reflog_message(&msg, \"updating ORIG_HEAD\", NULL);\n+\t\tupdate_ref(msg.buf, \"ORIG_HEAD\", orig, old_orig, 0, MSG_ON_ERR);\n+\t}\n+\telse if (old_orig)\n+\t\tdelete_ref(\"ORIG_HEAD\", old_orig, 0);\n+\tset_reflog_message(&msg, \"updating HEAD\", rev);\n+\tupdate_ref_status = update_ref(msg.buf, \"HEAD\", sha1, orig, 0, MSG_ON_ERR);\n+\tstrbuf_release(&msg);\n+\treturn update_ref_status;\n+}\n+\n int cmd_reset(int argc, const char **argv, const char *prefix)\n {\n \tint reset_type = NONE, update_ref_status = 0, quiet = 0;\n \tint patch_mode = 0;\n \tconst char *rev;\n-\tunsigned char sha1[20], *orig = NULL, sha1_orig[20],\n-\t\t\t\t*old_orig = NULL, sha1_old_orig[20];\n+\tunsigned char sha1[20];\n \tconst char **pathspec = NULL;\n \tstruct commit *commit;\n-\tstruct strbuf msg = STRBUF_INIT;\n \tconst struct option options[] = {\n \t\tOPT__QUIET(&quiet, N_(\"be quiet, only report errors\")),\n \t\tOPT_SET_INT(0, \"mixed\", &reset_type,\n@@ -332,17 +351,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \n \t/* Any resets update HEAD to the head being switched to,\n \t * saving the previous head in ORIG_HEAD before. */\n-\tif (!get_sha1(\"ORIG_HEAD\", sha1_old_orig))\n-\t\told_orig = sha1_old_orig;\n-\tif (!get_sha1(\"HEAD\", sha1_orig)) {\n-\t\torig = sha1_orig;\n-\t\tset_reflog_message(&msg, \"updating ORIG_HEAD\", NULL);\n-\t\tupdate_ref(msg.buf, \"ORIG_HEAD\", orig, old_orig, 0, MSG_ON_ERR);\n-\t}\n-\telse if (old_orig)\n-\t\tdelete_ref(\"ORIG_HEAD\", old_orig, 0);\n-\tset_reflog_message(&msg, \"updating HEAD\", rev);\n-\tupdate_ref_status = update_ref(msg.buf, \"HEAD\", sha1, orig, 0, MSG_ON_ERR);\n+\tupdate_ref_status = update_refs(rev, sha1);\n \n \tswitch (reset_type) {\n \tcase HARD:\n@@ -359,7 +368,5 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \n \tremove_branch_state();\n \n-\tstrbuf_release(&msg);\n-\n \treturn update_ref_status;\n }\n-- \n1.8.1.rc3.331.g1ef2165\n"},{"id":"206367","messageId":"1357719376-16406-9-git-send-email-martinvonz@gmail.com","threadId":"32568","inReplyTo":"1357719376-16406-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH 08/19] reset.c: share call to die_if_unmerged_cache()","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-09T08:16:05Z","receivedAt":"2013-01-09T08:16:05Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"Use a single condition to guard the call to die_if_unmerged_cache for\nboth --soft and --keep. This avoids the small distraction of the\nprecondition check from the logic following it.\n\nAlso change an instance of\n\n if (e)\n err = err || f();\n\nto the almost as short, but clearer\n\n if (e && !err)\n err = f();\n\n(which is equivalent since we only care whether exit code is 0)\n---\n builtin/reset.c | 14 ++++++--------\n 1 file changed, 6 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 4d556e7..42d1563 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -336,15 +336,13 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t/* Soft reset does not touch the index file nor the working tree\n \t * at all, but requires them in a good order. Other resets reset\n \t * the index file to the tree object we are switching to. */\n-\tif (reset_type == SOFT)\n+\tif (reset_type == SOFT || reset_type == KEEP)\n \t\tdie_if_unmerged_cache(reset_type);\n-\telse {\n-\t\tint err;\n-\t\tif (reset_type == KEEP)\n-\t\t\tdie_if_unmerged_cache(reset_type);\n-\t\terr = reset_index_file(sha1, reset_type, quiet);\n-\t\tif (reset_type == KEEP)\n-\t\t\terr = err || reset_index_file(sha1, MIXED, quiet);\n+\n+\tif (reset_type != SOFT) {\n+\t\tint err = reset_index_file(sha1, reset_type, quiet);\n+\t\tif (reset_type == KEEP && !err)\n+\t\t\terr = reset_index_file(sha1, MIXED, quiet);\n \t\tif (err)\n \t\t\tdie(_(\"Could not reset index file to revision '%s'.\"), rev);\n \t}\n-- \n1.8.1.rc3.331.g1ef2165\n"},{"id":"206361","messageId":"1357719376-16406-10-git-send-email-martinvonz@gmail.com","threadId":"32568","inReplyTo":"1357719376-16406-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH 09/19] reset.c: replace switch by if-else","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-09T08:16:06Z","receivedAt":"2013-01-09T08:16:06Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"---\n builtin/reset.c | 13 +++----------\n 1 file changed, 3 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 42d1563..05ccfd4 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -351,18 +351,11 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t * saving the previous head in ORIG_HEAD before. */\n \tupdate_ref_status = update_refs(rev, sha1);\n \n-\tswitch (reset_type) {\n-\tcase HARD:\n-\t\tif (!update_ref_status && !quiet)\n-\t\t\tprint_new_head_line(commit);\n-\t\tbreak;\n-\tcase SOFT: /* Nothing else to do. */\n-\t\tbreak;\n-\tcase MIXED: /* Report what has not been updated. */\n+\tif (reset_type == HARD && !update_ref_status && !quiet)\n+\t\tprint_new_head_line(commit);\n+\telse if (reset_type == MIXED) /* Report what has not been updated. */\n \t\tupdate_index_refresh(0, NULL,\n \t\t\t\tquiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);\n-\t\tbreak;\n-\t}\n \n \tremove_branch_state();\n \n-- \n1.8.1.rc3.331.g1ef2165\n"},{"id":"206362","messageId":"1357719376-16406-11-git-send-email-martinvonz@gmail.com","threadId":"32568","inReplyTo":"1357719376-16406-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH 10/19] reset --keep: only write index file once","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-09T08:16:07Z","receivedAt":"2013-01-09T08:16:07Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"\"git reset --keep\" calls reset_index_file() twice, first doing a\ntwo-way merge to the target revision, updating the index and worktree,\nand then resetting the index. After each call, we write the index\nfile.\n\nIn the unlikely event that the second call to reset_index_file()\nfails, the index will have been merged to the target revision, but\nHEAD will not be updated, leaving the user with a dirty index.\n\nBy moving the locking, writing and committing out of\nreset_index_file() and into the caller, we can avoid writing the index\ntwice, thereby making the sure we don't end up in the half-way reset\nstate. As a bonus, we speed up \"git reset --keep\" a little on the\nlinux-2.6 repo (best of five, warm cache):\n\n Before After\nreal 0m0.315s 0m0.296s\nuser 0m0.290s 0m0.280s\nsys 0m0.020s 0m0.010s\n---\n builtin/reset.c | 21 ++++++++++-----------\n 1 file changed, 10 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 05ccfd4..8e5d097 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -38,14 +38,12 @@ static inline int is_merge(void)\n \treturn !access(git_path(\"MERGE_HEAD\"), F_OK);\n }\n \n-static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet)\n+static int reset_index(const unsigned char *sha1, int reset_type, int quiet)\n {\n \tint nr = 1;\n-\tint newfd;\n \tstruct tree_desc desc[2];\n \tstruct tree *tree;\n \tstruct unpack_trees_options opts;\n-\tstruct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n \n \tmemset(&opts, 0, sizeof(opts));\n \topts.head_idx = 1;\n@@ -67,8 +65,6 @@ static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet\n \t\topts.reset = 1;\n \t}\n \n-\tnewfd = hold_locked_index(lock, 1);\n-\n \tread_cache_unmerged();\n \n \tif (reset_type == KEEP) {\n@@ -91,10 +87,6 @@ static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet\n \t\tprime_cache_tree(&active_cache_tree, tree);\n \t}\n \n-\tif (write_cache(newfd, active_cache, active_nr) ||\n-\t commit_locked_index(lock))\n-\t\treturn error(_(\"Could not write new index file.\"));\n-\n \treturn 0;\n }\n \n@@ -340,9 +332,16 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\tdie_if_unmerged_cache(reset_type);\n \n \tif (reset_type != SOFT) {\n-\t\tint err = reset_index_file(sha1, reset_type, quiet);\n+\t\tstruct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n+\t\tint newfd = hold_locked_index(lock, 1);\n+\t\tint err = reset_index(sha1, reset_type, quiet);\n \t\tif (reset_type == KEEP && !err)\n-\t\t\terr = reset_index_file(sha1, MIXED, quiet);\n+\t\t\terr = reset_index(sha1, MIXED, quiet);\n+\t\tif (!err &&\n+\t\t (write_cache(newfd, active_cache, active_nr) ||\n+\t\t commit_locked_index(lock))) {\n+\t\t\terr = error(_(\"Could not write new index file.\"));\n+\t\t}\n \t\tif (err)\n \t\t\tdie(_(\"Could not reset index file to revision '%s'.\"), rev);\n \t}\n-- \n1.8.1.rc3.331.g1ef2165\n"},{"id":"206372","messageId":"1357719376-16406-12-git-send-email-martinvonz@gmail.com","threadId":"32568","inReplyTo":"1357719376-16406-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH 11/19] reset: avoid redundant error message","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-09T08:16:08Z","receivedAt":"2013-01-09T08:16:08Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"If writing or committing the new index file fails, we print \"Could not\nwrite new index file.\" followed by \"Could not reset index file to\nrevision $rev.\". The first message seems to imply the second, so print\nonly the first message.\n---\n builtin/reset.c | 8 +++-----\n 1 file changed, 3 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 8e5d097..54e3c5b 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -337,13 +337,11 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\tint err = reset_index(sha1, reset_type, quiet);\n \t\tif (reset_type == KEEP && !err)\n \t\t\terr = reset_index(sha1, MIXED, quiet);\n-\t\tif (!err &&\n-\t\t (write_cache(newfd, active_cache, active_nr) ||\n-\t\t commit_locked_index(lock))) {\n-\t\t\terr = error(_(\"Could not write new index file.\"));\n-\t\t}\n \t\tif (err)\n \t\t\tdie(_(\"Could not reset index file to revision '%s'.\"), rev);\n+\t\tif (write_cache(newfd, active_cache, active_nr) ||\n+\t\t commit_locked_index(lock))\n+\t\t\tdie(_(\"Could not write new index file.\"));\n \t}\n \n \t/* Any resets update HEAD to the head being switched to,\n-- \n1.8.1.rc3.331.g1ef2165\n"},{"id":"206374","messageId":"1357719376-16406-13-git-send-email-martinvonz@gmail.com","threadId":"32568","inReplyTo":"1357719376-16406-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH 12/19] reset.c: move update_index_refresh() call out of read_from_tree()","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-09T08:16:09Z","receivedAt":"2013-01-09T08:16:09Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"The final part of cmd_reset() essentially looks like:\n\n if (pathspec) {\n ...\n read_from_tree(...);\n } else {\n ...\n reset_index(...);\n update_index_refresh(...);\n ...\n }\n\nwhere read_from_tree() internally also calls\nupdate_index_refresh(). Move the call to update_index_refresh() out of\nread_from_tree for symmetry with the 'else' block, making\nread_from_tree() and reset_index() closer in functionality.\n---\n builtin/reset.c | 18 +++++++++---------\n 1 file changed, 9 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 54e3c5b..a21ba31 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -145,11 +145,8 @@ static void update_index_from_diff(struct diff_queue_struct *q,\n \t}\n }\n \n-static int read_from_tree(const char **pathspec, unsigned char *tree_sha1,\n-\t\t\t int refresh_flags)\n+static int read_from_tree(const char **pathspec, unsigned char *tree_sha1)\n {\n-\tstruct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n-\tint index_fd;\n \tstruct diff_options opt;\n \n \tmemset(&opt, 0, sizeof(opt));\n@@ -157,7 +154,6 @@ static int read_from_tree(const char **pathspec, unsigned char *tree_sha1,\n \topt.output_format = DIFF_FORMAT_CALLBACK;\n \topt.format_callback = update_index_from_diff;\n \n-\tindex_fd = hold_locked_index(lock, 1);\n \tread_cache();\n \tif (do_diff_cache(tree_sha1, &opt))\n \t\treturn 1;\n@@ -165,7 +161,7 @@ static int read_from_tree(const char **pathspec, unsigned char *tree_sha1,\n \tdiff_flush(&opt);\n \tdiff_tree_release_paths(&opt);\n \n-\treturn update_index_refresh(index_fd, lock, refresh_flags);\n+\treturn 0;\n }\n \n static void set_reflog_message(struct strbuf *sb, const char *action,\n@@ -321,9 +317,13 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\tdie(_(\"%s reset is not allowed in a bare repository\"),\n \t\t _(reset_type_names[reset_type]));\n \n-\tif (pathspec)\n-\t\treturn read_from_tree(pathspec, sha1,\n-\t\t\t\tquiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);\n+\tif (pathspec) {\n+\t\tstruct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n+\t\tint index_fd = hold_locked_index(lock, 1);\n+\t\treturn read_from_tree(pathspec, sha1) ||\n+\t\t\tupdate_index_refresh(index_fd, lock,\n+\t\t\t\t\tquiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);\n+\t}\n \n \t/* Soft reset does not touch the index file nor the working tree\n \t * at all, but requires them in a good order. Other resets reset\n-- \n1.8.1.rc3.331.g1ef2165\n"},{"id":"206364","messageId":"1357719376-16406-14-git-send-email-martinvonz@gmail.com","threadId":"32568","inReplyTo":"1357719376-16406-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH 13/19] reset.c: move lock, write and commit out of update_index_refresh()","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-09T08:16:10Z","receivedAt":"2013-01-09T08:16:10Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"In preparation for the/a following patch, move the locking, writing\nand committing of the index file out of update_index_refresh(). The\ncode duplication caused will soon be taken care of. What remains of\nupdate_index_refresh() is just one line, but it is still called from\ntwo places, so let's leave it for now.\n\nIn the process, we expose and fix the minor UI bug that makes us print\n\"Could not refresh index\" when we fail to write the index file when\ninvoked with a pathspec. Copy the error message from the pathspec-less\ncodepath (\"Could not write new index file.\").\n---\n builtin/reset.c | 34 ++++++++++++++++++----------------\n 1 file changed, 18 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex a21ba31..2243b95 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -109,19 +109,10 @@ static void print_new_head_line(struct commit *commit)\n \t\tprintf(\"\\n\");\n }\n \n-static int update_index_refresh(int fd, struct lock_file *index_lock, int flags)\n+static void update_index_refresh(int flags)\n {\n-\tif (!index_lock) {\n-\t\tindex_lock = xcalloc(1, sizeof(struct lock_file));\n-\t\tfd = hold_locked_index(index_lock, 1);\n-\t}\n-\n \trefresh_index(&the_index, (flags), NULL, NULL,\n \t\t _(\"Unstaged changes after reset:\"));\n-\tif (write_cache(fd, active_cache, active_nr) ||\n-\t\t\tcommit_locked_index(index_lock))\n-\t\treturn error (\"Could not refresh index\");\n-\treturn 0;\n }\n \n static void update_index_from_diff(struct diff_queue_struct *q,\n@@ -320,9 +311,14 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \tif (pathspec) {\n \t\tstruct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n \t\tint index_fd = hold_locked_index(lock, 1);\n-\t\treturn read_from_tree(pathspec, sha1) ||\n-\t\t\tupdate_index_refresh(index_fd, lock,\n-\t\t\t\t\tquiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);\n+\t\tif (read_from_tree(pathspec, sha1))\n+\t\t\treturn 1;\n+\t\tupdate_index_refresh(\n+\t\t\tquiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);\n+\t\tif (write_cache(index_fd, active_cache, active_nr) ||\n+\t\t commit_locked_index(lock))\n+\t\t\treturn error(\"Could not write new index file.\");\n+\t\treturn 0;\n \t}\n \n \t/* Soft reset does not touch the index file nor the working tree\n@@ -350,9 +346,15 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \n \tif (reset_type == HARD && !update_ref_status && !quiet)\n \t\tprint_new_head_line(commit);\n-\telse if (reset_type == MIXED) /* Report what has not been updated. */\n-\t\tupdate_index_refresh(0, NULL,\n-\t\t\t\tquiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);\n+\telse if (reset_type == MIXED) { /* Report what has not been updated. */\n+\t\tstruct lock_file *index_lock = xcalloc(1, sizeof(struct lock_file));\n+\t\tint fd = hold_locked_index(index_lock, 1);\n+\t\tupdate_index_refresh(\n+\t\t\tquiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);\n+\t\tif (write_cache(fd, active_cache, active_nr) ||\n+\t\t commit_locked_index(index_lock))\n+\t\t\terror(\"Could not refresh index\");\n+\t}\n \n \tremove_branch_state();\n \n-- \n1.8.1.rc3.331.g1ef2165\n"},{"id":"206356","messageId":"1357719376-16406-15-git-send-email-martinvonz@gmail.com","threadId":"32568","inReplyTo":"1357719376-16406-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH 14/19] reset [--mixed]: don't write index file twice","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-09T08:16:11Z","receivedAt":"2013-01-09T08:16:11Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"When doing a mixed reset without paths, the index is locked, read,\nreset, and written back as part of the actual reset operation (in\nreset_index()). Then, when showing the list of worktree modifications,\nwe lock the index again, refresh it, and write it.\n\nChange this so we only write the index once, making \"git reset\" a\nlittle faster. It does mean that the index lock will be held a little\nlonger, but the difference is small compared to the time spent\nrefreshing the index.\n\nThere is one minor functional difference: We used to say \"Could not\nwrite new index file.\" if the first write failed, and \"Could not\nrefresh index\" if the second write failed. Now, we will only use the\nfirst message.\n\nThis speeds up \"git reset\" a little on the linux-2.6 repo (best of\nfive, warm cache):\n\n Before After\nreal 0m0.239s 0m0.214s\nuser 0m0.160s 0m0.130s\nsys 0m0.070s 0m0.080s\n---\n builtin/reset.c | 14 +++++---------\n 1 file changed, 5 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 2243b95..254afa9 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -335,6 +335,11 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t\terr = reset_index(sha1, MIXED, quiet);\n \t\tif (err)\n \t\t\tdie(_(\"Could not reset index file to revision '%s'.\"), rev);\n+\n+\t\tif (reset_type == MIXED) /* Report what has not been updated. */\n+\t\t\tupdate_index_refresh(\n+\t\t\t\tquiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);\n+\n \t\tif (write_cache(newfd, active_cache, active_nr) ||\n \t\t commit_locked_index(lock))\n \t\t\tdie(_(\"Could not write new index file.\"));\n@@ -346,15 +351,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \n \tif (reset_type == HARD && !update_ref_status && !quiet)\n \t\tprint_new_head_line(commit);\n-\telse if (reset_type == MIXED) { /* Report what has not been updated. */\n-\t\tstruct lock_file *index_lock = xcalloc(1, sizeof(struct lock_file));\n-\t\tint fd = hold_locked_index(index_lock, 1);\n-\t\tupdate_index_refresh(\n-\t\t\tquiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);\n-\t\tif (write_cache(fd, active_cache, active_nr) ||\n-\t\t commit_locked_index(index_lock))\n-\t\t\terror(\"Could not refresh index\");\n-\t}\n \n \tremove_branch_state();\n \n-- \n1.8.1.rc3.331.g1ef2165\n"},{"id":"206358","messageId":"1357719376-16406-16-git-send-email-martinvonz@gmail.com","threadId":"32568","inReplyTo":"1357719376-16406-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH 15/19] reset.c: finish entire cmd_reset() whether or not pathspec is given","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-09T08:16:12Z","receivedAt":"2013-01-09T08:16:12Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"By not returning from inside the \"if (pathspec)\" block, we can let the\npathspec-aware and pathspec-less code share a bit more, making it\neasier to make future changes that should affect both cases. This also\nhighlights the similarity between read_from_tree() and reset_index().\n---\nShould error reporting be aligned too? Speaking of which,\ndo_diff_cache() never returns anything by 0. Is the return value for\nfuture-proofing?\n\n builtin/reset.c | 42 ++++++++++++++++++------------------------\n 1 file changed, 18 insertions(+), 24 deletions(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 254afa9..9bcad29 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -308,19 +308,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\tdie(_(\"%s reset is not allowed in a bare repository\"),\n \t\t _(reset_type_names[reset_type]));\n \n-\tif (pathspec) {\n-\t\tstruct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n-\t\tint index_fd = hold_locked_index(lock, 1);\n-\t\tif (read_from_tree(pathspec, sha1))\n-\t\t\treturn 1;\n-\t\tupdate_index_refresh(\n-\t\t\tquiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);\n-\t\tif (write_cache(index_fd, active_cache, active_nr) ||\n-\t\t commit_locked_index(lock))\n-\t\t\treturn error(\"Could not write new index file.\");\n-\t\treturn 0;\n-\t}\n-\n \t/* Soft reset does not touch the index file nor the working tree\n \t * at all, but requires them in a good order. Other resets reset\n \t * the index file to the tree object we are switching to. */\n@@ -330,11 +317,16 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \tif (reset_type != SOFT) {\n \t\tstruct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n \t\tint newfd = hold_locked_index(lock, 1);\n-\t\tint err = reset_index(sha1, reset_type, quiet);\n-\t\tif (reset_type == KEEP && !err)\n-\t\t\terr = reset_index(sha1, MIXED, quiet);\n-\t\tif (err)\n-\t\t\tdie(_(\"Could not reset index file to revision '%s'.\"), rev);\n+\t\tif (pathspec) {\n+\t\t\tif (read_from_tree(pathspec, sha1))\n+\t\t\t\treturn 1;\n+\t\t} else {\n+\t\t\tint err = reset_index(sha1, reset_type, quiet);\n+\t\t\tif (reset_type == KEEP && !err)\n+\t\t\t\terr = reset_index(sha1, MIXED, quiet);\n+\t\t\tif (err)\n+\t\t\t\tdie(_(\"Could not reset index file to revision '%s'.\"), rev);\n+\t\t}\n \n \t\tif (reset_type == MIXED) /* Report what has not been updated. */\n \t\t\tupdate_index_refresh(\n@@ -345,14 +337,16 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t\tdie(_(\"Could not write new index file.\"));\n \t}\n \n-\t/* Any resets update HEAD to the head being switched to,\n-\t * saving the previous head in ORIG_HEAD before. */\n-\tupdate_ref_status = update_refs(rev, sha1);\n+\tif (!pathspec) {\n+\t\t/* Any resets without paths update HEAD to the head being\n+\t\t * switched to, saving the previous head in ORIG_HEAD before. */\n+\t\tupdate_ref_status = update_refs(rev, sha1);\n \n-\tif (reset_type == HARD && !update_ref_status && !quiet)\n-\t\tprint_new_head_line(commit);\n+\t\tif (reset_type == HARD && !update_ref_status && !quiet)\n+\t\t\tprint_new_head_line(commit);\n \n-\tremove_branch_state();\n+\t\tremove_branch_state();\n+\t}\n \n \treturn update_ref_status;\n }\n-- \n1.8.1.rc3.331.g1ef2165\n"},{"id":"206368","messageId":"1357719376-16406-17-git-send-email-martinvonz@gmail.com","threadId":"32568","inReplyTo":"1357719376-16406-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH 16/19] reset [--mixed] --quiet: don't refresh index","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-09T08:16:13Z","receivedAt":"2013-01-09T08:16:13Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"\"git reset [--mixed]\" without --quiet refreshes the index in order to\ndisplay the \"Unstaged changes after reset\". When --quiet is given,\nthat output is suppressed, removing the need to refresh the index.\nOther porcelain commands that care about a refreshed index should\nalready be refreshing it, so running e.g. \"git reset -q && git diff\"\nis still safe.\n\nThis commit together with 686b2de (oneway_merge(): only lstat() when\ntold to update worktree, 2012-12-20) removes all calls to lstat() the\nworktree from the command.\n\nThis speeds up \"git reset -q\" a little on the linux-2.6 repo (best\nof five, warm cache):\n\n Before After\nreal 0m0.215s 0m0.176s\nuser 0m0.150s 0m0.130s\nsys 0m0.060s 0m0.040s\n\nAnd with cold cache (best of five):\n\n Before After\nreal 0m11.351s 0m8.420s\nuser 0m0.230s 0m0.220s\nsys 0m0.270s 0m0.060s\n---\nThere is a test case in t7102 called '--mixed refreshes the index',\nbut it only checks that right output it printed. Is the test case not\ntesting right or not named right? As you can see, I suspect it's the\nname/description that should change.\n\n builtin/reset.c | 12 +++---------\n 1 file changed, 3 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 9bcad29..a2e69eb 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -109,12 +109,6 @@ static void print_new_head_line(struct commit *commit)\n \t\tprintf(\"\\n\");\n }\n \n-static void update_index_refresh(int flags)\n-{\n-\trefresh_index(&the_index, (flags), NULL, NULL,\n-\t\t _(\"Unstaged changes after reset:\"));\n-}\n-\n static void update_index_from_diff(struct diff_queue_struct *q,\n \t\tstruct diff_options *opt, void *data)\n {\n@@ -328,9 +322,9 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t\t\tdie(_(\"Could not reset index file to revision '%s'.\"), rev);\n \t\t}\n \n-\t\tif (reset_type == MIXED) /* Report what has not been updated. */\n-\t\t\tupdate_index_refresh(\n-\t\t\t\tquiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);\n+\t\tif (reset_type == MIXED && !quiet) /* Report what has not been updated. */\n+\t\t\trefresh_index(&the_index, REFRESH_IN_PORCELAIN, NULL, NULL,\n+\t\t\t\t _(\"Unstaged changes after reset:\"));\n \n \t\tif (write_cache(newfd, active_cache, active_nr) ||\n \t\t commit_locked_index(lock))\n-- \n1.8.1.rc3.331.g1ef2165\n"},{"id":"206366","messageId":"1357719376-16406-18-git-send-email-martinvonz@gmail.com","threadId":"32568","inReplyTo":"1357719376-16406-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH 17/19] reset $sha1 $pathspec: require $sha1 only to be treeish","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-09T08:16:14Z","receivedAt":"2013-01-09T08:16:14Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"Resetting with paths does not update HEAD and there is nothing else\nthat a commit should be needed for. Relax the argument parsing so only\na tree is required.\n\nThe sha1 is only passed to read_from_tree(), which already only\nrequires a tree.\n\nThe \"rev\" variable we pass to run_add_interactive() will resolve to a\ntree. This is fine since interactive_reset only needs the parameter to\nbe a treeish and doesn't use it for display purposes.\n---\nIs it correct that interactive_reset does not use the revision\nspecifier for display purposes? Or, worse, that it requires it to be a\ncommit in some cases? I tried it and didn't see any problem.\n\nCan the two blocks of code that look up commit or tree be made to\nshare more? I'm not very familiar with what functions are available. I\nthink I tried keeping a separate \"struct object *object\" to be able to\nput the last three lines outside the blocks, but didn't like the\nresult.\n\n builtin/reset.c | 46 ++++++++++++++++++++++++++--------------------\n t/t7102-reset.sh | 8 ++++++++\n 2 files changed, 34 insertions(+), 20 deletions(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex a2e69eb..4c223bd 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -177,9 +177,10 @@ const char **parse_args(int argc, const char **argv, const char *prefix, const c\n \t/*\n \t * Possible arguments are:\n \t *\n-\t * git reset [-opts] ...\n-\t * git reset [-opts] -- ...\n-\t * git reset [-opts] -- ...\n+\t * git reset [-opts] []\n+\t * git reset [-opts] [...]\n+\t * git reset [-opts] -- [...]\n+\t * git reset [-opts] -- [...]\n \t * git reset [-opts] ...\n \t *\n \t * At this point, argv points immediately after [-opts].\n@@ -194,11 +195,13 @@ const char **parse_args(int argc, const char **argv, const char *prefix, const c\n \t\t}\n \t\t/*\n \t\t * Otherwise, argv[0] could be either or and\n-\t\t * has to be unambiguous.\n+\t\t * has to be unambiguous. If there is a single argument, it\n+\t\t * can not be a tree\n \t\t */\n-\t\telse if (!get_sha1_committish(argv[0], unused)) {\n+\t\telse if ((argc == 1 && !get_sha1_committish(argv[0], unused)) ||\n+\t\t\t (argc > 1 && !get_sha1_treeish(argv[0], unused))) {\n \t\t\t/*\n-\t\t\t * Ok, argv[0] looks like a rev; it should not\n+\t\t\t * Ok, argv[0] looks like a commit/tree; it should not\n \t\t\t * be a filename.\n \t\t\t */\n \t\t\tverify_non_filename(prefix, argv[0]);\n@@ -240,7 +243,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \tconst char *rev;\n \tunsigned char sha1[20];\n \tconst char **pathspec = NULL;\n-\tstruct commit *commit;\n+\tstruct commit *commit = NULL;\n \tconst struct option options[] = {\n \t\tOPT__QUIET(&quiet, N_(\"be quiet, only report errors\")),\n \t\tOPT_SET_INT(0, \"mixed\", &reset_type,\n@@ -262,19 +265,22 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t\tPARSE_OPT_KEEP_DASHDASH);\n \tpathspec = parse_args(argc, argv, prefix, &rev);\n \n-\tif (get_sha1_committish(rev, sha1))\n-\t\tdie(_(\"Failed to resolve '%s' as a valid ref.\"), rev);\n-\n-\t/*\n-\t * NOTE: As \"git reset $treeish -- $path\" should be usable on\n-\t * any tree-ish, this is not strictly correct. We are not\n-\t * moving the HEAD to any commit; we are merely resetting the\n-\t * entries in the index to that of a treeish.\n-\t */\n-\tcommit = lookup_commit_reference(sha1);\n-\tif (!commit)\n-\t\tdie(_(\"Could not parse object '%s'.\"), rev);\n-\thashcpy(sha1, commit->object.sha1);\n+\tif (!pathspec) {\n+\t\tif (get_sha1_committish(rev, sha1))\n+\t\t\tdie(_(\"Failed to resolve '%s' as a valid revision.\"), rev);\n+\t\tcommit = lookup_commit_reference(sha1);\n+\t\tif (!commit)\n+\t\t\tdie(_(\"Could not parse object '%s'.\"), rev);\n+\t\thashcpy(sha1, commit->object.sha1);\n+\t} else {\n+\t\tstruct tree *tree;\n+\t\tif (get_sha1_treeish(rev, sha1))\n+\t\t\tdie(_(\"Failed to resolve '%s' as a valid tree.\"), rev);\n+\t\ttree = parse_tree_indirect(sha1);\n+\t\tif (!tree)\n+\t\t\tdie(_(\"Could not parse object '%s'.\"), rev);\n+\t\thashcpy(sha1, tree->object.sha1);\n+\t}\n \n \tif (patch_mode) {\n \t\tif (reset_type != NONE)\ndiff --git a/t/t7102-reset.sh b/t/t7102-reset.sh\nindex 81b2570..1fa2a5f 100755\n--- a/t/t7102-reset.sh\n+++ b/t/t7102-reset.sh\n@@ -497,4 +497,12 @@ test_expect_success 'disambiguation (4)' '\n \ttest ! -f secondfile\n '\n \n+test_expect_success 'reset with paths accepts tree' '\n+\t# for simpler tests, drop last commit containing added files\n+\tgit reset --hard HEAD^ &&\n+\tgit reset HEAD^^{tree} -- . &&\n+\tgit diff --cached HEAD^ --exit-code &&\n+\tgit diff HEAD --exit-code\n+'\n+\n test_done\n-- \n1.8.1.rc3.331.g1ef2165\n"},{"id":"206373","messageId":"1357719376-16406-19-git-send-email-martinvonz@gmail.com","threadId":"32568","inReplyTo":"1357719376-16406-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH 18/19] reset: allow reset on unborn branch","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-09T08:16:15Z","receivedAt":"2013-01-09T08:16:15Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"Some users seem to think, knowingly or not, that being on an unborn\nbranch is like having a commit with an empty tree checked out, but\nwhen run on an unborn branch, \"git reset\" currently fails with:\n\n fatal: Failed to resolve 'HEAD' as a valid ref.\n\nInstead of making users figure out that they should run\n\n git rm --cached -r .\n\n, let's teach \"git reset\" without a revision argument, when on an\nunborn branch, to behave as if the user asked to reset to an empty\ntree. Don't take the analogy with an empty commit too far, though, but\nstill disallow explictly referring to HEAD in \"git reset HEAD\".\n---\n builtin/reset.c | 17 ++++++++------\n t/t7106-reset-unborn-branch.sh | 52 ++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 62 insertions(+), 7 deletions(-)\n create mode 100755 t/t7106-reset-unborn-branch.sh\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 4c223bd..5cd48ac 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -239,7 +239,7 @@ static int update_refs(const char *rev, const unsigned char *sha1) {\n int cmd_reset(int argc, const char **argv, const char *prefix)\n {\n \tint reset_type = NONE, update_ref_status = 0, quiet = 0;\n-\tint patch_mode = 0;\n+\tint patch_mode = 0, unborn;\n \tconst char *rev;\n \tunsigned char sha1[20];\n \tconst char **pathspec = NULL;\n@@ -264,8 +264,11 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \targc = parse_options(argc, argv, prefix, options, git_reset_usage,\n \t\t\t\t\t\tPARSE_OPT_KEEP_DASHDASH);\n \tpathspec = parse_args(argc, argv, prefix, &rev);\n-\n-\tif (!pathspec) {\n+\tunborn = !strcmp(rev, \"HEAD\") && get_sha1(\"HEAD\", sha1);\n+\tif (unborn) {\n+\t\t/* reset on unborn branch: treat as reset to empty tree */\n+\t\thashcpy(sha1, EMPTY_TREE_SHA1_BIN);\n+\t} else if (!pathspec) {\n \t\tif (get_sha1_committish(rev, sha1))\n \t\t\tdie(_(\"Failed to resolve '%s' as a valid revision.\"), rev);\n \t\tcommit = lookup_commit_reference(sha1);\n@@ -285,7 +288,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \tif (patch_mode) {\n \t\tif (reset_type != NONE)\n \t\t\tdie(_(\"--patch is incompatible with --{hard,mixed,soft}\"));\n-\t\treturn run_add_interactive(rev, \"--patch=reset\", pathspec);\n+\t\treturn run_add_interactive(sha1_to_hex(sha1), \"--patch=reset\", pathspec);\n \t}\n \n \t/* git reset tree [--] paths... can be used to\n@@ -337,16 +340,16 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t\tdie(_(\"Could not write new index file.\"));\n \t}\n \n-\tif (!pathspec) {\n+\tif (!pathspec && !unborn) {\n \t\t/* Any resets without paths update HEAD to the head being\n \t\t * switched to, saving the previous head in ORIG_HEAD before. */\n \t\tupdate_ref_status = update_refs(rev, sha1);\n \n \t\tif (reset_type == HARD && !update_ref_status && !quiet)\n \t\t\tprint_new_head_line(commit);\n-\n-\t\tremove_branch_state();\n \t}\n+\tif (!pathspec)\n+\t\tremove_branch_state();\n \n \treturn update_ref_status;\n }\ndiff --git a/t/t7106-reset-unborn-branch.sh b/t/t7106-reset-unborn-branch.sh\nnew file mode 100755\nindex 0000000..4ff6af4\n--- /dev/null\n+++ b/t/t7106-reset-unborn-branch.sh\n@@ -0,0 +1,52 @@\n+#!/bin/sh\n+\n+test_description='git reset should work on unborn branch'\n+. ./test-lib.sh\n+\n+test_expect_success 'setup' '\n+\techo a >a &&\n+\techo b >b\n+'\n+\n+test_expect_success 'reset' '\n+\tgit add a b &&\n+\tgit reset &&\n+\ttest \"$(git ls-files)\" == \"\"\n+'\n+\n+test_expect_success 'reset HEAD' '\n+\trm .git/index &&\n+\tgit add a b &&\n+\ttest_must_fail git reset HEAD\n+'\n+\n+test_expect_success 'reset $file' '\n+\trm .git/index &&\n+\tgit add a b &&\n+\tgit reset a &&\n+\ttest \"$(git ls-files)\" == \"b\"\n+'\n+\n+test_expect_success 'reset -p' '\n+\trm .git/index &&\n+\tgit add a &&\n+\techo y | git reset -p &&\n+\ttest \"$(git ls-files)\" == \"\"\n+'\n+\n+test_expect_success 'reset --soft is a no-op' '\n+\trm .git/index &&\n+\tgit add a &&\n+\tgit reset --soft\n+\ttest \"$(git ls-files)\" == \"a\"\n+'\n+\n+test_expect_success 'reset --hard' '\n+\trm .git/index &&\n+\tgit add a &&\n+\tgit reset --hard &&\n+\ttest \"$(git ls-files)\" == \"\" &&\n+\ttest_path_is_missing a\n+'\n+\n+test_done\n-- \n1.8.1.rc3.331.g1ef2165\n"},{"id":"206359","messageId":"1357719376-16406-20-git-send-email-martinvonz@gmail.com","threadId":"32568","inReplyTo":"1357719376-16406-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH 19/19] reset [--mixed]: use diff-based reset whether or not pathspec was given","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-09T08:16:16Z","receivedAt":"2013-01-09T08:16:16Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"Thanks to b65982b (Optimize \"diff-index --cached\" using cache-tree,\n2009-05-20), resetting with paths is much faster than resetting\nwithout paths. Some timings for the linux-2.6 repo to illustrate this\n(best of five, warm cache):\n\n reset reset .\nreal 0m0.219s 0m0.080s\nuser 0m0.140s 0m0.040s\nsys 0m0.070s 0m0.030s\n\nThese two commands should do the same thing, so instead of having the\nuser type the trailing \" .\" to get the faster do_diff_cache()-based\nimplementation, always use it when doing a mixed reset, with or\nwithout paths (so \"git reset $rev\" would also be faster).\n\nComparing before and after (best of five):\n\n Before After\nreset (warm cache): 0.21 0.07\nreset -q (warm cache) 0.17 0.03\nreset (cold cache): 10.31 2.72\nreset -q (cold cache) 7.64 0.38\n---\nAre unmerged entries handled the same? read_from_tree() calls\nread_cache(), while reset_index() calls read_cache_unmerged(). I\nhaven't figured out if/why they should be different.\n\nAre there other differences, or could unpack_trees() learn the same\noptimization as do_diff_cache()? Actually, the commit mentioned above\ndoes say\n\n Tweak unpack_trees() logic that is used to read in the tree object\n to catch the case where the tree entry we are looking at matches the\n index as a whole by looking at the cache-tree.\n\nIf there are differences, we are clearly missing tests for them. And\nit seems like any difference between them should be fixed, so \"git\nreset\" and \"git reset .\" (from root of tree) do the same thing even\nbefore this patch.\n\n builtin/reset.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 5cd48ac..6db0a10 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -320,7 +320,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \tif (reset_type != SOFT) {\n \t\tstruct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n \t\tint newfd = hold_locked_index(lock, 1);\n-\t\tif (pathspec) {\n+\t\tif (reset_type == MIXED) {\n \t\t\tif (read_from_tree(pathspec, sha1))\n \t\t\t\treturn 1;\n \t\t} else {\n-- \n1.8.1.rc3.331.g1ef2165\n"},{"id":"206377","messageId":"20130109114234.GA3528@ftbfs.org","threadId":"32568","inReplyTo":"1357719376-16406-4-git-send-email-martinvonz@gmail.com","subject":"Re: [PATCH 03/19] reset.c: pass pathspec around instead of (prefix, argv) pair","fromName":"Matt Kraai","fromEmail":"kraai@ftbfs.org","sentAt":"2013-01-09T11:42:34Z","receivedAt":"2013-01-09T11:42:34Z","isPatch":true,"sender":{"key":"kraai@ftbfs.org","avatar":null},"body":"On Wed, Jan 09, 2013 at 12:16:00AM -0800, Martin von Zweigbergk wrote:\n> We use the path arguments in two places in reset.c: in\n> interactive_reset() and read_from_tree(). Both of these call\n> get_pathspec(), so we pass the (prefix, arv) pair to both\n ^^^\nargv is misspelled.\n\n-- \nMatt\n"},{"id":"206378","messageId":"20130109115434.GB3528@ftbfs.org","threadId":"32568","inReplyTo":"1357719376-16406-8-git-send-email-martinvonz@gmail.com","subject":"Re: [PATCH 07/19] reset.c: extract function for updating {ORIG,}HEAD","fromName":"Matt Kraai","fromEmail":"kraai@ftbfs.org","sentAt":"2013-01-09T11:54:35Z","receivedAt":"2013-01-09T11:54:35Z","isPatch":true,"sender":{"key":"kraai@ftbfs.org","avatar":null},"body":"In the summary, {ORIG,} should be {ORIG_,}.\n\n-- \nMatt\n"},{"id":"206390","messageId":"20130109170119.GA5332@sigill.intra.peff.net","threadId":"32568","inReplyTo":"1357719376-16406-17-git-send-email-martinvonz@gmail.com","subject":"Re: [PATCH 16/19] reset [--mixed] --quiet: don't refresh index","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-01-09T17:01:19Z","receivedAt":"2013-01-09T17:01:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 09, 2013 at 12:16:13AM -0800, Martin von Zweigbergk wrote:\n\n> \"git reset [--mixed]\" without --quiet refreshes the index in order to\n> display the \"Unstaged changes after reset\". When --quiet is given,\n> that output is suppressed, removing the need to refresh the index.\n> Other porcelain commands that care about a refreshed index should\n> already be refreshing it, so running e.g. \"git reset -q && git diff\"\n> is still safe.\n\nHmm. But \"git reset -q && git diff-files\" would not be?\n\nWe have never been very clear about which commands refresh the index.\nSince \"reset\" is about manipulating the index, I'd expect it to be\nrefreshed afterwards. On the other hand, since we have never guaranteed\nanything, perhaps a careful script should always use \"git update-index\n--refresh\". I would not be too surprised if some of our own scripts are\nnot that careful, though.\n\n-Peff\n"},{"id":"206400","messageId":"CANiSa6joBuAJVHkMfNbVMHFJ6BFOh7sGRw_txRO81CKudRwsfA@mail.gmail.com","threadId":"32568","inReplyTo":"20130109170119.GA5332@sigill.intra.peff.net","subject":"Re: [PATCH 16/19] reset [--mixed] --quiet: don't refresh index","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-09T18:43:20Z","receivedAt":"2013-01-09T18:43:20Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"On Wed, Jan 9, 2013 at 9:01 AM, Jeff King wrote:\n> On Wed, Jan 09, 2013 at 12:16:13AM -0800, Martin von Zweigbergk wrote:\n>\n>> \"git reset [--mixed]\" without --quiet refreshes the index in order to\n>> display the \"Unstaged changes after reset\". When --quiet is given,\n>> that output is suppressed, removing the need to refresh the index.\n>> Other porcelain commands that care about a refreshed index should\n>> already be refreshing it, so running e.g. \"git reset -q && git diff\"\n>> is still safe.\n>\n> Hmm. But \"git reset -q && git diff-files\" would not be?\n\nRight. Actually, \"git reset -q && git diff\" was perhaps not a good\nexample, because its analogous plumbing command would be \"git reset -q\n&& git diff-files -p\", which is also safe. But, as you say, \"git reset\n-q && git diff-files\" (without -p) might list files for which only the\nstat information has changed.\n\n> We have never been very clear about which commands refresh the index.\n\nYes, git-reset's documentation doesn't mention it.\n\n> Since \"reset\" is about manipulating the index, I'd expect it to be\n> refreshed afterwards. On the other hand, since we have never guaranteed\n> anything, perhaps a careful script should always use \"git update-index\n> --refresh\".\n\nSince \"git diff-files\" is a plumbing command, users of it to a\nhopefully a bit more careful than regular users, but you never know.\n\n> I would not be too surprised if some of our own scripts are\n> not that careful, though.\n\nI didn't find any, but I might have missed something.\n\nRegardless, this patch was tangential. The goal of this series can be\nachieved independently of this patch, so if it's too risky, we can\ndrop easily drop it.\n\nAlso, even though it does make \"git reset -q\" faster, I'm not sure how\nimportant that is in practice. Most use cases would probably refresh\nthe index afterwards anyway. In such cases, the improvement on warm\ncache would still be there, but the relative improvement in the cold\ncache case would be pretty much gone (since the entire tree would be\nstat'ed by the following refresh anyway).\n\n\nMartin\n"},{"id":"206401","messageId":"7v7gnm6uhm.fsf@alter.siamese.dyndns.org","threadId":"32568","inReplyTo":"CANiSa6joBuAJVHkMfNbVMHFJ6BFOh7sGRw_txRO81CKudRwsfA@mail.gmail.com","subject":"Re: [PATCH 16/19] reset [--mixed] --quiet: don't refresh index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-09T19:12:37Z","receivedAt":"2013-01-09T19:12:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk writes:\n\n>> We have never been very clear about which commands refresh the index.\n>\n> Yes, git-reset's documentation doesn't mention it.\n>\n>> Since \"reset\" is about manipulating the index, I'd expect it to be\n>> refreshed afterwards. On the other hand, since we have never guaranteed\n>> anything, perhaps a careful script should always use \"git update-index\n>> --refresh\".\n>\n> Since \"git diff-files\" is a plumbing command, users of it to a\n> hopefully a bit more careful than regular users, but you never know.\n>\n>> I would not be too surprised if some of our own scripts are\n>> not that careful, though.\n>\n> I didn't find any, but I might have missed something.\n\ncontrib/examples/ have some, but looking at it makes me realize that\nwe have been fairly careful to avoid using \"git reset\" which is a\nPorcelain.\n\nAnd as a Porcelain, I would rather expect it to leave the resulting\nindex refreshed.\n"},{"id":"206404","messageId":"7vy5g25f9b.fsf@alter.siamese.dyndns.org","threadId":"32568","inReplyTo":"1357719376-16406-4-git-send-email-martinvonz@gmail.com","subject":"Re: [PATCH 03/19] reset.c: pass pathspec around instead of (prefix, argv) pair","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-09T19:26:56Z","receivedAt":"2013-01-09T19:26:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk writes:\n\n> We use the path arguments in two places in reset.c: in\n> interactive_reset() and read_from_tree(). Both of these call\n> get_pathspec(), so we pass the (prefix, arv) pair to both\n> functions. Move the call to get_pathspec() out of these methods, for\n> two reasons: 1) One argument is simpler than two. 2) It lets us use\n> the (arguably clearer) \"if (pathspec)\" in place of \"if (i < argc)\".\n> ---\n> If I understand correctly, this should be rebased on top of\n> nd/parse-pathspec. Please let me know.\n\nYeah, this will conflict with the get_pathspec-to-parse_pathspec\nconversion Duy has been working on.\n\nWithout the interactions with that topic, the conversion seems\nstraightforward to me, though.\n\n>\n> builtin/reset.c | 27 ++++++++++-----------------\n> 1 file changed, 10 insertions(+), 17 deletions(-)\n>\n> diff --git a/builtin/reset.c b/builtin/reset.c\n> index 65413d0..045c960 100644\n> --- a/builtin/reset.c\n> +++ b/builtin/reset.c\n> @@ -153,26 +153,15 @@ static void update_index_from_diff(struct diff_queue_struct *q,\n> \t}\n> }\n> \n> -static int interactive_reset(const char *revision, const char **argv,\n> -\t\t\t const char *prefix)\n> -{\n> -\tconst char **pathspec = NULL;\n> -\n> -\tif (*argv)\n> -\t\tpathspec = get_pathspec(prefix, argv);\n> -\n> -\treturn run_add_interactive(revision, \"--patch=reset\", pathspec);\n> -}\n> -\n> -static int read_from_tree(const char *prefix, const char **argv,\n> -\t\tunsigned char *tree_sha1, int refresh_flags)\n> +static int read_from_tree(const char **pathspec, unsigned char *tree_sha1,\n> +\t\t\t int refresh_flags)\n> {\n> \tstruct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n> \tint index_fd;\n> \tstruct diff_options opt;\n> \n> \tmemset(&opt, 0, sizeof(opt));\n> -\tdiff_tree_setup_paths(get_pathspec(prefix, (const char **)argv), &opt);\n> +\tdiff_tree_setup_paths(pathspec, &opt);\n> \topt.output_format = DIFF_FORMAT_CALLBACK;\n> \topt.format_callback = update_index_from_diff;\n> \n> @@ -216,6 +205,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n> \tconst char *rev = \"HEAD\";\n> \tunsigned char sha1[20], *orig = NULL, sha1_orig[20],\n> \t\t\t\t*old_orig = NULL, sha1_old_orig[20];\n> +\tconst char **pathspec = NULL;\n> \tstruct commit *commit;\n> \tstruct strbuf msg = STRBUF_INIT;\n> \tconst struct option options[] = {\n> @@ -287,22 +277,25 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n> \t\tdie(_(\"Could not parse object '%s'.\"), rev);\n> \thashcpy(sha1, commit->object.sha1);\n> \n> +\tif (i < argc)\n> +\t\tpathspec = get_pathspec(prefix, argv + i);\n> +\n> \tif (patch_mode) {\n> \t\tif (reset_type != NONE)\n> \t\t\tdie(_(\"--patch is incompatible with --{hard,mixed,soft}\"));\n> -\t\treturn interactive_reset(rev, argv + i, prefix);\n> +\t\treturn run_add_interactive(rev, \"--patch=reset\", pathspec);\n> \t}\n> \n> \t/* git reset tree [--] paths... can be used to\n> \t * load chosen paths from the tree into the index without\n> \t * affecting the working tree nor HEAD. */\n> -\tif (i < argc) {\n> +\tif (pathspec) {\n> \t\tif (reset_type == MIXED)\n> \t\t\twarning(_(\"--mixed with paths is deprecated; use 'git reset -- ' instead.\"));\n> \t\telse if (reset_type != NONE)\n> \t\t\tdie(_(\"Cannot do %s reset with paths.\"),\n> \t\t\t\t\t_(reset_type_names[reset_type]));\n> -\t\treturn read_from_tree(prefix, argv + i, sha1,\n> +\t\treturn read_from_tree(pathspec, sha1,\n> \t\t\t\tquiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);\n> \t}\n> \tif (reset_type == NONE)\n"},{"id":"206405","messageId":"7vtxqq5f0g.fsf@alter.siamese.dyndns.org","threadId":"32568","inReplyTo":"1357719376-16406-5-git-send-email-martinvonz@gmail.com","subject":"Re: [PATCH 04/19] reset: don't allow \"git reset -- $pathspec\" in bare repo","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-09T19:32:15Z","receivedAt":"2013-01-09T19:32:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk writes:\n\n> ---\n> builtin/reset.c | 6 ++++--\n> 1 file changed, 4 insertions(+), 2 deletions(-)\n\nWith the patch that does not have any explicit check for bareness\nnor new error message to scold user with, it is rather hard to tell\nwhat is going on, without any description on what (if anything) is\nbroken at the end user level and what remedy is done about that\nbreakage...\n\n>\n> diff --git a/builtin/reset.c b/builtin/reset.c\n> index 045c960..664fad9 100644\n> --- a/builtin/reset.c\n> +++ b/builtin/reset.c\n> @@ -295,8 +295,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n> \t\telse if (reset_type != NONE)\n> \t\t\tdie(_(\"Cannot do %s reset with paths.\"),\n> \t\t\t\t\t_(reset_type_names[reset_type]));\n> -\t\treturn read_from_tree(pathspec, sha1,\n> -\t\t\t\tquiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);\n> \t}\n> \tif (reset_type == NONE)\n> \t\treset_type = MIXED; /* by default */\n> @@ -308,6 +306,10 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n> \t\tdie(_(\"%s reset is not allowed in a bare repository\"),\n> \t\t _(reset_type_names[reset_type]));\n> \n> +\tif (pathspec)\n> +\t\treturn read_from_tree(pathspec, sha1,\n> +\t\t\t\tquiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);\n> +\n> \t/* Soft reset does not touch the index file nor the working tree\n> \t * at all, but requires them in a good order. Other resets reset\n> \t * the index file to the tree object we are switching to. */\n"},{"id":"206407","messageId":"CANiSa6jm-6Jw1kwx3h=ct302YkuE7yntj6gV8ZDraG9gtX6CZw@mail.gmail.com","threadId":"32568","inReplyTo":"7v7gnm6uhm.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 16/19] reset [--mixed] --quiet: don't refresh index","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-09T19:38:59Z","receivedAt":"2013-01-09T19:38:59Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"On Wed, Jan 9, 2013 at 11:12 AM, Junio C Hamano wrote:\n> Martin von Zweigbergk writes:\n>\n> And as a Porcelain, I would rather expect it to leave the resulting\n> index refreshed.\n\nYeah, I guess you're right. Regular users (those using only porcelain)\nshouldn't notice, but it does make sense to think that the index would\nbe refreshed after running a porcelain. And the risk of breaking\npeople's scripts seems real too. I'll drop patch this from the re-roll\n(which I'll also make sure I'll sign off)\n\n(FYI, the reason I wrote this patch was because I was surprised that\n\"git reset\" did anything with the worktree at all.)\n"},{"id":"206408","messageId":"7vpq1e5ent.fsf@alter.siamese.dyndns.org","threadId":"32568","inReplyTo":"1357719376-16406-7-git-send-email-martinvonz@gmail.com","subject":"Re: [PATCH 06/19] reset.c: remove unnecessary variable 'i'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-09T19:39:50Z","receivedAt":"2013-01-09T19:39:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk writes:\n\n> Throughout most of parse_args(), the variable 'i' remains at 0. In the\n> remaining few cases, we can do pointer arithmentic on argv itself\n> instead.\n> ---\n> This is clearly mostly a matter of taste. The remainder of the series\n> does not depend on it in any way.\n\nI agree that it indeed is a matter of taste between\n\n (1) look at av[i], check with (i < ac) for the end, and increment i to\n iterate over the arguments; and\n\n (2) look at av[0], check with (0 < ac) for the end, and increment\n av and decrement ac at the same time to iterate over the\n arguments.\n\nWhen (ac, av) appear as a pair, however, adjusting only av without\nadjusting ac is asking for future trouble. It violates a common\nexpectation that av[ac] points at the NULL at the end of the list.\n\nIf a code chooses to use !av[0] as the terminating condition and\nnever looks at ac, then incrementing only av is fine, but in such a\ncase, the function probably should lose ac altogether.\n\n> builtin/reset.c | 29 ++++++++++++++---------------\n> 1 file changed, 14 insertions(+), 15 deletions(-)\n>\n> diff --git a/builtin/reset.c b/builtin/reset.c\n> index 9473725..68be05c 100644\n> --- a/builtin/reset.c\n> +++ b/builtin/reset.c\n> @@ -199,7 +199,6 @@ static void die_if_unmerged_cache(int reset_type)\n> }\n> \n> const char **parse_args(int argc, const char **argv, const char *prefix, const char **rev_ret) {\n> -\tint i = 0;\n> \tconst char *rev = \"HEAD\";\n> \tunsigned char unused[20];\n> \t/*\n> @@ -210,34 +209,34 @@ const char **parse_args(int argc, const char **argv, const char *prefix, const c\n> \t * git reset [-opts] -- ...\n> \t * git reset [-opts] ...\n> \t *\n> -\t * At this point, argv[i] points immediately after [-opts].\n> +\t * At this point, argv points immediately after [-opts].\n> \t */\n> \n> -\tif (i < argc) {\n> -\t\tif (!strcmp(argv[i], \"--\")) {\n> -\t\t\ti++; /* reset to HEAD, possibly with paths */\n> -\t\t} else if (i + 1 < argc && !strcmp(argv[i+1], \"--\")) {\n> -\t\t\trev = argv[i];\n> -\t\t\ti += 2;\n> +\tif (argc) {\n> +\t\tif (!strcmp(argv[0], \"--\")) {\n> +\t\t\targv++; /* reset to HEAD, possibly with paths */\n> +\t\t} else if (argc > 1 && !strcmp(argv[1], \"--\")) {\n> +\t\t\trev = argv[0];\n> +\t\t\targv += 2;\n> \t\t}\n> \t\t/*\n> -\t\t * Otherwise, argv[i] could be either or and\n> +\t\t * Otherwise, argv[0] could be either or and\n> \t\t * has to be unambiguous.\n> \t\t */\n> -\t\telse if (!get_sha1_committish(argv[i], unused)) {\n> +\t\telse if (!get_sha1_committish(argv[0], unused)) {\n> \t\t\t/*\n> -\t\t\t * Ok, argv[i] looks like a rev; it should not\n> +\t\t\t * Ok, argv[0] looks like a rev; it should not\n> \t\t\t * be a filename.\n> \t\t\t */\n> -\t\t\tverify_non_filename(prefix, argv[i]);\n> -\t\t\trev = argv[i++];\n> +\t\t\tverify_non_filename(prefix, argv[0]);\n> +\t\t\trev = *argv++;\n> \t\t} else {\n> \t\t\t/* Otherwise we treat this as a filename */\n> -\t\t\tverify_filename(prefix, argv[i], 1);\n> +\t\t\tverify_filename(prefix, argv[0], 1);\n> \t\t}\n> \t}\n> \t*rev_ret = rev;\n> -\treturn i < argc ? get_pathspec(prefix, argv + i) : NULL;\n> +\treturn *argv ? get_pathspec(prefix, argv) : NULL;\n> }\n> \n> int cmd_reset(int argc, const char **argv, const char *prefix)\n"},{"id":"206410","messageId":"7vlic25e9d.fsf@alter.siamese.dyndns.org","threadId":"32568","inReplyTo":"1357719376-16406-9-git-send-email-martinvonz@gmail.com","subject":"Re: [PATCH 08/19] reset.c: share call to die_if_unmerged_cache()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-09T19:48:30Z","receivedAt":"2013-01-09T19:48:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk writes:\n\n> Use a single condition to guard the call to die_if_unmerged_cache for\n> both --soft and --keep. This avoids the small distraction of the\n> precondition check from the logic following it.\n>\n> Also change an instance of\n>\n> if (e)\n> err = err || f();\n>\n> to the almost as short, but clearer\n>\n> if (e && !err)\n> err = f();\n>\n> (which is equivalent since we only care whether exit code is 0)\n\nIt is not just equivalent, but should give us identical result, even\nif we cared the actual value.\n\nAnd I tend to agree that the latter is more readable, especially\nwhen f() can be longer, which is often the case in real life.\n\nHappy to see this change.\n\n> ---\n> builtin/reset.c | 14 ++++++--------\n> 1 file changed, 6 insertions(+), 8 deletions(-)\n>\n> diff --git a/builtin/reset.c b/builtin/reset.c\n> index 4d556e7..42d1563 100644\n> --- a/builtin/reset.c\n> +++ b/builtin/reset.c\n> @@ -336,15 +336,13 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n> \t/* Soft reset does not touch the index file nor the working tree\n> \t * at all, but requires them in a good order. Other resets reset\n> \t * the index file to the tree object we are switching to. */\n> -\tif (reset_type == SOFT)\n> +\tif (reset_type == SOFT || reset_type == KEEP)\n> \t\tdie_if_unmerged_cache(reset_type);\n> -\telse {\n> -\t\tint err;\n> -\t\tif (reset_type == KEEP)\n> -\t\t\tdie_if_unmerged_cache(reset_type);\n> -\t\terr = reset_index_file(sha1, reset_type, quiet);\n> -\t\tif (reset_type == KEEP)\n> -\t\t\terr = err || reset_index_file(sha1, MIXED, quiet);\n> +\n> +\tif (reset_type != SOFT) {\n> +\t\tint err = reset_index_file(sha1, reset_type, quiet);\n> +\t\tif (reset_type == KEEP && !err)\n> +\t\t\terr = reset_index_file(sha1, MIXED, quiet);\n> \t\tif (err)\n> \t\t\tdie(_(\"Could not reset index file to revision '%s'.\"), rev);\n> \t}\n"},{"id":"206414","messageId":"7vhamq5e1v.fsf@alter.siamese.dyndns.org","threadId":"32568","inReplyTo":"1357719376-16406-10-git-send-email-martinvonz@gmail.com","subject":"Re: [PATCH 09/19] reset.c: replace switch by if-else","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-09T19:53:00Z","receivedAt":"2013-01-09T19:53:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk writes:\n\n> ---\n> builtin/reset.c | 13 +++----------\n> 1 file changed, 3 insertions(+), 10 deletions(-)\n>\n> diff --git a/builtin/reset.c b/builtin/reset.c\n> index 42d1563..05ccfd4 100644\n> --- a/builtin/reset.c\n> +++ b/builtin/reset.c\n> @@ -351,18 +351,11 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n> \t * saving the previous head in ORIG_HEAD before. */\n> \tupdate_ref_status = update_refs(rev, sha1);\n> \n> -\tswitch (reset_type) {\n> -\tcase HARD:\n> -\t\tif (!update_ref_status && !quiet)\n> -\t\t\tprint_new_head_line(commit);\n> -\t\tbreak;\n> -\tcase SOFT: /* Nothing else to do. */\n> -\t\tbreak;\n> -\tcase MIXED: /* Report what has not been updated. */\n> +\tif (reset_type == HARD && !update_ref_status && !quiet)\n> +\t\tprint_new_head_line(commit);\n> +\telse if (reset_type == MIXED) /* Report what has not been updated. */\n> \t\tupdate_index_refresh(0, NULL,\n> \t\t\t\tquiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);\n> -\t\tbreak;\n> -\t}\n\nJustification?\n\nIt might be shorter, but I somehow find the original _much_ easier\nto follow, and to possibly extend. The case arms delineate the\nmajor modes of operation, and when somebody is interested in what\nhappens in \"reset --hard\", the case labels allow eyes to immediately\nspot and skip uninteresting case arms. On the other hand, the\nupdated one forces you to read the if/else cascade through.\n"},{"id":"206416","messageId":"7vd2xe5dxu.fsf@alter.siamese.dyndns.org","threadId":"32568","inReplyTo":"1357719376-16406-11-git-send-email-martinvonz@gmail.com","subject":"Re: [PATCH 10/19] reset --keep: only write index file once","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-09T19:55:25Z","receivedAt":"2013-01-09T19:55:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk writes:\n\n> \"git reset --keep\" calls reset_index_file() twice, first doing a\n> two-way merge to the target revision, updating the index and worktree,\n> and then resetting the index. After each call, we write the index\n> file.\n>\n> In the unlikely event that the second call to reset_index_file()\n> fails, the index will have been merged to the target revision, but\n> HEAD will not be updated, leaving the user with a dirty index.\n>\n> By moving the locking, writing and committing out of\n> reset_index_file() and into the caller, we can avoid writing the index\n> twice, thereby making the sure we don't end up in the half-way reset\n> state.\n\nNice.\n"},{"id":"206417","messageId":"7v8v825drk.fsf@alter.siamese.dyndns.org","threadId":"32568","inReplyTo":"1357719376-16406-16-git-send-email-martinvonz@gmail.com","subject":"Re: [PATCH 15/19] reset.c: finish entire cmd_reset() whether or not pathspec is given","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-09T19:59:11Z","receivedAt":"2013-01-09T19:59:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk writes:\n\n> By not returning from inside the \"if (pathspec)\" block, we can let the\n> pathspec-aware and pathspec-less code share a bit more, making it\n> easier to make future changes that should affect both cases. This also\n> highlights the similarity between read_from_tree() and reset_index().\n> ---\n> Should error reporting be aligned too? Speaking of which,\n> do_diff_cache() never returns anything by 0. Is the return value for\n> future-proofing?\n\nPerhaps, and yes.\n\n>\n> builtin/reset.c | 42 ++++++++++++++++++------------------------\n> 1 file changed, 18 insertions(+), 24 deletions(-)\n>\n> diff --git a/builtin/reset.c b/builtin/reset.c\n> index 254afa9..9bcad29 100644\n> --- a/builtin/reset.c\n> +++ b/builtin/reset.c\n> @@ -308,19 +308,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n> \t\tdie(_(\"%s reset is not allowed in a bare repository\"),\n> \t\t _(reset_type_names[reset_type]));\n> \n> -\tif (pathspec) {\n> -\t\tstruct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n> -\t\tint index_fd = hold_locked_index(lock, 1);\n> -\t\tif (read_from_tree(pathspec, sha1))\n> -\t\t\treturn 1;\n> -\t\tupdate_index_refresh(\n> -\t\t\tquiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);\n> -\t\tif (write_cache(index_fd, active_cache, active_nr) ||\n> -\t\t commit_locked_index(lock))\n> -\t\t\treturn error(\"Could not write new index file.\");\n> -\t\treturn 0;\n> -\t}\n> -\n> \t/* Soft reset does not touch the index file nor the working tree\n> \t * at all, but requires them in a good order. Other resets reset\n> \t * the index file to the tree object we are switching to. */\n> @@ -330,11 +317,16 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n> \tif (reset_type != SOFT) {\n> \t\tstruct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n> \t\tint newfd = hold_locked_index(lock, 1);\n> -\t\tint err = reset_index(sha1, reset_type, quiet);\n> -\t\tif (reset_type == KEEP && !err)\n> -\t\t\terr = reset_index(sha1, MIXED, quiet);\n> -\t\tif (err)\n> -\t\t\tdie(_(\"Could not reset index file to revision '%s'.\"), rev);\n> +\t\tif (pathspec) {\n> +\t\t\tif (read_from_tree(pathspec, sha1))\n> +\t\t\t\treturn 1;\n> +\t\t} else {\n> +\t\t\tint err = reset_index(sha1, reset_type, quiet);\n> +\t\t\tif (reset_type == KEEP && !err)\n> +\t\t\t\terr = reset_index(sha1, MIXED, quiet);\n> +\t\t\tif (err)\n> +\t\t\t\tdie(_(\"Could not reset index file to revision '%s'.\"), rev);\n> +\t\t}\n> \n> \t\tif (reset_type == MIXED) /* Report what has not been updated. */\n> \t\t\tupdate_index_refresh(\n> @@ -345,14 +337,16 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n> \t\t\tdie(_(\"Could not write new index file.\"));\n> \t}\n> \n> -\t/* Any resets update HEAD to the head being switched to,\n> -\t * saving the previous head in ORIG_HEAD before. */\n> -\tupdate_ref_status = update_refs(rev, sha1);\n> +\tif (!pathspec) {\n> +\t\t/* Any resets without paths update HEAD to the head being\n> +\t\t * switched to, saving the previous head in ORIG_HEAD before. */\n> +\t\tupdate_ref_status = update_refs(rev, sha1);\n> \n> -\tif (reset_type == HARD && !update_ref_status && !quiet)\n> -\t\tprint_new_head_line(commit);\n> +\t\tif (reset_type == HARD && !update_ref_status && !quiet)\n> +\t\t\tprint_new_head_line(commit);\n> \n> -\tremove_branch_state();\n> +\t\tremove_branch_state();\n> +\t}\n> \n> \treturn update_ref_status;\n> }\n"},{"id":"206418","messageId":"7v4niq5dgq.fsf@alter.siamese.dyndns.org","threadId":"32568","inReplyTo":"1357719376-16406-17-git-send-email-martinvonz@gmail.com","subject":"Re: [PATCH 16/19] reset [--mixed] --quiet: don't refresh index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-09T20:05:41Z","receivedAt":"2013-01-09T20:05:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk writes:\n\n> There is a test case in t7102 called '--mixed refreshes the index',\n> but it only checks that right output it printed.\n\nI think that comes from 620a6cd (builtin-reset: avoid forking\n\"update-index --refresh\", 2007-11-03). Before that commit, we\nrefreshed the index with --mixed, and the test tries to make sure we\ncontinue to do so after the change. Even though it is not testing\nif the index has stat only changes (which is rather cumbersome to\nwrite---you need to futz with timestamp or something) and using the\noutput from refresh machinery as a substitute, I think the intent of\nthat commit is fairly clear.\n\n> builtin/reset.c | 12 +++---------\n> 1 file changed, 3 insertions(+), 9 deletions(-)\n>\n> diff --git a/builtin/reset.c b/builtin/reset.c\n> index 9bcad29..a2e69eb 100644\n> --- a/builtin/reset.c\n> +++ b/builtin/reset.c\n> @@ -109,12 +109,6 @@ static void print_new_head_line(struct commit *commit)\n> \t\tprintf(\"\\n\");\n> }\n> \n> -static void update_index_refresh(int flags)\n> -{\n> -\trefresh_index(&the_index, (flags), NULL, NULL,\n> -\t\t _(\"Unstaged changes after reset:\"));\n> -}\n> -\n> static void update_index_from_diff(struct diff_queue_struct *q,\n> \t\tstruct diff_options *opt, void *data)\n> {\n> @@ -328,9 +322,9 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n> \t\t\t\tdie(_(\"Could not reset index file to revision '%s'.\"), rev);\n> \t\t}\n> \n> -\t\tif (reset_type == MIXED) /* Report what has not been updated. */\n> -\t\t\tupdate_index_refresh(\n> -\t\t\t\tquiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);\n> +\t\tif (reset_type == MIXED && !quiet) /* Report what has not been updated. */\n> +\t\t\trefresh_index(&the_index, REFRESH_IN_PORCELAIN, NULL, NULL,\n> +\t\t\t\t _(\"Unstaged changes after reset:\"));\n> \n> \t\tif (write_cache(newfd, active_cache, active_nr) ||\n> \t\t commit_locked_index(lock))\n"},{"id":"206421","messageId":"7vzk0i3y1w.fsf@alter.siamese.dyndns.org","threadId":"32568","inReplyTo":"1357719376-16406-18-git-send-email-martinvonz@gmail.com","subject":"Re: [PATCH 17/19] reset $sha1 $pathspec: require $sha1 only to be treeish","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-09T20:23:55Z","receivedAt":"2013-01-09T20:23:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk writes:\n\n> Resetting with paths does not update HEAD and there is nothing else\n> that a commit should be needed for. Relax the argument parsing so only\n> a tree is required.\n>\n> The sha1 is only passed to read_from_tree(), which already only\n> requires a tree.\n>\n> The \"rev\" variable we pass to run_add_interactive() will resolve to a\n> tree. This is fine since interactive_reset only needs the parameter to\n> be a treeish and doesn't use it for display purposes.\n> ---\n> Is it correct that interactive_reset does not use the revision\n> specifier for display purposes? Or, worse, that it requires it to be a\n> commit in some cases? I tried it and didn't see any problem.\n\nAs far as I know, it is only given to git-diff-index as the tree-ish,\nand resulting patch text is used for application via git-apply just\nlike any patch coming from any origin, so I think it should be fine.\n\n> Can the two blocks of code that look up commit or tree be made to\n> share more? I'm not very familiar with what functions are available. I\n> think I tried keeping a separate \"struct object *object\" to be able to\n> put the last three lines outside the blocks, but didn't like the\n> result.\n\nI think the patch looks fine from the sharing perspective, but it\nmay be even nicer to have a separate variable to hold a commit\nobject limited to the scope of if (!pathspec) block to make them\nmore symmetric. The commit is only needed later to show \"we are now\nat this commit\", but that code can find the commit itself given the\nobject name in sha1[].\n\n> builtin/reset.c | 46 ++++++++++++++++++++++++++--------------------\n> t/t7102-reset.sh | 8 ++++++++\n> 2 files changed, 34 insertions(+), 20 deletions(-)\n>\n> diff --git a/builtin/reset.c b/builtin/reset.c\n> index a2e69eb..4c223bd 100644\n> --- a/builtin/reset.c\n> +++ b/builtin/reset.c\n> @@ -177,9 +177,10 @@ const char **parse_args(int argc, const char **argv, const char *prefix, const c\n> \t/*\n> \t * Possible arguments are:\n> \t *\n> -\t * git reset [-opts] ...\n> -\t * git reset [-opts] -- ...\n> -\t * git reset [-opts] -- ...\n> +\t * git reset [-opts] []\n> +\t * git reset [-opts] [...]\n> +\t * git reset [-opts] -- [...]\n> +\t * git reset [-opts] -- [...]\n> \t * git reset [-opts] ...\n> \t *\n> \t * At this point, argv points immediately after [-opts].\n> @@ -194,11 +195,13 @@ const char **parse_args(int argc, const char **argv, const char *prefix, const c\n> \t\t}\n> \t\t/*\n> \t\t * Otherwise, argv[0] could be either or and\n> -\t\t * has to be unambiguous.\n> +\t\t * has to be unambiguous. If there is a single argument, it\n> +\t\t * can not be a tree\n> \t\t */\n> -\t\telse if (!get_sha1_committish(argv[0], unused)) {\n> +\t\telse if ((argc == 1 && !get_sha1_committish(argv[0], unused)) ||\n> +\t\t\t (argc > 1 && !get_sha1_treeish(argv[0], unused))) {\n> \t\t\t/*\n> -\t\t\t * Ok, argv[0] looks like a rev; it should not\n> +\t\t\t * Ok, argv[0] looks like a commit/tree; it should not\n> \t\t\t * be a filename.\n> \t\t\t */\n> \t\t\tverify_non_filename(prefix, argv[0]);\n> @@ -240,7 +243,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n> \tconst char *rev;\n> \tunsigned char sha1[20];\n> \tconst char **pathspec = NULL;\n> -\tstruct commit *commit;\n> +\tstruct commit *commit = NULL;\n> \tconst struct option options[] = {\n> \t\tOPT__QUIET(&quiet, N_(\"be quiet, only report errors\")),\n> \t\tOPT_SET_INT(0, \"mixed\", &reset_type,\n> @@ -262,19 +265,22 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n> \t\t\t\t\t\tPARSE_OPT_KEEP_DASHDASH);\n> \tpathspec = parse_args(argc, argv, prefix, &rev);\n> \n> -\tif (get_sha1_committish(rev, sha1))\n> -\t\tdie(_(\"Failed to resolve '%s' as a valid ref.\"), rev);\n> -\n> -\t/*\n> -\t * NOTE: As \"git reset $treeish -- $path\" should be usable on\n> -\t * any tree-ish, this is not strictly correct. We are not\n> -\t * moving the HEAD to any commit; we are merely resetting the\n> -\t * entries in the index to that of a treeish.\n> -\t */\n> -\tcommit = lookup_commit_reference(sha1);\n> -\tif (!commit)\n> -\t\tdie(_(\"Could not parse object '%s'.\"), rev);\n> -\thashcpy(sha1, commit->object.sha1);\n> +\tif (!pathspec) {\n> +\t\tif (get_sha1_committish(rev, sha1))\n> +\t\t\tdie(_(\"Failed to resolve '%s' as a valid revision.\"), rev);\n> +\t\tcommit = lookup_commit_reference(sha1);\n> +\t\tif (!commit)\n> +\t\t\tdie(_(\"Could not parse object '%s'.\"), rev);\n> +\t\thashcpy(sha1, commit->object.sha1);\n> +\t} else {\n> +\t\tstruct tree *tree;\n> +\t\tif (get_sha1_treeish(rev, sha1))\n> +\t\t\tdie(_(\"Failed to resolve '%s' as a valid tree.\"), rev);\n> +\t\ttree = parse_tree_indirect(sha1);\n> +\t\tif (!tree)\n> +\t\t\tdie(_(\"Could not parse object '%s'.\"), rev);\n> +\t\thashcpy(sha1, tree->object.sha1);\n> +\t}\n> \n> \tif (patch_mode) {\n> \t\tif (reset_type != NONE)\n> diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh\n> index 81b2570..1fa2a5f 100755\n> --- a/t/t7102-reset.sh\n> +++ b/t/t7102-reset.sh\n> @@ -497,4 +497,12 @@ test_expect_success 'disambiguation (4)' '\n> \ttest ! -f secondfile\n> '\n> \n> +test_expect_success 'reset with paths accepts tree' '\n> +\t# for simpler tests, drop last commit containing added files\n> +\tgit reset --hard HEAD^ &&\n> +\tgit reset HEAD^^{tree} -- . &&\n> +\tgit diff --cached HEAD^ --exit-code &&\n> +\tgit diff HEAD --exit-code\n> +'\n> +\n> test_done\n"},{"id":"206422","messageId":"7vvcb63xvw.fsf@alter.siamese.dyndns.org","threadId":"32568","inReplyTo":"1357719376-16406-20-git-send-email-martinvonz@gmail.com","subject":"Re: [PATCH 19/19] reset [--mixed]: use diff-based reset whether or not pathspec was given","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-09T20:27:31Z","receivedAt":"2013-01-09T20:27:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk writes:\n\n> Thanks to b65982b (Optimize \"diff-index --cached\" using cache-tree,\n> 2009-05-20), resetting with paths is much faster than resetting\n> without paths. Some timings for the linux-2.6 repo to illustrate this\n> (best of five, warm cache):\n>\n> reset reset .\n> real 0m0.219s 0m0.080s\n> user 0m0.140s 0m0.040s\n> sys 0m0.070s 0m0.030s\n\nNice.\n"},{"id":"206444","messageId":"CANiSa6gK+RqovV+NKWgV57hz-p_O085HN7WCg9qvQAD-Ynpfjw@mail.gmail.com","threadId":"32568","inReplyTo":"7vtxqq5f0g.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 04/19] reset: don't allow \"git reset -- $pathspec\" in bare repo","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-10T08:24:10Z","receivedAt":"2013-01-10T08:24:10Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"On Wed, Jan 9, 2013 at 11:32 AM, Junio C Hamano wrote:\n> Martin von Zweigbergk writes:\n>\n>> ---\n>> builtin/reset.c | 6 ++++--\n>> 1 file changed, 4 insertions(+), 2 deletions(-)\n>\n> With the patch that does not have any explicit check for bareness\n> nor new error message to scold user with, it is rather hard to tell\n> what is going on, without any description on what (if anything) is\n> broken at the end user level and what remedy is done about that\n> breakage...\n\nWill include the following in a re-roll.\n\n reset: don't allow \"git reset -- $pathspec\" in bare repo\n\n Running e.g. \"git reset .\" in a bare repo results in an index file\n being created from the HEAD commit. The differences compared to the\n index are then printed as usual, but since there is no worktree, it\n will appear as if all files are deleted. For example, in a bare clone\n of git.git:\n\n Unstaged changes after reset:\n D .gitattributes\n D .gitignore\n D .mailmap\n ...\n\n This happens because the check for is_bare_repository() happens after\n we branch off into read_from_tree() to reset with paths. Fix by moving\n the branching point after the check.\n"},{"id":"206445","messageId":"CANiSa6jJcYRUyswSktq86pWttxBjZedZcPheRgPCf+EubC_kLg@mail.gmail.com","threadId":"32568","inReplyTo":"7vpq1e5ent.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 06/19] reset.c: remove unnecessary variable 'i'","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-10T08:41:49Z","receivedAt":"2013-01-10T08:41:49Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"On Wed, Jan 9, 2013 at 11:39 AM, Junio C Hamano wrote:\n> Martin von Zweigbergk writes:\n>\n>> Throughout most of parse_args(), the variable 'i' remains at 0. In the\n>> remaining few cases, we can do pointer arithmentic on argv itself\n>> instead.\n>> ---\n>> This is clearly mostly a matter of taste. The remainder of the series\n>> does not depend on it in any way.\n>\n> I agree that it indeed is a matter of taste between\n>\n> (1) look at av[i], check with (i < ac) for the end, and increment i to\n> iterate over the arguments; and\n>\n> (2) look at av[0], check with (0 < ac) for the end, and increment\n> av and decrement ac at the same time to iterate over the\n> arguments.\n>\n> When (ac, av) appear as a pair, however, adjusting only av without\n> adjusting ac is asking for future trouble. It violates a common\n> expectation that av[ac] points at the NULL at the end of the list.\n\nGood points.\n\n> If a code chooses to use !av[0] as the terminating condition and\n> never looks at ac, then incrementing only av is fine, but in such a\n> case, the function probably should lose ac altogether.\n\nMakes sense. I've picked this style for now (i.e. dropped both 'i' and\n'argc'). I was surprised by the style that referred to the variable in\nmany places where it was know to be 0, but I'm no experienced C\nprogrammer, so if that's a common practice when it comes to argument\nparsing, I'm also happy to drop the patch. Let me know what you\nprefer.\n"},{"id":"206446","messageId":"CANiSa6jeD6=fuXoQD5UWX1ZX18OFAVYTPLEuVi5vp6R0ivLLzQ@mail.gmail.com","threadId":"32568","inReplyTo":"7vlic25e9d.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 08/19] reset.c: share call to die_if_unmerged_cache()","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-10T08:51:59Z","receivedAt":"2013-01-10T08:51:59Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"On Wed, Jan 9, 2013 at 11:48 AM, Junio C Hamano wrote:\n> Martin von Zweigbergk writes:\n>\n>> Use a single condition to guard the call to die_if_unmerged_cache for\n>> both --soft and --keep. This avoids the small distraction of the\n>> precondition check from the logic following it.\n>>\n>> Also change an instance of\n>>\n>> if (e)\n>> err = err || f();\n>>\n>> to the almost as short, but clearer\n>>\n>> if (e && !err)\n>> err = f();\n>>\n>> (which is equivalent since we only care whether exit code is 0)\n>\n> It is not just equivalent, but should give us identical result, even\n> if we cared the actual value.\n\nIf err is initially 0, and f() evaluates to 2, err would be 1 in the\nfirst case, but 2 in the second case, right?\n\nI think the two might be identical in e.g. JavaScript and Python, but\nI don't use either much.\n"},{"id":"206449","messageId":"CACsJy8Apu1BJ2t+vpbzpQ4Wni==Azzmp99a+TmBzR3h8qpx=8g@mail.gmail.com","threadId":"32568","inReplyTo":"7vy5g25f9b.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 03/19] reset.c: pass pathspec around instead of (prefix, argv) pair","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-01-10T11:05:24Z","receivedAt":"2013-01-10T11:05:24Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Jan 10, 2013 at 2:26 AM, Junio C Hamano wrote:\n> Martin von Zweigbergk writes:\n>\n>> We use the path arguments in two places in reset.c: in\n>> interactive_reset() and read_from_tree(). Both of these call\n>> get_pathspec(), so we pass the (prefix, arv) pair to both\n>> functions. Move the call to get_pathspec() out of these methods, for\n>> two reasons: 1) One argument is simpler than two. 2) It lets us use\n>> the (arguably clearer) \"if (pathspec)\" in place of \"if (i < argc)\".\n>> ---\n>> If I understand correctly, this should be rebased on top of\n>> nd/parse-pathspec. Please let me know.\n>\n> Yeah, this will conflict with the get_pathspec-to-parse_pathspec\n> conversion Duy has been working on.\n\nOr I could hold off nd/parse-pathspec if this series has a better\nchance of graduation first. Decision?\n-- \nDuy\n"},{"id":"206462","messageId":"7vvcb429um.fsf@alter.siamese.dyndns.org","threadId":"32568","inReplyTo":"CANiSa6gK+RqovV+NKWgV57hz-p_O085HN7WCg9qvQAD-Ynpfjw@mail.gmail.com","subject":"Re: [PATCH 04/19] reset: don't allow \"git reset -- $pathspec\" in bare repo","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-10T18:04:17Z","receivedAt":"2013-01-10T18:04:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk writes:\n\n> ... Fix by moving\n> the branching point after the check.\n\nOK, that is what I missed. We have an existing check for mixed\nreset, which was originally meant to handle case without any\npathspec but can use the same error condition (i.e. type is mixed\nand repository is bare) and error message (i.e. no mixed reset in\na bare repository). \"reset with pathspec\" was done before that\ncheck kicked in.\n\nThanks for clarification (and sorry for the noise).\n"},{"id":"206476","messageId":"7vfw28eiu3.fsf@alter.siamese.dyndns.org","threadId":"32568","inReplyTo":"CACsJy8Apu1BJ2t+vpbzpQ4Wni==Azzmp99a+TmBzR3h8qpx=8g@mail.gmail.com","subject":"Re: [PATCH 03/19] reset.c: pass pathspec around instead of (prefix, argv) pair","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-10T23:09:24Z","receivedAt":"2013-01-10T23:09:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> On Thu, Jan 10, 2013 at 2:26 AM, Junio C Hamano wrote:\n>> Martin von Zweigbergk writes:\n>>\n>>> We use the path arguments in two places in reset.c: in\n>>> interactive_reset() and read_from_tree(). Both of these call\n>>> get_pathspec(), so we pass the (prefix, arv) pair to both\n>>> functions. Move the call to get_pathspec() out of these methods, for\n>>> two reasons: 1) One argument is simpler than two. 2) It lets us use\n>>> the (arguably clearer) \"if (pathspec)\" in place of \"if (i < argc)\".\n>>> ---\n>>> If I understand correctly, this should be rebased on top of\n>>> nd/parse-pathspec. Please let me know.\n>>\n>> Yeah, this will conflict with the get_pathspec-to-parse_pathspec\n>> conversion Duy has been working on.\n>\n> Or I could hold off nd/parse-pathspec if this series has a better\n> chance of graduation first. Decision?\n\nI am greedy and want to have both ;-)\n\nBefore deciding that, I'd appreciate a second set of eyes giving\nMartin's series an independent review, to see if it is going in the\nright direction. I think I didn't spot anything questionable in it\nmyself, but second opinion always helps.\n\nThere is no textual conflict between the two topics at the moment,\nbut because the ultimate goal of your series is to remove all uses\nof the pathspec.raw[] field outside the implementation of pathspec\nmatching, it might help to rename the field to _private_raw (or\nremove it), and either make get_pathspec() private or disappear, to\nensure that the compiler will help us catching semantic conflicts\nwith new users of it at a late stage of your series.\n"},{"id":"206497","messageId":"CANiSa6gz-DBv+2gUDPdhgmeYdHg3-OVO80a7NvdLn4vYRyKEnA@mail.gmail.com","threadId":"32568","inReplyTo":"7vhamq5e1v.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 09/19] reset.c: replace switch by if-else","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-11T06:35:47Z","receivedAt":"2013-01-11T06:35:47Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"On Wed, Jan 9, 2013 at 11:53 AM, Junio C Hamano wrote:\n> Martin von Zweigbergk writes:\n>\n>> ---\n>> builtin/reset.c | 13 +++----------\n>> 1 file changed, 3 insertions(+), 10 deletions(-)\n>>\n>> diff --git a/builtin/reset.c b/builtin/reset.c\n>> index 42d1563..05ccfd4 100644\n>> --- a/builtin/reset.c\n>> +++ b/builtin/reset.c\n>> @@ -351,18 +351,11 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n>> * saving the previous head in ORIG_HEAD before. */\n>> update_ref_status = update_refs(rev, sha1);\n>>\n>> - switch (reset_type) {\n>> - case HARD:\n>> - if (!update_ref_status && !quiet)\n>> - print_new_head_line(commit);\n>> - break;\n>> - case SOFT: /* Nothing else to do. */\n>> - break;\n>> - case MIXED: /* Report what has not been updated. */\n>> + if (reset_type == HARD && !update_ref_status && !quiet)\n>> + print_new_head_line(commit);\n>> + else if (reset_type == MIXED) /* Report what has not been updated. */\n>> update_index_refresh(0, NULL,\n>> quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);\n>> - break;\n>> - }\n>\n> Justification?\n\nClairvoyance -- the HARD case will soon be the only non-empty case.\nIt's also missing KEEP and MERGE (but the empty SOFT block is there).\n\nI'll update the message. I will also move the patch a little later in\nthe series, closer to where it will be useful.\n"},{"id":"206507","messageId":"CACsJy8C4_Cvy+Q52gaHeWgdo_yZDcCW=tpaNOWuthURfPN8NrA@mail.gmail.com","threadId":"32568","inReplyTo":"7vfw28eiu3.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 03/19] reset.c: pass pathspec around instead of (prefix, argv) pair","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-01-11T11:10:06Z","receivedAt":"2013-01-11T11:10:06Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Jan 11, 2013 at 6:09 AM, Junio C Hamano wrote:\n>> Or I could hold off nd/parse-pathspec if this series has a better\n>> chance of graduation first. Decision?\n>\n> I am greedy and want to have both ;-)\n\nApparently I have no problems with your being greedy.\n\n> There is no textual conflict between the two topics at the moment,\n> but because the ultimate goal of your series is to remove all uses\n> of the pathspec.raw[] field outside the implementation of pathspec\n> matching, it might help to rename the field to _private_raw (or\n> remove it), and either make get_pathspec() private or disappear, to\n> ensure that the compiler will help us catching semantic conflicts\n> with new users of it at a late stage of your series.\n\nThere are still some uses for get_pathspec() and new call sites won't\ncause big problems because they would need init_pathspec() to convert\nget_pathspec() results to struct pathspec. I will rename raw[] though.\n-- \nDuy\n"},{"id":"206543","messageId":"7vwqvjbq3y.fsf@alter.siamese.dyndns.org","threadId":"32568","inReplyTo":"CANiSa6gz-DBv+2gUDPdhgmeYdHg3-OVO80a7NvdLn4vYRyKEnA@mail.gmail.com","subject":"Re: [PATCH 09/19] reset.c: replace switch by if-else","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-11T17:12:49Z","receivedAt":"2013-01-11T17:12:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk writes:\n\n>> Justification?\n>\n> Clairvoyance ...\n\n;-)\n"},{"id":"206900","messageId":"1358228871-7142-1-git-send-email-martinvonz@gmail.com","threadId":"32568","inReplyTo":"1357719376-16406-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH v2 00/19] reset improvements","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-15T05:47:32Z","receivedAt":"2013-01-15T05:47:32Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"Changes since v1:\n\n - Spelling fixes.\n\n - Explained how \"git reset -- $pathspec\" in bare repo is broken.\n\n - Provided motivation for replacement of switch by if-else\n\n - Fixed argv/argc handling by removing use of argc.\n\n - Replaced \"don't refresh index on --quiet\" patch by one that just\n inlines update_index_refresh()\n\n - Incorporated fixes from Junio's repo\n\n - Provided some motivation for \"replace switch by if-else\" amd moved\n the patch later in the series.\n\nThanks for reviewing!\n\n\nMartin von Zweigbergk (19):\n reset $pathspec: no need to discard index\n reset $pathspec: exit with code 0 if successful\n reset.c: pass pathspec around instead of (prefix, argv) pair\n reset: don't allow \"git reset -- $pathspec\" in bare repo\n reset.c: extract function for parsing arguments\n reset.c: remove unnecessary variable 'i'\n reset.c: extract function for updating {ORIG_,}HEAD\n reset.c: share call to die_if_unmerged_cache()\n reset --keep: only write index file once\n reset: avoid redundant error message\n reset.c: replace switch by if-else\n reset.c: move update_index_refresh() call out of read_from_tree()\n reset.c: move lock, write and commit out of update_index_refresh()\n reset [--mixed]: only write index file once\n reset.c: finish entire cmd_reset() whether or not pathspec is given\n reset.c: inline update_index_refresh()\n reset $sha1 $pathspec: require $sha1 only to be treeish\n reset: allow reset on unborn branch\n reset [--mixed]: use diff-based reset whether or not pathspec was\n given\n\n builtin/reset.c | 283 +++++++++++++++++++----------------------\n t/t2013-checkout-submodule.sh | 2 +-\n t/t7102-reset.sh | 26 +++-\n t/t7106-reset-unborn-branch.sh | 52 ++++++++\n 4 files changed, 203 insertions(+), 160 deletions(-)\n create mode 100755 t/t7106-reset-unborn-branch.sh\n\n-- \n1.8.1.1.454.gce43f05\n"},{"id":"206884","messageId":"1358228871-7142-2-git-send-email-martinvonz@gmail.com","threadId":"32568","inReplyTo":"1358228871-7142-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH v2 01/19] reset $pathspec: no need to discard index","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-15T05:47:33Z","receivedAt":"2013-01-15T05:47:33Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"Since 34110cd (Make 'unpack_trees()' have a separate source and\ndestination index, 2008-03-06), the index no longer gets clobbered by\ndo_diff_cache() and we can remove the code for discarding and\nre-reading it.\n\nThere are two paths to update_index_refresh() from cmd_reset(), but on\nboth paths, either read_cache() or read_cache_unmerged() will have\nbeen called, so the call to read_cache() in this method is redundant\n(although practically free).\n\nThis speeds up \"git reset -- .\" a little on the linux-2.6 repo (best\nof five, warm cache):\n\n Before After\nreal 0m0.093s 0m0.080s\nuser 0m0.040s 0m0.020s\nsys 0m0.050s 0m0.050s\n\nSigned-off-by: Martin von Zweigbergk \n---\n builtin/reset.c | 16 +---------------\n 1 file changed, 1 insertion(+), 15 deletions(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 915cc9f..8cc7c72 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -126,9 +126,6 @@ static int update_index_refresh(int fd, struct lock_file *index_lock, int flags)\n \t\tfd = hold_locked_index(index_lock, 1);\n \t}\n \n-\tif (read_cache() < 0)\n-\t\treturn error(_(\"Could not read index\"));\n-\n \tresult = refresh_index(&the_index, (flags), NULL, NULL,\n \t\t\t _(\"Unstaged changes after reset:\")) ? 1 : 0;\n \tif (write_cache(fd, active_cache, active_nr) ||\n@@ -141,12 +138,6 @@ static void update_index_from_diff(struct diff_queue_struct *q,\n \t\tstruct diff_options *opt, void *data)\n {\n \tint i;\n-\tint *discard_flag = data;\n-\n-\t/* do_diff_cache() mangled the index */\n-\tdiscard_cache();\n-\t*discard_flag = 1;\n-\tread_cache();\n \n \tfor (i = 0; i < q->nr; i++) {\n \t\tstruct diff_filespec *one = q->queue[i]->one;\n@@ -179,17 +170,15 @@ static int read_from_tree(const char *prefix, const char **argv,\n \t\tunsigned char *tree_sha1, int refresh_flags)\n {\n \tstruct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n-\tint index_fd, index_was_discarded = 0;\n+\tint index_fd;\n \tstruct diff_options opt;\n \n \tmemset(&opt, 0, sizeof(opt));\n \tdiff_tree_setup_paths(get_pathspec(prefix, (const char **)argv), &opt);\n \topt.output_format = DIFF_FORMAT_CALLBACK;\n \topt.format_callback = update_index_from_diff;\n-\topt.format_callback_data = &index_was_discarded;\n \n \tindex_fd = hold_locked_index(lock, 1);\n-\tindex_was_discarded = 0;\n \tread_cache();\n \tif (do_diff_cache(tree_sha1, &opt))\n \t\treturn 1;\n@@ -197,9 +186,6 @@ static int read_from_tree(const char *prefix, const char **argv,\n \tdiff_flush(&opt);\n \tdiff_tree_release_paths(&opt);\n \n-\tif (!index_was_discarded)\n-\t\t/* The index is still clobbered from do_diff_cache() */\n-\t\tdiscard_cache();\n \treturn update_index_refresh(index_fd, lock, refresh_flags);\n }\n \n-- \n1.8.1.1.454.gce43f05\n"},{"id":"206888","messageId":"1358228871-7142-3-git-send-email-martinvonz@gmail.com","threadId":"32568","inReplyTo":"1358228871-7142-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH v2 02/19] reset $pathspec: exit with code 0 if successful","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-15T05:47:34Z","receivedAt":"2013-01-15T05:47:34Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"\"git reset $pathspec\" currently exits with a non-zero exit code if the\nworktree is dirty after resetting, which is inconsistent with reset\nwithout pathspec, and it makes it harder to know whether the command\nreally failed. Change it to exit with code 0 regardless of whether the\nworktree is dirty so that non-zero indicates an error.\n\nThis makes the 4 \"disambiguation\" test cases in t7102 clearer since\nthey all used to \"fail\", 3 of which \"failed\" due to changes in the\nwork tree. Now only the ambiguous one fails.\n\nSigned-off-by: Martin von Zweigbergk \n---\n builtin/reset.c | 8 +++-----\n t/t2013-checkout-submodule.sh | 2 +-\n t/t7102-reset.sh | 18 ++++++++++++------\n 3 files changed, 16 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 8cc7c72..65413d0 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -119,19 +119,17 @@ static void print_new_head_line(struct commit *commit)\n \n static int update_index_refresh(int fd, struct lock_file *index_lock, int flags)\n {\n-\tint result;\n-\n \tif (!index_lock) {\n \t\tindex_lock = xcalloc(1, sizeof(struct lock_file));\n \t\tfd = hold_locked_index(index_lock, 1);\n \t}\n \n-\tresult = refresh_index(&the_index, (flags), NULL, NULL,\n-\t\t\t _(\"Unstaged changes after reset:\")) ? 1 : 0;\n+\trefresh_index(&the_index, (flags), NULL, NULL,\n+\t\t _(\"Unstaged changes after reset:\"));\n \tif (write_cache(fd, active_cache, active_nr) ||\n \t\t\tcommit_locked_index(index_lock))\n \t\treturn error (\"Could not refresh index\");\n-\treturn result;\n+\treturn 0;\n }\n \n static void update_index_from_diff(struct diff_queue_struct *q,\ndiff --git a/t/t2013-checkout-submodule.sh b/t/t2013-checkout-submodule.sh\nindex 70edbb3..06b18f8 100755\n--- a/t/t2013-checkout-submodule.sh\n+++ b/t/t2013-checkout-submodule.sh\n@@ -23,7 +23,7 @@ test_expect_success '\"reset \" updates the index' '\n \tgit update-index --refresh &&\n \tgit diff-files --quiet &&\n \tgit diff-index --quiet --cached HEAD &&\n-\ttest_must_fail git reset HEAD^ submodule &&\n+\tgit reset HEAD^ submodule &&\n \ttest_must_fail git diff-files --quiet &&\n \tgit reset submodule &&\n \tgit diff-files --quiet\ndiff --git a/t/t7102-reset.sh b/t/t7102-reset.sh\nindex b096dc8..81b2570 100755\n--- a/t/t7102-reset.sh\n+++ b/t/t7102-reset.sh\n@@ -388,7 +388,8 @@ test_expect_success 'test --mixed ' '\n \techo 4 > file4 &&\n \techo 5 > file1 &&\n \tgit add file1 file3 file4 &&\n-\ttest_must_fail git reset HEAD -- file1 file2 file3 &&\n+\tgit reset HEAD -- file1 file2 file3 &&\n+\ttest_must_fail git diff --quiet &&\n \tgit diff > output &&\n \ttest_cmp output expect &&\n \tgit diff --cached > output &&\n@@ -402,7 +403,8 @@ test_expect_success 'test resetting the index at give paths' '\n \t>sub/file2 &&\n \tgit update-index --add sub/file1 sub/file2 &&\n \tT=$(git write-tree) &&\n-\ttest_must_fail git reset HEAD sub/file2 &&\n+\tgit reset HEAD sub/file2 &&\n+\ttest_must_fail git diff --quiet &&\n \tU=$(git write-tree) &&\n \techo \"$T\" &&\n \techo \"$U\" &&\n@@ -440,7 +442,8 @@ test_expect_success 'resetting specific path that is unmerged' '\n \t\techo \"100644 $F3 3\tfile2\"\n \t} | git update-index --index-info &&\n \tgit ls-files -u &&\n-\ttest_must_fail git reset HEAD file2 &&\n+\tgit reset HEAD file2 &&\n+\ttest_must_fail git diff --quiet &&\n \tgit diff-index --exit-code --cached HEAD\n '\n \n@@ -449,7 +452,8 @@ test_expect_success 'disambiguation (1)' '\n \tgit reset --hard &&\n \t>secondfile &&\n \tgit add secondfile &&\n-\ttest_must_fail git reset secondfile &&\n+\tgit reset secondfile &&\n+\ttest_must_fail git diff --quiet -- secondfile &&\n \ttest -z \"$(git diff --cached --name-only)\" &&\n \ttest -f secondfile &&\n \ttest ! -s secondfile\n@@ -474,7 +478,8 @@ test_expect_success 'disambiguation (3)' '\n \t>secondfile &&\n \tgit add secondfile &&\n \trm -f secondfile &&\n-\ttest_must_fail git reset HEAD secondfile &&\n+\tgit reset HEAD secondfile &&\n+\ttest_must_fail git diff --quiet &&\n \ttest -z \"$(git diff --cached --name-only)\" &&\n \ttest ! -f secondfile\n \n@@ -486,7 +491,8 @@ test_expect_success 'disambiguation (4)' '\n \t>secondfile &&\n \tgit add secondfile &&\n \trm -f secondfile &&\n-\ttest_must_fail git reset -- secondfile &&\n+\tgit reset -- secondfile &&\n+\ttest_must_fail git diff --quiet &&\n \ttest -z \"$(git diff --cached --name-only)\" &&\n \ttest ! -f secondfile\n '\n-- \n1.8.1.1.454.gce43f05\n"},{"id":"206885","messageId":"1358228871-7142-4-git-send-email-martinvonz@gmail.com","threadId":"32568","inReplyTo":"1358228871-7142-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH v2 03/19] reset.c: pass pathspec around instead of (prefix, argv) pair","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-15T05:47:35Z","receivedAt":"2013-01-15T05:47:35Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"We use the path arguments in two places in reset.c: in\ninteractive_reset() and read_from_tree(). Both of these call\nget_pathspec(), so we pass the (prefix, argv) pair to both\nfunctions. Move the call to get_pathspec() out of these methods, for\ntwo reasons: 1) One argument is simpler than two. 2) It lets us use\nthe (arguably clearer) \"if (pathspec)\" in place of \"if (i < argc)\".\n\nSigned-off-by: Martin von Zweigbergk \n---\n builtin/reset.c | 27 ++++++++++-----------------\n 1 file changed, 10 insertions(+), 17 deletions(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 65413d0..045c960 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -153,26 +153,15 @@ static void update_index_from_diff(struct diff_queue_struct *q,\n \t}\n }\n \n-static int interactive_reset(const char *revision, const char **argv,\n-\t\t\t const char *prefix)\n-{\n-\tconst char **pathspec = NULL;\n-\n-\tif (*argv)\n-\t\tpathspec = get_pathspec(prefix, argv);\n-\n-\treturn run_add_interactive(revision, \"--patch=reset\", pathspec);\n-}\n-\n-static int read_from_tree(const char *prefix, const char **argv,\n-\t\tunsigned char *tree_sha1, int refresh_flags)\n+static int read_from_tree(const char **pathspec, unsigned char *tree_sha1,\n+\t\t\t int refresh_flags)\n {\n \tstruct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n \tint index_fd;\n \tstruct diff_options opt;\n \n \tmemset(&opt, 0, sizeof(opt));\n-\tdiff_tree_setup_paths(get_pathspec(prefix, (const char **)argv), &opt);\n+\tdiff_tree_setup_paths(pathspec, &opt);\n \topt.output_format = DIFF_FORMAT_CALLBACK;\n \topt.format_callback = update_index_from_diff;\n \n@@ -216,6 +205,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \tconst char *rev = \"HEAD\";\n \tunsigned char sha1[20], *orig = NULL, sha1_orig[20],\n \t\t\t\t*old_orig = NULL, sha1_old_orig[20];\n+\tconst char **pathspec = NULL;\n \tstruct commit *commit;\n \tstruct strbuf msg = STRBUF_INIT;\n \tconst struct option options[] = {\n@@ -287,22 +277,25 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\tdie(_(\"Could not parse object '%s'.\"), rev);\n \thashcpy(sha1, commit->object.sha1);\n \n+\tif (i < argc)\n+\t\tpathspec = get_pathspec(prefix, argv + i);\n+\n \tif (patch_mode) {\n \t\tif (reset_type != NONE)\n \t\t\tdie(_(\"--patch is incompatible with --{hard,mixed,soft}\"));\n-\t\treturn interactive_reset(rev, argv + i, prefix);\n+\t\treturn run_add_interactive(rev, \"--patch=reset\", pathspec);\n \t}\n \n \t/* git reset tree [--] paths... can be used to\n \t * load chosen paths from the tree into the index without\n \t * affecting the working tree nor HEAD. */\n-\tif (i < argc) {\n+\tif (pathspec) {\n \t\tif (reset_type == MIXED)\n \t\t\twarning(_(\"--mixed with paths is deprecated; use 'git reset -- ' instead.\"));\n \t\telse if (reset_type != NONE)\n \t\t\tdie(_(\"Cannot do %s reset with paths.\"),\n \t\t\t\t\t_(reset_type_names[reset_type]));\n-\t\treturn read_from_tree(prefix, argv + i, sha1,\n+\t\treturn read_from_tree(pathspec, sha1,\n \t\t\t\tquiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);\n \t}\n \tif (reset_type == NONE)\n-- \n1.8.1.1.454.gce43f05\n"},{"id":"206886","messageId":"1358228871-7142-5-git-send-email-martinvonz@gmail.com","threadId":"32568","inReplyTo":"1358228871-7142-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH v2 04/19] reset: don't allow \"git reset -- $pathspec\" in bare repo","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-15T05:47:36Z","receivedAt":"2013-01-15T05:47:36Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"Running e.g. \"git reset .\" in a bare repo results in an index file\nbeing created from the HEAD commit. The differences compared to the\nindex are then printed as usual, but since there is no worktree, it\nwill appear as if all files are deleted. For example, in a bare clone\nof git.git:\n\n Unstaged changes after reset:\n D .gitattributes\n D .gitignore\n D .mailmap\n ...\n\nThis happens because the check for is_bare_repository() happens after\nwe branch off into read_from_tree() to reset with paths. Fix by moving\nthe branching point after the check.\n\nSigned-off-by: Martin von Zweigbergk \n---\n builtin/reset.c | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 045c960..664fad9 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -295,8 +295,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\telse if (reset_type != NONE)\n \t\t\tdie(_(\"Cannot do %s reset with paths.\"),\n \t\t\t\t\t_(reset_type_names[reset_type]));\n-\t\treturn read_from_tree(pathspec, sha1,\n-\t\t\t\tquiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);\n \t}\n \tif (reset_type == NONE)\n \t\treset_type = MIXED; /* by default */\n@@ -308,6 +306,10 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\tdie(_(\"%s reset is not allowed in a bare repository\"),\n \t\t _(reset_type_names[reset_type]));\n \n+\tif (pathspec)\n+\t\treturn read_from_tree(pathspec, sha1,\n+\t\t\t\tquiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);\n+\n \t/* Soft reset does not touch the index file nor the working tree\n \t * at all, but requires them in a good order. Other resets reset\n \t * the index file to the tree object we are switching to. */\n-- \n1.8.1.1.454.gce43f05\n"},{"id":"206893","messageId":"1358228871-7142-6-git-send-email-martinvonz@gmail.com","threadId":"32568","inReplyTo":"1358228871-7142-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH v2 05/19] reset.c: extract function for parsing arguments","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-15T05:47:37Z","receivedAt":"2013-01-15T05:47:37Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"Declutter cmd_reset() a bit by moving out the argument parsing to its\nown function.\n\nSigned-off-by: Martin von Zweigbergk \n---\n builtin/reset.c | 70 +++++++++++++++++++++++++++++++--------------------------\n 1 file changed, 38 insertions(+), 32 deletions(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 664fad9..58f0f61 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -198,36 +198,11 @@ static void die_if_unmerged_cache(int reset_type)\n \n }\n \n-int cmd_reset(int argc, const char **argv, const char *prefix)\n+static const char **parse_args(int argc, const char **argv, const char *prefix, const char **rev_ret)\n {\n-\tint i = 0, reset_type = NONE, update_ref_status = 0, quiet = 0;\n-\tint patch_mode = 0;\n+\tint i = 0;\n \tconst char *rev = \"HEAD\";\n-\tunsigned char sha1[20], *orig = NULL, sha1_orig[20],\n-\t\t\t\t*old_orig = NULL, sha1_old_orig[20];\n-\tconst char **pathspec = NULL;\n-\tstruct commit *commit;\n-\tstruct strbuf msg = STRBUF_INIT;\n-\tconst struct option options[] = {\n-\t\tOPT__QUIET(&quiet, N_(\"be quiet, only report errors\")),\n-\t\tOPT_SET_INT(0, \"mixed\", &reset_type,\n-\t\t\t\t\t\tN_(\"reset HEAD and index\"), MIXED),\n-\t\tOPT_SET_INT(0, \"soft\", &reset_type, N_(\"reset only HEAD\"), SOFT),\n-\t\tOPT_SET_INT(0, \"hard\", &reset_type,\n-\t\t\t\tN_(\"reset HEAD, index and working tree\"), HARD),\n-\t\tOPT_SET_INT(0, \"merge\", &reset_type,\n-\t\t\t\tN_(\"reset HEAD, index and working tree\"), MERGE),\n-\t\tOPT_SET_INT(0, \"keep\", &reset_type,\n-\t\t\t\tN_(\"reset HEAD but keep local changes\"), KEEP),\n-\t\tOPT_BOOLEAN('p', \"patch\", &patch_mode, N_(\"select hunks interactively\")),\n-\t\tOPT_END()\n-\t};\n-\n-\tgit_config(git_default_config, NULL);\n-\n-\targc = parse_options(argc, argv, prefix, options, git_reset_usage,\n-\t\t\t\t\t\tPARSE_OPT_KEEP_DASHDASH);\n-\n+\tunsigned char unused[20];\n \t/*\n \t * Possible arguments are:\n \t *\n@@ -250,7 +225,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t * Otherwise, argv[i] could be either or and\n \t\t * has to be unambiguous.\n \t\t */\n-\t\telse if (!get_sha1_committish(argv[i], sha1)) {\n+\t\telse if (!get_sha1_committish(argv[i], unused)) {\n \t\t\t/*\n \t\t\t * Ok, argv[i] looks like a rev; it should not\n \t\t\t * be a filename.\n@@ -262,6 +237,40 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t\tverify_filename(prefix, argv[i], 1);\n \t\t}\n \t}\n+\t*rev_ret = rev;\n+\treturn i < argc ? get_pathspec(prefix, argv + i) : NULL;\n+}\n+\n+int cmd_reset(int argc, const char **argv, const char *prefix)\n+{\n+\tint reset_type = NONE, update_ref_status = 0, quiet = 0;\n+\tint patch_mode = 0;\n+\tconst char *rev;\n+\tunsigned char sha1[20], *orig = NULL, sha1_orig[20],\n+\t\t\t\t*old_orig = NULL, sha1_old_orig[20];\n+\tconst char **pathspec = NULL;\n+\tstruct commit *commit;\n+\tstruct strbuf msg = STRBUF_INIT;\n+\tconst struct option options[] = {\n+\t\tOPT__QUIET(&quiet, N_(\"be quiet, only report errors\")),\n+\t\tOPT_SET_INT(0, \"mixed\", &reset_type,\n+\t\t\t\t\t\tN_(\"reset HEAD and index\"), MIXED),\n+\t\tOPT_SET_INT(0, \"soft\", &reset_type, N_(\"reset only HEAD\"), SOFT),\n+\t\tOPT_SET_INT(0, \"hard\", &reset_type,\n+\t\t\t\tN_(\"reset HEAD, index and working tree\"), HARD),\n+\t\tOPT_SET_INT(0, \"merge\", &reset_type,\n+\t\t\t\tN_(\"reset HEAD, index and working tree\"), MERGE),\n+\t\tOPT_SET_INT(0, \"keep\", &reset_type,\n+\t\t\t\tN_(\"reset HEAD but keep local changes\"), KEEP),\n+\t\tOPT_BOOLEAN('p', \"patch\", &patch_mode, N_(\"select hunks interactively\")),\n+\t\tOPT_END()\n+\t};\n+\n+\tgit_config(git_default_config, NULL);\n+\n+\targc = parse_options(argc, argv, prefix, options, git_reset_usage,\n+\t\t\t\t\t\tPARSE_OPT_KEEP_DASHDASH);\n+\tpathspec = parse_args(argc, argv, prefix, &rev);\n \n \tif (get_sha1_committish(rev, sha1))\n \t\tdie(_(\"Failed to resolve '%s' as a valid ref.\"), rev);\n@@ -277,9 +286,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\tdie(_(\"Could not parse object '%s'.\"), rev);\n \thashcpy(sha1, commit->object.sha1);\n \n-\tif (i < argc)\n-\t\tpathspec = get_pathspec(prefix, argv + i);\n-\n \tif (patch_mode) {\n \t\tif (reset_type != NONE)\n \t\t\tdie(_(\"--patch is incompatible with --{hard,mixed,soft}\"));\n-- \n1.8.1.1.454.gce43f05\n"},{"id":"206904","messageId":"1358228871-7142-7-git-send-email-martinvonz@gmail.com","threadId":"32568","inReplyTo":"1358228871-7142-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH v2 06/19] reset.c: remove unnecessary variable 'i'","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-15T05:47:38Z","receivedAt":"2013-01-15T05:47:38Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"Throughout most of parse_args(), the variable 'i' remains at 0. Many\nreferences are still made to the variable even when it could only have\nthe value 0. This made at least me, who has relatively little\nexperience with C programming styles, think that parts of the function\nwas meant to be part of a loop. To avoid such confusion, remove the\nvariable and also the 'argc' parameter and check for NULL trailing\nargv instead.\n\nSigned-off-by: Martin von Zweigbergk \n---\nI explained a bit more why I was confused by the current style, but\nI'm also perfectly happy if you just drop the patch (there would be\nsome minor conflicts in a later patch, though).\n\n builtin/reset.c | 33 ++++++++++++++++-----------------\n 1 file changed, 16 insertions(+), 17 deletions(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 58f0f61..d89cf4d 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -198,9 +198,8 @@ static void die_if_unmerged_cache(int reset_type)\n \n }\n \n-static const char **parse_args(int argc, const char **argv, const char *prefix, const char **rev_ret)\n+static const char **parse_args(const char **argv, const char *prefix, const char **rev_ret)\n {\n-\tint i = 0;\n \tconst char *rev = \"HEAD\";\n \tunsigned char unused[20];\n \t/*\n@@ -211,34 +210,34 @@ static const char **parse_args(int argc, const char **argv, const char *prefix,\n \t * git reset [-opts] -- ...\n \t * git reset [-opts] ...\n \t *\n-\t * At this point, argv[i] points immediately after [-opts].\n+\t * At this point, argv points immediately after [-opts].\n \t */\n \n-\tif (i < argc) {\n-\t\tif (!strcmp(argv[i], \"--\")) {\n-\t\t\ti++; /* reset to HEAD, possibly with paths */\n-\t\t} else if (i + 1 < argc && !strcmp(argv[i+1], \"--\")) {\n-\t\t\trev = argv[i];\n-\t\t\ti += 2;\n+\tif (argv[0]) {\n+\t\tif (!strcmp(argv[0], \"--\")) {\n+\t\t\targv++; /* reset to HEAD, possibly with paths */\n+\t\t} else if (argv[1] && !strcmp(argv[1], \"--\")) {\n+\t\t\trev = argv[0];\n+\t\t\targv += 2;\n \t\t}\n \t\t/*\n-\t\t * Otherwise, argv[i] could be either or and\n+\t\t * Otherwise, argv[0] could be either or and\n \t\t * has to be unambiguous.\n \t\t */\n-\t\telse if (!get_sha1_committish(argv[i], unused)) {\n+\t\telse if (!get_sha1_committish(argv[0], unused)) {\n \t\t\t/*\n-\t\t\t * Ok, argv[i] looks like a rev; it should not\n+\t\t\t * Ok, argv[0] looks like a rev; it should not\n \t\t\t * be a filename.\n \t\t\t */\n-\t\t\tverify_non_filename(prefix, argv[i]);\n-\t\t\trev = argv[i++];\n+\t\t\tverify_non_filename(prefix, argv[0]);\n+\t\t\trev = *argv++;\n \t\t} else {\n \t\t\t/* Otherwise we treat this as a filename */\n-\t\t\tverify_filename(prefix, argv[i], 1);\n+\t\t\tverify_filename(prefix, argv[0], 1);\n \t\t}\n \t}\n \t*rev_ret = rev;\n-\treturn i < argc ? get_pathspec(prefix, argv + i) : NULL;\n+\treturn argv[0] ? get_pathspec(prefix, argv) : NULL;\n }\n \n int cmd_reset(int argc, const char **argv, const char *prefix)\n@@ -270,7 +269,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \n \targc = parse_options(argc, argv, prefix, options, git_reset_usage,\n \t\t\t\t\t\tPARSE_OPT_KEEP_DASHDASH);\n-\tpathspec = parse_args(argc, argv, prefix, &rev);\n+\tpathspec = parse_args(argv, prefix, &rev);\n \n \tif (get_sha1_committish(rev, sha1))\n \t\tdie(_(\"Failed to resolve '%s' as a valid ref.\"), rev);\n-- \n1.8.1.1.454.gce43f05\n"},{"id":"206898","messageId":"1358228871-7142-8-git-send-email-martinvonz@gmail.com","threadId":"32568","inReplyTo":"1358228871-7142-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH v2 07/19] reset.c: extract function for updating {ORIG_,}HEAD","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-15T05:47:39Z","receivedAt":"2013-01-15T05:47:39Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"By extracting the code for updating the HEAD and ORIG_HEAD symbolic\nreferences to a separate function, we declutter cmd_reset() a bit and\nwe make it clear that e.g. the four variables {,sha1_}{,old_}orig are\nonly used by this code.\n\nSigned-off-by: Martin von Zweigbergk \n---\n builtin/reset.c | 39 +++++++++++++++++++++++----------------\n 1 file changed, 23 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex d89cf4d..2187d64 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -240,16 +240,35 @@ static const char **parse_args(const char **argv, const char *prefix, const char\n \treturn argv[0] ? get_pathspec(prefix, argv) : NULL;\n }\n \n+static int update_refs(const char *rev, const unsigned char *sha1)\n+{\n+\tint update_ref_status;\n+\tstruct strbuf msg = STRBUF_INIT;\n+\tunsigned char *orig = NULL, sha1_orig[20],\n+\t\t*old_orig = NULL, sha1_old_orig[20];\n+\n+\tif (!get_sha1(\"ORIG_HEAD\", sha1_old_orig))\n+\t\told_orig = sha1_old_orig;\n+\tif (!get_sha1(\"HEAD\", sha1_orig)) {\n+\t\torig = sha1_orig;\n+\t\tset_reflog_message(&msg, \"updating ORIG_HEAD\", NULL);\n+\t\tupdate_ref(msg.buf, \"ORIG_HEAD\", orig, old_orig, 0, MSG_ON_ERR);\n+\t} else if (old_orig)\n+\t\tdelete_ref(\"ORIG_HEAD\", old_orig, 0);\n+\tset_reflog_message(&msg, \"updating HEAD\", rev);\n+\tupdate_ref_status = update_ref(msg.buf, \"HEAD\", sha1, orig, 0, MSG_ON_ERR);\n+\tstrbuf_release(&msg);\n+\treturn update_ref_status;\n+}\n+\n int cmd_reset(int argc, const char **argv, const char *prefix)\n {\n \tint reset_type = NONE, update_ref_status = 0, quiet = 0;\n \tint patch_mode = 0;\n \tconst char *rev;\n-\tunsigned char sha1[20], *orig = NULL, sha1_orig[20],\n-\t\t\t\t*old_orig = NULL, sha1_old_orig[20];\n+\tunsigned char sha1[20];\n \tconst char **pathspec = NULL;\n \tstruct commit *commit;\n-\tstruct strbuf msg = STRBUF_INIT;\n \tconst struct option options[] = {\n \t\tOPT__QUIET(&quiet, N_(\"be quiet, only report errors\")),\n \t\tOPT_SET_INT(0, \"mixed\", &reset_type,\n@@ -333,17 +352,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \n \t/* Any resets update HEAD to the head being switched to,\n \t * saving the previous head in ORIG_HEAD before. */\n-\tif (!get_sha1(\"ORIG_HEAD\", sha1_old_orig))\n-\t\told_orig = sha1_old_orig;\n-\tif (!get_sha1(\"HEAD\", sha1_orig)) {\n-\t\torig = sha1_orig;\n-\t\tset_reflog_message(&msg, \"updating ORIG_HEAD\", NULL);\n-\t\tupdate_ref(msg.buf, \"ORIG_HEAD\", orig, old_orig, 0, MSG_ON_ERR);\n-\t}\n-\telse if (old_orig)\n-\t\tdelete_ref(\"ORIG_HEAD\", old_orig, 0);\n-\tset_reflog_message(&msg, \"updating HEAD\", rev);\n-\tupdate_ref_status = update_ref(msg.buf, \"HEAD\", sha1, orig, 0, MSG_ON_ERR);\n+\tupdate_ref_status = update_refs(rev, sha1);\n \n \tswitch (reset_type) {\n \tcase HARD:\n@@ -360,7 +369,5 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \n \tremove_branch_state();\n \n-\tstrbuf_release(&msg);\n-\n \treturn update_ref_status;\n }\n-- \n1.8.1.1.454.gce43f05\n"},{"id":"206902","messageId":"1358228871-7142-9-git-send-email-martinvonz@gmail.com","threadId":"32568","inReplyTo":"1358228871-7142-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH v2 08/19] reset.c: share call to die_if_unmerged_cache()","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-15T05:47:40Z","receivedAt":"2013-01-15T05:47:40Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"Use a single condition to guard the call to die_if_unmerged_cache for\nboth --soft and --keep. This avoids the small distraction of the\nprecondition check from the logic following it.\n\nAlso change an instance of\n\n if (e)\n err = err || f();\n\nto the almost as short, but clearer\n\n if (e && !err)\n err = f();\n\n(which is equivalent since we only care whether exit code is 0)\n\nSigned-off-by: Martin von Zweigbergk \n---\n builtin/reset.c | 14 ++++++--------\n 1 file changed, 6 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 2187d64..4e34195 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -337,15 +337,13 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t/* Soft reset does not touch the index file nor the working tree\n \t * at all, but requires them in a good order. Other resets reset\n \t * the index file to the tree object we are switching to. */\n-\tif (reset_type == SOFT)\n+\tif (reset_type == SOFT || reset_type == KEEP)\n \t\tdie_if_unmerged_cache(reset_type);\n-\telse {\n-\t\tint err;\n-\t\tif (reset_type == KEEP)\n-\t\t\tdie_if_unmerged_cache(reset_type);\n-\t\terr = reset_index_file(sha1, reset_type, quiet);\n-\t\tif (reset_type == KEEP)\n-\t\t\terr = err || reset_index_file(sha1, MIXED, quiet);\n+\n+\tif (reset_type != SOFT) {\n+\t\tint err = reset_index_file(sha1, reset_type, quiet);\n+\t\tif (reset_type == KEEP && !err)\n+\t\t\terr = reset_index_file(sha1, MIXED, quiet);\n \t\tif (err)\n \t\t\tdie(_(\"Could not reset index file to revision '%s'.\"), rev);\n \t}\n-- \n1.8.1.1.454.gce43f05\n"},{"id":"206899","messageId":"1358228871-7142-10-git-send-email-martinvonz@gmail.com","threadId":"32568","inReplyTo":"1358228871-7142-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH v2 09/19] reset --keep: only write index file once","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-15T05:47:41Z","receivedAt":"2013-01-15T05:47:41Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"\"git reset --keep\" calls reset_index_file() twice, first doing a\ntwo-way merge to the target revision, updating the index and worktree,\nand then resetting the index. After each call, we write the index\nfile.\n\nIn the unlikely event that the second call to reset_index_file()\nfails, the index will have been merged to the target revision, but\nHEAD will not be updated, leaving the user with a dirty index.\n\nBy moving the locking, writing and committing out of\nreset_index_file() and into the caller, we can avoid writing the index\ntwice, thereby making the sure we don't end up in the half-way reset\nstate. As a bonus, we speed up \"git reset --keep\" a little on the\nlinux-2.6 repo (best of five, warm cache):\n\n Before After\nreal 0m0.315s 0m0.296s\nuser 0m0.290s 0m0.280s\nsys 0m0.020s 0m0.010s\n\nSigned-off-by: Martin von Zweigbergk \n---\n builtin/reset.c | 21 ++++++++++-----------\n 1 file changed, 10 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 4e34195..7c440ad 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -38,14 +38,12 @@ static inline int is_merge(void)\n \treturn !access(git_path(\"MERGE_HEAD\"), F_OK);\n }\n \n-static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet)\n+static int reset_index(const unsigned char *sha1, int reset_type, int quiet)\n {\n \tint nr = 1;\n-\tint newfd;\n \tstruct tree_desc desc[2];\n \tstruct tree *tree;\n \tstruct unpack_trees_options opts;\n-\tstruct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n \n \tmemset(&opts, 0, sizeof(opts));\n \topts.head_idx = 1;\n@@ -67,8 +65,6 @@ static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet\n \t\topts.reset = 1;\n \t}\n \n-\tnewfd = hold_locked_index(lock, 1);\n-\n \tread_cache_unmerged();\n \n \tif (reset_type == KEEP) {\n@@ -91,10 +87,6 @@ static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet\n \t\tprime_cache_tree(&active_cache_tree, tree);\n \t}\n \n-\tif (write_cache(newfd, active_cache, active_nr) ||\n-\t commit_locked_index(lock))\n-\t\treturn error(_(\"Could not write new index file.\"));\n-\n \treturn 0;\n }\n \n@@ -341,9 +333,16 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\tdie_if_unmerged_cache(reset_type);\n \n \tif (reset_type != SOFT) {\n-\t\tint err = reset_index_file(sha1, reset_type, quiet);\n+\t\tstruct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n+\t\tint newfd = hold_locked_index(lock, 1);\n+\t\tint err = reset_index(sha1, reset_type, quiet);\n \t\tif (reset_type == KEEP && !err)\n-\t\t\terr = reset_index_file(sha1, MIXED, quiet);\n+\t\t\terr = reset_index(sha1, MIXED, quiet);\n+\t\tif (!err &&\n+\t\t (write_cache(newfd, active_cache, active_nr) ||\n+\t\t commit_locked_index(lock))) {\n+\t\t\terr = error(_(\"Could not write new index file.\"));\n+\t\t}\n \t\tif (err)\n \t\t\tdie(_(\"Could not reset index file to revision '%s'.\"), rev);\n \t}\n-- \n1.8.1.1.454.gce43f05\n"},{"id":"206892","messageId":"1358228871-7142-11-git-send-email-martinvonz@gmail.com","threadId":"32568","inReplyTo":"1358228871-7142-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH v2 10/19] reset: avoid redundant error message","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-15T05:47:42Z","receivedAt":"2013-01-15T05:47:42Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"If writing or committing the new index file fails, we print \"Could not\nwrite new index file.\" followed by \"Could not reset index file to\nrevision $rev.\". The first message seems to imply the second, so print\nonly the first message.\n\nSigned-off-by: Martin von Zweigbergk \n---\n builtin/reset.c | 8 +++-----\n 1 file changed, 3 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 7c440ad..97fa9f7 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -338,13 +338,11 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\tint err = reset_index(sha1, reset_type, quiet);\n \t\tif (reset_type == KEEP && !err)\n \t\t\terr = reset_index(sha1, MIXED, quiet);\n-\t\tif (!err &&\n-\t\t (write_cache(newfd, active_cache, active_nr) ||\n-\t\t commit_locked_index(lock))) {\n-\t\t\terr = error(_(\"Could not write new index file.\"));\n-\t\t}\n \t\tif (err)\n \t\t\tdie(_(\"Could not reset index file to revision '%s'.\"), rev);\n+\t\tif (write_cache(newfd, active_cache, active_nr) ||\n+\t\t commit_locked_index(lock))\n+\t\t\tdie(_(\"Could not write new index file.\"));\n \t}\n \n \t/* Any resets update HEAD to the head being switched to,\n-- \n1.8.1.1.454.gce43f05\n"},{"id":"206896","messageId":"1358228871-7142-12-git-send-email-martinvonz@gmail.com","threadId":"32568","inReplyTo":"1358228871-7142-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH v2 11/19] reset.c: replace switch by if-else","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-15T05:47:43Z","receivedAt":"2013-01-15T05:47:43Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"The switch statement towards the end of reset.c is missing case arms\nfor KEEP and MERGE for no obvious reason, and soon the only non-empty\ncase arm will be the one for HARD. So let's proactively replace it by\nif-else, which will let us move one if statement out without leaving\nfunny-looking left-overs.\n\nSigned-off-by: Martin von Zweigbergk \n---\n builtin/reset.c | 13 +++----------\n 1 file changed, 3 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 97fa9f7..c3eb2eb 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -349,18 +349,11 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t * saving the previous head in ORIG_HEAD before. */\n \tupdate_ref_status = update_refs(rev, sha1);\n \n-\tswitch (reset_type) {\n-\tcase HARD:\n-\t\tif (!update_ref_status && !quiet)\n-\t\t\tprint_new_head_line(commit);\n-\t\tbreak;\n-\tcase SOFT: /* Nothing else to do. */\n-\t\tbreak;\n-\tcase MIXED: /* Report what has not been updated. */\n+\tif (reset_type == HARD && !update_ref_status && !quiet)\n+\t\tprint_new_head_line(commit);\n+\telse if (reset_type == MIXED) /* Report what has not been updated. */\n \t\tupdate_index_refresh(0, NULL,\n \t\t\t\tquiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);\n-\t\tbreak;\n-\t}\n \n \tremove_branch_state();\n \n-- \n1.8.1.1.454.gce43f05\n"},{"id":"206901","messageId":"1358228871-7142-13-git-send-email-martinvonz@gmail.com","threadId":"32568","inReplyTo":"1358228871-7142-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH v2 12/19] reset.c: move update_index_refresh() call out of read_from_tree()","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-15T05:47:44Z","receivedAt":"2013-01-15T05:47:44Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"The final part of cmd_reset() essentially looks like:\n\n if (pathspec) {\n ...\n read_from_tree(...);\n } else {\n ...\n reset_index(...);\n update_index_refresh(...);\n ...\n }\n\nwhere read_from_tree() internally also calls\nupdate_index_refresh(). Move the call to update_index_refresh() out of\nread_from_tree for symmetry with the 'else' block, making\nread_from_tree() and reset_index() closer in functionality.\n\nSigned-off-by: Martin von Zweigbergk \n---\n builtin/reset.c | 18 +++++++++---------\n 1 file changed, 9 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex c3eb2eb..70733c2 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -145,11 +145,8 @@ static void update_index_from_diff(struct diff_queue_struct *q,\n \t}\n }\n \n-static int read_from_tree(const char **pathspec, unsigned char *tree_sha1,\n-\t\t\t int refresh_flags)\n+static int read_from_tree(const char **pathspec, unsigned char *tree_sha1)\n {\n-\tstruct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n-\tint index_fd;\n \tstruct diff_options opt;\n \n \tmemset(&opt, 0, sizeof(opt));\n@@ -157,7 +154,6 @@ static int read_from_tree(const char **pathspec, unsigned char *tree_sha1,\n \topt.output_format = DIFF_FORMAT_CALLBACK;\n \topt.format_callback = update_index_from_diff;\n \n-\tindex_fd = hold_locked_index(lock, 1);\n \tread_cache();\n \tif (do_diff_cache(tree_sha1, &opt))\n \t\treturn 1;\n@@ -165,7 +161,7 @@ static int read_from_tree(const char **pathspec, unsigned char *tree_sha1,\n \tdiff_flush(&opt);\n \tdiff_tree_release_paths(&opt);\n \n-\treturn update_index_refresh(index_fd, lock, refresh_flags);\n+\treturn 0;\n }\n \n static void set_reflog_message(struct strbuf *sb, const char *action,\n@@ -322,9 +318,13 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\tdie(_(\"%s reset is not allowed in a bare repository\"),\n \t\t _(reset_type_names[reset_type]));\n \n-\tif (pathspec)\n-\t\treturn read_from_tree(pathspec, sha1,\n-\t\t\t\tquiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);\n+\tif (pathspec) {\n+\t\tstruct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n+\t\tint index_fd = hold_locked_index(lock, 1);\n+\t\treturn read_from_tree(pathspec, sha1) ||\n+\t\t\tupdate_index_refresh(index_fd, lock,\n+\t\t\t\t\tquiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);\n+\t}\n \n \t/* Soft reset does not touch the index file nor the working tree\n \t * at all, but requires them in a good order. Other resets reset\n-- \n1.8.1.1.454.gce43f05\n"},{"id":"206895","messageId":"1358228871-7142-14-git-send-email-martinvonz@gmail.com","threadId":"32568","inReplyTo":"1358228871-7142-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH v2 13/19] reset.c: move lock, write and commit out of update_index_refresh()","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-15T05:47:45Z","receivedAt":"2013-01-15T05:47:45Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"In preparation for the/a following patch, move the locking, writing\nand committing of the index file out of update_index_refresh(). The\ncode duplication caused will soon be taken care of. What remains of\nupdate_index_refresh() is just one line, but it is still called from\ntwo places, so let's leave it for now.\n\nIn the process, we expose and fix the minor UI bug that makes us print\n\"Could not refresh index\" when we fail to write the index file when\ninvoked with a pathspec. Copy the error message from the pathspec-less\ncodepath (\"Could not write new index file.\").\n\nSigned-off-by: Martin von Zweigbergk \n---\n builtin/reset.c | 34 ++++++++++++++++++----------------\n 1 file changed, 18 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 70733c2..c1d6ef2 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -109,19 +109,10 @@ static void print_new_head_line(struct commit *commit)\n \t\tprintf(\"\\n\");\n }\n \n-static int update_index_refresh(int fd, struct lock_file *index_lock, int flags)\n+static void update_index_refresh(int flags)\n {\n-\tif (!index_lock) {\n-\t\tindex_lock = xcalloc(1, sizeof(struct lock_file));\n-\t\tfd = hold_locked_index(index_lock, 1);\n-\t}\n-\n \trefresh_index(&the_index, (flags), NULL, NULL,\n \t\t _(\"Unstaged changes after reset:\"));\n-\tif (write_cache(fd, active_cache, active_nr) ||\n-\t\t\tcommit_locked_index(index_lock))\n-\t\treturn error (\"Could not refresh index\");\n-\treturn 0;\n }\n \n static void update_index_from_diff(struct diff_queue_struct *q,\n@@ -321,9 +312,14 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \tif (pathspec) {\n \t\tstruct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n \t\tint index_fd = hold_locked_index(lock, 1);\n-\t\treturn read_from_tree(pathspec, sha1) ||\n-\t\t\tupdate_index_refresh(index_fd, lock,\n-\t\t\t\t\tquiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);\n+\t\tif (read_from_tree(pathspec, sha1))\n+\t\t\treturn 1;\n+\t\tupdate_index_refresh(\n+\t\t\tquiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);\n+\t\tif (write_cache(index_fd, active_cache, active_nr) ||\n+\t\t commit_locked_index(lock))\n+\t\t\treturn error(\"Could not write new index file.\");\n+\t\treturn 0;\n \t}\n \n \t/* Soft reset does not touch the index file nor the working tree\n@@ -351,9 +347,15 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \n \tif (reset_type == HARD && !update_ref_status && !quiet)\n \t\tprint_new_head_line(commit);\n-\telse if (reset_type == MIXED) /* Report what has not been updated. */\n-\t\tupdate_index_refresh(0, NULL,\n-\t\t\t\tquiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);\n+\telse if (reset_type == MIXED) { /* Report what has not been updated. */\n+\t\tstruct lock_file *index_lock = xcalloc(1, sizeof(struct lock_file));\n+\t\tint fd = hold_locked_index(index_lock, 1);\n+\t\tupdate_index_refresh(\n+\t\t\tquiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);\n+\t\tif (write_cache(fd, active_cache, active_nr) ||\n+\t\t commit_locked_index(index_lock))\n+\t\t\terror(\"Could not refresh index\");\n+\t}\n \n \tremove_branch_state();\n \n-- \n1.8.1.1.454.gce43f05\n"},{"id":"206891","messageId":"1358228871-7142-15-git-send-email-martinvonz@gmail.com","threadId":"32568","inReplyTo":"1358228871-7142-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH v2 14/19] reset [--mixed]: only write index file once","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-15T05:47:46Z","receivedAt":"2013-01-15T05:47:46Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"When doing a mixed reset without paths, the index is locked, read,\nreset, and written back as part of the actual reset operation (in\nreset_index()). Then, when showing the list of worktree modifications,\nwe lock the index again, refresh it, and write it.\n\nChange this so we only write the index once, making \"git reset\" a\nlittle faster. It does mean that the index lock will be held a little\nlonger, but the difference is small compared to the time spent\nrefreshing the index.\n\nThere is one minor functional difference: We used to say \"Could not\nwrite new index file.\" if the first write failed, and \"Could not\nrefresh index\" if the second write failed. Now, we will only use the\nfirst message.\n\nThis speeds up \"git reset\" a little on the linux-2.6 repo (best of\nfive, warm cache):\n\n Before After\nreal 0m0.239s 0m0.214s\nuser 0m0.160s 0m0.130s\nsys 0m0.070s 0m0.080s\n\nSigned-off-by: Martin von Zweigbergk \n---\n builtin/reset.c | 14 +++++---------\n 1 file changed, 5 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex c1d6ef2..e8a3e41 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -336,6 +336,11 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t\terr = reset_index(sha1, MIXED, quiet);\n \t\tif (err)\n \t\t\tdie(_(\"Could not reset index file to revision '%s'.\"), rev);\n+\n+\t\tif (reset_type == MIXED) /* Report what has not been updated. */\n+\t\t\tupdate_index_refresh(\n+\t\t\t\tquiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);\n+\n \t\tif (write_cache(newfd, active_cache, active_nr) ||\n \t\t commit_locked_index(lock))\n \t\t\tdie(_(\"Could not write new index file.\"));\n@@ -347,15 +352,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \n \tif (reset_type == HARD && !update_ref_status && !quiet)\n \t\tprint_new_head_line(commit);\n-\telse if (reset_type == MIXED) { /* Report what has not been updated. */\n-\t\tstruct lock_file *index_lock = xcalloc(1, sizeof(struct lock_file));\n-\t\tint fd = hold_locked_index(index_lock, 1);\n-\t\tupdate_index_refresh(\n-\t\t\tquiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);\n-\t\tif (write_cache(fd, active_cache, active_nr) ||\n-\t\t commit_locked_index(index_lock))\n-\t\t\terror(\"Could not refresh index\");\n-\t}\n \n \tremove_branch_state();\n \n-- \n1.8.1.1.454.gce43f05\n"},{"id":"206890","messageId":"1358228871-7142-16-git-send-email-martinvonz@gmail.com","threadId":"32568","inReplyTo":"1358228871-7142-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH v2 15/19] reset.c: finish entire cmd_reset() whether or not pathspec is given","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-15T05:47:47Z","receivedAt":"2013-01-15T05:47:47Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"By not returning from inside the \"if (pathspec)\" block, we can let the\npathspec-aware and pathspec-less code share a bit more, making it\neasier to make future changes that should affect both cases. This also\nhighlights the similarity between read_from_tree() and reset_index().\n\nSigned-off-by: Martin von Zweigbergk \n---\n builtin/reset.c | 42 ++++++++++++++++++------------------------\n 1 file changed, 18 insertions(+), 24 deletions(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex e8a3e41..c316d9b 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -309,19 +309,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\tdie(_(\"%s reset is not allowed in a bare repository\"),\n \t\t _(reset_type_names[reset_type]));\n \n-\tif (pathspec) {\n-\t\tstruct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n-\t\tint index_fd = hold_locked_index(lock, 1);\n-\t\tif (read_from_tree(pathspec, sha1))\n-\t\t\treturn 1;\n-\t\tupdate_index_refresh(\n-\t\t\tquiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);\n-\t\tif (write_cache(index_fd, active_cache, active_nr) ||\n-\t\t commit_locked_index(lock))\n-\t\t\treturn error(\"Could not write new index file.\");\n-\t\treturn 0;\n-\t}\n-\n \t/* Soft reset does not touch the index file nor the working tree\n \t * at all, but requires them in a good order. Other resets reset\n \t * the index file to the tree object we are switching to. */\n@@ -331,11 +318,16 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \tif (reset_type != SOFT) {\n \t\tstruct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n \t\tint newfd = hold_locked_index(lock, 1);\n-\t\tint err = reset_index(sha1, reset_type, quiet);\n-\t\tif (reset_type == KEEP && !err)\n-\t\t\terr = reset_index(sha1, MIXED, quiet);\n-\t\tif (err)\n-\t\t\tdie(_(\"Could not reset index file to revision '%s'.\"), rev);\n+\t\tif (pathspec) {\n+\t\t\tif (read_from_tree(pathspec, sha1))\n+\t\t\t\treturn 1;\n+\t\t} else {\n+\t\t\tint err = reset_index(sha1, reset_type, quiet);\n+\t\t\tif (reset_type == KEEP && !err)\n+\t\t\t\terr = reset_index(sha1, MIXED, quiet);\n+\t\t\tif (err)\n+\t\t\t\tdie(_(\"Could not reset index file to revision '%s'.\"), rev);\n+\t\t}\n \n \t\tif (reset_type == MIXED) /* Report what has not been updated. */\n \t\t\tupdate_index_refresh(\n@@ -346,14 +338,16 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t\tdie(_(\"Could not write new index file.\"));\n \t}\n \n-\t/* Any resets update HEAD to the head being switched to,\n-\t * saving the previous head in ORIG_HEAD before. */\n-\tupdate_ref_status = update_refs(rev, sha1);\n+\tif (!pathspec) {\n+\t\t/* Any resets without paths update HEAD to the head being\n+\t\t * switched to, saving the previous head in ORIG_HEAD before. */\n+\t\tupdate_ref_status = update_refs(rev, sha1);\n \n-\tif (reset_type == HARD && !update_ref_status && !quiet)\n-\t\tprint_new_head_line(commit);\n+\t\tif (reset_type == HARD && !update_ref_status && !quiet)\n+\t\t\tprint_new_head_line(commit);\n \n-\tremove_branch_state();\n+\t\tremove_branch_state();\n+\t}\n \n \treturn update_ref_status;\n }\n-- \n1.8.1.1.454.gce43f05\n"},{"id":"206897","messageId":"1358228871-7142-17-git-send-email-martinvonz@gmail.com","threadId":"32568","inReplyTo":"1358228871-7142-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH v2 16/19] reset.c: inline update_index_refresh()","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-15T05:47:48Z","receivedAt":"2013-01-15T05:47:48Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"Now that there is only one caller left to the single-line method\nupdate_index_refresh(), inline it.\n\nSigned-off-by: Martin von Zweigbergk \n---\n builtin/reset.c | 14 +++++---------\n 1 file changed, 5 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex c316d9b..520c1a5 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -109,12 +109,6 @@ static void print_new_head_line(struct commit *commit)\n \t\tprintf(\"\\n\");\n }\n \n-static void update_index_refresh(int flags)\n-{\n-\trefresh_index(&the_index, (flags), NULL, NULL,\n-\t\t _(\"Unstaged changes after reset:\"));\n-}\n-\n static void update_index_from_diff(struct diff_queue_struct *q,\n \t\tstruct diff_options *opt, void *data)\n {\n@@ -329,9 +323,11 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t\t\tdie(_(\"Could not reset index file to revision '%s'.\"), rev);\n \t\t}\n \n-\t\tif (reset_type == MIXED) /* Report what has not been updated. */\n-\t\t\tupdate_index_refresh(\n-\t\t\t\tquiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);\n+\t\tif (reset_type == MIXED) { /* Report what has not been updated. */\n+\t\t\tint flags = quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN;\n+\t\t\trefresh_index(&the_index, flags, NULL, NULL,\n+\t\t\t\t _(\"Unstaged changes after reset:\"));\n+\t\t}\n \n \t\tif (write_cache(newfd, active_cache, active_nr) ||\n \t\t commit_locked_index(lock))\n-- \n1.8.1.1.454.gce43f05\n"},{"id":"206887","messageId":"1358228871-7142-18-git-send-email-martinvonz@gmail.com","threadId":"32568","inReplyTo":"1358228871-7142-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH v2 17/19] reset $sha1 $pathspec: require $sha1 only to be treeish","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-15T05:47:49Z","receivedAt":"2013-01-15T05:47:49Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"Resetting with paths does not update HEAD and there is nothing else\nthat a commit should be needed for. Relax the argument parsing so only\na tree is required.\n\nThe sha1 is only passed to read_from_tree(), which already only\nrequires a tree.\n\nThe \"rev\" variable we pass to run_add_interactive() will resolve to a\ntree. This is fine since interactive_reset only needs the parameter to\nbe a treeish and doesn't use it for display purposes.\n\nSigned-off-by: Martin von Zweigbergk \n---\n builtin/reset.c | 48 +++++++++++++++++++++++++++---------------------\n t/t7102-reset.sh | 8 ++++++++\n 2 files changed, 35 insertions(+), 21 deletions(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 520c1a5..b776867 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -178,9 +178,10 @@ static const char **parse_args(const char **argv, const char *prefix, const char\n \t/*\n \t * Possible arguments are:\n \t *\n-\t * git reset [-opts] ...\n-\t * git reset [-opts] -- ...\n-\t * git reset [-opts] -- ...\n+\t * git reset [-opts] []\n+\t * git reset [-opts] [...]\n+\t * git reset [-opts] -- [...]\n+\t * git reset [-opts] -- [...]\n \t * git reset [-opts] ...\n \t *\n \t * At this point, argv points immediately after [-opts].\n@@ -195,11 +196,13 @@ static const char **parse_args(const char **argv, const char *prefix, const char\n \t\t}\n \t\t/*\n \t\t * Otherwise, argv[0] could be either or and\n-\t\t * has to be unambiguous.\n+\t\t * has to be unambiguous. If there is a single argument, it\n+\t\t * can not be a tree\n \t\t */\n-\t\telse if (!get_sha1_committish(argv[0], unused)) {\n+\t\telse if ((!argv[1] && !get_sha1_committish(argv[0], unused)) ||\n+\t\t\t (argv[1] && !get_sha1_treeish(argv[0], unused))) {\n \t\t\t/*\n-\t\t\t * Ok, argv[0] looks like a rev; it should not\n+\t\t\t * Ok, argv[0] looks like a commit/tree; it should not\n \t\t\t * be a filename.\n \t\t\t */\n \t\t\tverify_non_filename(prefix, argv[0]);\n@@ -241,7 +244,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \tconst char *rev;\n \tunsigned char sha1[20];\n \tconst char **pathspec = NULL;\n-\tstruct commit *commit;\n \tconst struct option options[] = {\n \t\tOPT__QUIET(&quiet, N_(\"be quiet, only report errors\")),\n \t\tOPT_SET_INT(0, \"mixed\", &reset_type,\n@@ -263,19 +265,23 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t\tPARSE_OPT_KEEP_DASHDASH);\n \tpathspec = parse_args(argv, prefix, &rev);\n \n-\tif (get_sha1_committish(rev, sha1))\n-\t\tdie(_(\"Failed to resolve '%s' as a valid ref.\"), rev);\n-\n-\t/*\n-\t * NOTE: As \"git reset $treeish -- $path\" should be usable on\n-\t * any tree-ish, this is not strictly correct. We are not\n-\t * moving the HEAD to any commit; we are merely resetting the\n-\t * entries in the index to that of a treeish.\n-\t */\n-\tcommit = lookup_commit_reference(sha1);\n-\tif (!commit)\n-\t\tdie(_(\"Could not parse object '%s'.\"), rev);\n-\thashcpy(sha1, commit->object.sha1);\n+\tif (!pathspec) {\n+\t\tstruct commit *commit;\n+\t\tif (get_sha1_committish(rev, sha1))\n+\t\t\tdie(_(\"Failed to resolve '%s' as a valid revision.\"), rev);\n+\t\tcommit = lookup_commit_reference(sha1);\n+\t\tif (!commit)\n+\t\t\tdie(_(\"Could not parse object '%s'.\"), rev);\n+\t\thashcpy(sha1, commit->object.sha1);\n+\t} else {\n+\t\tstruct tree *tree;\n+\t\tif (get_sha1_treeish(rev, sha1))\n+\t\t\tdie(_(\"Failed to resolve '%s' as a valid tree.\"), rev);\n+\t\ttree = parse_tree_indirect(sha1);\n+\t\tif (!tree)\n+\t\t\tdie(_(\"Could not parse object '%s'.\"), rev);\n+\t\thashcpy(sha1, tree->object.sha1);\n+\t}\n \n \tif (patch_mode) {\n \t\tif (reset_type != NONE)\n@@ -340,7 +346,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\tupdate_ref_status = update_refs(rev, sha1);\n \n \t\tif (reset_type == HARD && !update_ref_status && !quiet)\n-\t\t\tprint_new_head_line(commit);\n+\t\t\tprint_new_head_line(lookup_commit_reference(sha1));\n \n \t\tremove_branch_state();\n \t}\ndiff --git a/t/t7102-reset.sh b/t/t7102-reset.sh\nindex 81b2570..1fa2a5f 100755\n--- a/t/t7102-reset.sh\n+++ b/t/t7102-reset.sh\n@@ -497,4 +497,12 @@ test_expect_success 'disambiguation (4)' '\n \ttest ! -f secondfile\n '\n \n+test_expect_success 'reset with paths accepts tree' '\n+\t# for simpler tests, drop last commit containing added files\n+\tgit reset --hard HEAD^ &&\n+\tgit reset HEAD^^{tree} -- . &&\n+\tgit diff --cached HEAD^ --exit-code &&\n+\tgit diff HEAD --exit-code\n+'\n+\n test_done\n-- \n1.8.1.1.454.gce43f05\n"},{"id":"206894","messageId":"1358228871-7142-19-git-send-email-martinvonz@gmail.com","threadId":"32568","inReplyTo":"1358228871-7142-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH v2 18/19] reset: allow reset on unborn branch","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-15T05:47:50Z","receivedAt":"2013-01-15T05:47:50Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"Some users seem to think, knowingly or not, that being on an unborn\nbranch is like having a commit with an empty tree checked out, but\nwhen run on an unborn branch, \"git reset\" currently fails with:\n\n fatal: Failed to resolve 'HEAD' as a valid ref.\n\nInstead of making users figure out that they should run\n\n git rm --cached -r .\n\n, let's teach \"git reset\" without a revision argument, when on an\nunborn branch, to behave as if the user asked to reset to an empty\ntree. Don't take the analogy with an empty commit too far, though, but\nstill disallow explictly referring to HEAD in \"git reset HEAD\".\n\nSigned-off-by: Martin von Zweigbergk \n---\n builtin/reset.c | 16 ++++++++-----\n t/t7106-reset-unborn-branch.sh | 52 ++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 62 insertions(+), 6 deletions(-)\n create mode 100755 t/t7106-reset-unborn-branch.sh\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex b776867..45b01eb 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -240,7 +240,7 @@ static int update_refs(const char *rev, const unsigned char *sha1)\n int cmd_reset(int argc, const char **argv, const char *prefix)\n {\n \tint reset_type = NONE, update_ref_status = 0, quiet = 0;\n-\tint patch_mode = 0;\n+\tint patch_mode = 0, unborn;\n \tconst char *rev;\n \tunsigned char sha1[20];\n \tconst char **pathspec = NULL;\n@@ -265,7 +265,11 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t\tPARSE_OPT_KEEP_DASHDASH);\n \tpathspec = parse_args(argv, prefix, &rev);\n \n-\tif (!pathspec) {\n+\tunborn = !strcmp(rev, \"HEAD\") && get_sha1(\"HEAD\", sha1);\n+\tif (unborn) {\n+\t\t/* reset on unborn branch: treat as reset to empty tree */\n+\t\thashcpy(sha1, EMPTY_TREE_SHA1_BIN);\n+\t} else if (!pathspec) {\n \t\tstruct commit *commit;\n \t\tif (get_sha1_committish(rev, sha1))\n \t\t\tdie(_(\"Failed to resolve '%s' as a valid revision.\"), rev);\n@@ -286,7 +290,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \tif (patch_mode) {\n \t\tif (reset_type != NONE)\n \t\t\tdie(_(\"--patch is incompatible with --{hard,mixed,soft}\"));\n-\t\treturn run_add_interactive(rev, \"--patch=reset\", pathspec);\n+\t\treturn run_add_interactive(sha1_to_hex(sha1), \"--patch=reset\", pathspec);\n \t}\n \n \t/* git reset tree [--] paths... can be used to\n@@ -340,16 +344,16 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t\tdie(_(\"Could not write new index file.\"));\n \t}\n \n-\tif (!pathspec) {\n+\tif (!pathspec && !unborn) {\n \t\t/* Any resets without paths update HEAD to the head being\n \t\t * switched to, saving the previous head in ORIG_HEAD before. */\n \t\tupdate_ref_status = update_refs(rev, sha1);\n \n \t\tif (reset_type == HARD && !update_ref_status && !quiet)\n \t\t\tprint_new_head_line(lookup_commit_reference(sha1));\n-\n-\t\tremove_branch_state();\n \t}\n+\tif (!pathspec)\n+\t\tremove_branch_state();\n \n \treturn update_ref_status;\n }\ndiff --git a/t/t7106-reset-unborn-branch.sh b/t/t7106-reset-unborn-branch.sh\nnew file mode 100755\nindex 0000000..8062cf5\n--- /dev/null\n+++ b/t/t7106-reset-unborn-branch.sh\n@@ -0,0 +1,52 @@\n+#!/bin/sh\n+\n+test_description='git reset should work on unborn branch'\n+. ./test-lib.sh\n+\n+test_expect_success 'setup' '\n+\techo a >a &&\n+\techo b >b\n+'\n+\n+test_expect_success 'reset' '\n+\tgit add a b &&\n+\tgit reset &&\n+\ttest \"$(git ls-files)\" = \"\"\n+'\n+\n+test_expect_success 'reset HEAD' '\n+\trm .git/index &&\n+\tgit add a b &&\n+\ttest_must_fail git reset HEAD\n+'\n+\n+test_expect_success 'reset $file' '\n+\trm .git/index &&\n+\tgit add a b &&\n+\tgit reset a &&\n+\ttest \"$(git ls-files)\" = \"b\"\n+'\n+\n+test_expect_success 'reset -p' '\n+\trm .git/index &&\n+\tgit add a &&\n+\techo y | git reset -p &&\n+\ttest \"$(git ls-files)\" = \"\"\n+'\n+\n+test_expect_success 'reset --soft is a no-op' '\n+\trm .git/index &&\n+\tgit add a &&\n+\tgit reset --soft\n+\ttest \"$(git ls-files)\" = \"a\"\n+'\n+\n+test_expect_success 'reset --hard' '\n+\trm .git/index &&\n+\tgit add a &&\n+\tgit reset --hard &&\n+\ttest \"$(git ls-files)\" = \"\" &&\n+\ttest_path_is_missing a\n+'\n+\n+test_done\n-- \n1.8.1.1.454.gce43f05\n"},{"id":"206889","messageId":"1358228871-7142-20-git-send-email-martinvonz@gmail.com","threadId":"32568","inReplyTo":"1358228871-7142-1-git-send-email-martinvonz@gmail.com","subject":"[PATCH v2 19/19] reset [--mixed]: use diff-based reset whether or not pathspec was given","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-15T05:47:51Z","receivedAt":"2013-01-15T05:47:51Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"Thanks to b65982b (Optimize \"diff-index --cached\" using cache-tree,\n2009-05-20), resetting with paths is much faster than resetting\nwithout paths. Some timings for the linux-2.6 repo to illustrate this\n(best of five, warm cache):\n\n reset reset .\nreal 0m0.219s 0m0.080s\nuser 0m0.140s 0m0.040s\nsys 0m0.070s 0m0.030s\n\nThese two commands should do the same thing, so instead of having the\nuser type the trailing \" .\" to get the faster do_diff_cache()-based\nimplementation, always use it when doing a mixed reset, with or\nwithout paths (so \"git reset $rev\" would also be faster).\n\nTiming \"git reset\" shows that it indeed becomes as fast as\n\"git reset .\" after this patch.\n\nSigned-off-by: Martin von Zweigbergk \n---\nIt seems like a better solution would be for unpack_trees() learn the\nsame tricks as do_diff_cache(). I'm leaving that a challange for the\nreader :-). I did have a look a unpack_trees(), but it looked rather\noverwhelming.\n\n builtin/reset.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 45b01eb..921afbe 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -322,7 +322,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \tif (reset_type != SOFT) {\n \t\tstruct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n \t\tint newfd = hold_locked_index(lock, 1);\n-\t\tif (pathspec) {\n+\t\tif (reset_type == MIXED) {\n \t\t\tif (read_from_tree(pathspec, sha1))\n \t\t\t\treturn 1;\n \t\t} else {\n-- \n1.8.1.1.454.gce43f05\n"},{"id":"206957","messageId":"CANiSa6i6p98Kjc=+4hjag46Oby-_aPuHqjPukUDsVULbjkPCpw@mail.gmail.com","threadId":"32568","inReplyTo":"A5E8E180685CEF45AB9E737A010799805E00DD@cdnz-ex1.corp.cubic.cub","subject":"Re: [PATCH v2 06/19] reset.c: remove unnecessary variable 'i'","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-15T18:36:40Z","receivedAt":"2013-01-15T18:36:40Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"I suppose this was meant for everyone. Adding back the others.\n\nOn Tue, Jan 15, 2013 at 10:27 AM, Holding, Lawrence (NZ)\n wrote:\n> Maybe use *argv instead of argv[0]?\n\nSure. Everywhere? Also in the lines added in patch 17/19 that refer to\nboth argv[0] and argv[1], such as \"argv[1] &&\n!get_sha1_treeish(argv[0], unused)\"? Or is this just a sign that I'm\nmaking the code _more_ confusing to those who are more familiar with\nC?\n"},{"id":"207079","messageId":"1358359235-10213-1-git-send-email-martinvonz@gmail.com","threadId":"32568","inReplyTo":"1358228871-7142-18-git-send-email-martinvonz@gmail.com","subject":"[PATCH v2 17/19] fixup! reset $sha1 $pathspec: require $sha1 only to be treeish","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-16T18:00:35Z","receivedAt":"2013-01-16T18:00:35Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"---\n\nSorry, I forgot the documentation updates. I hope this looks ok. Can\nyou squash this in, Junio? Thanks.\n\nI don't think any documentation update is necessary for the \"reset on\nunborn branch\" patch. Let me know if you think differently.\n\n\n Documentation/git-reset.txt | 18 +++++++++---------\n builtin/reset.c | 4 ++--\n 2 files changed, 11 insertions(+), 11 deletions(-)\n\ndiff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt\nindex 978d8da..a404b47 100644\n--- a/Documentation/git-reset.txt\n+++ b/Documentation/git-reset.txt\n@@ -8,20 +8,20 @@ git-reset - Reset current HEAD to the specified state\n SYNOPSIS\n --------\n [verse]\n-'git reset' [-q] [] [--] ...\n-'git reset' (--patch | -p) [] [--] [...]\n+'git reset' [-q] [] [--] ...\n+'git reset' (--patch | -p) [] [--] [...]\n 'git reset' [--soft | --mixed | --hard | --merge | --keep] [-q] []\n \n DESCRIPTION\n -----------\n-In the first and second form, copy entries from to the index.\n+In the first and second form, copy entries from to the index.\n In the third form, set the current branch head (HEAD) to , optionally\n-modifying index and working tree to match. The defaults to HEAD\n-in all forms.\n+modifying index and working tree to match. The / defaults\n+to HEAD in all forms.\n \n-'git reset' [-q] [] [--] ...::\n+'git reset' [-q] [] [--] ...::\n \tThis form resets the index entries for all to their\n-\tstate at . (It does not affect the working tree, nor\n+\tstate at . (It does not affect the working tree, nor\n \tthe current branch.)\n +\n This means that `git reset ` is the opposite of `git add\n@@ -34,9 +34,9 @@ Alternatively, using linkgit:git-checkout[1] and specifying a commit, you\n can copy the contents of a path out of a commit to the index and to the\n working tree in one go.\n \n-'git reset' (--patch | -p) [] [--] [...]::\n+'git reset' (--patch | -p) [] [--] [...]::\n \tInteractively select hunks in the difference between the index\n-\tand (defaults to HEAD). The chosen hunks are applied\n+\tand (defaults to HEAD). The chosen hunks are applied\n \tin reverse to the index.\n +\n This means that `git reset -p` is the opposite of `git add -p`, i.e.\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex b776867..cb84f1b 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -23,8 +23,8 @@\n \n static const char * const git_reset_usage[] = {\n \tN_(\"git reset [--mixed | --soft | --hard | --merge | --keep] [-q] []\"),\n-\tN_(\"git reset [-q] [--] ...\"),\n-\tN_(\"git reset --patch [] [--] [...]\"),\n+\tN_(\"git reset [-q] [--] ...\"),\n+\tN_(\"git reset --patch [] [--] [...]\"),\n \tNULL\n };\n \n-- \n1.8.1.1.454.gce43f05\n"},{"id":"207083","messageId":"CANiSa6hyWGyn5-UGF6_3KPw7ut6TOqZU+b=xLH7DBpa+tW8sZw@mail.gmail.com","threadId":"32568","inReplyTo":"1358359235-10213-1-git-send-email-martinvonz@gmail.com","subject":"Re: [PATCH v2 17/19] fixup! reset $sha1 $pathspec: require $sha1 only to be treeish","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-01-16T18:08:59Z","receivedAt":"2013-01-16T18:08:59Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"On Wed, Jan 16, 2013 at 10:00 AM, Martin von Zweigbergk\n wrote:\n> ---\n>\n> Sorry, I forgot the documentation updates. I hope this looks ok. Can\n> you squash this in, Junio? Thanks.\n\nI see the series just entered 'next', so I guess it would have to go\non top then. Perhaps with a commit message like as simple as the\nfollowing. Let me know if you prefer it to be resent as a proper\npatch. Sorry about the noise.\n\nreset: update documentation to require only tree-ish with paths\n\nWhen resetting with paths, we no longer require a commit argument, but\nonly a tree-ish. Update the documentation and synopsis accordingly.\n"}]}