{"thread":{"id":"34128","subject":"[PATCH] reset: trivial refactoring","startedAt":"2013-06-13T18:15:05Z","lastAt":"2013-06-13T21:34:13Z","messageCount":4,"participants":["Felipe Contreras","Martin von Zweigbergk","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"220755","messageId":"1371147305-20771-1-git-send-email-felipe.contreras@gmail.com","threadId":"34128","inReplyTo":null,"subject":"[PATCH] reset: trivial refactoring","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-06-13T18:15:05Z","receivedAt":"2013-06-13T18:15:05Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"After commit 3fde386 (reset [--mixed]: use diff-based reset whether or\nnot pathspec was given), some code can't be reached, and other code can\nbe moved to the 'reset_type == MIXED' check.\n\nLet's remove the check that can't be reached, and move the code is\nspecific to MIXED.\n\nSigned-off-by: Felipe Contreras \n---\n builtin/reset.c | 11 ++++-------\n 1 file changed, 4 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 6032131..68739ba 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -82,7 +82,7 @@ static int reset_index(const unsigned char *sha1, int reset_type, int quiet)\n \tif (unpack_trees(nr, desc, &opts))\n \t\treturn -1;\n \n-\tif (reset_type == MIXED || reset_type == HARD) {\n+\tif (reset_type == HARD) {\n \t\ttree = parse_tree_indirect(sha1);\n \t\tprime_cache_tree(&active_cache_tree, tree);\n \t}\n@@ -323,8 +323,11 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\tstruct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n \t\tint newfd = hold_locked_index(lock, 1);\n \t\tif (reset_type == MIXED) {\n+\t\t\tint flags = quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN;\n \t\t\tif (read_from_tree(pathspec, sha1))\n \t\t\t\treturn 1;\n+\t\t\trefresh_index(&the_index, flags, NULL, NULL,\n+\t\t\t\t _(\"Unstaged changes after reset:\"));\n \t\t} else {\n \t\t\tint err = reset_index(sha1, reset_type, quiet);\n \t\t\tif (reset_type == KEEP && !err)\n@@ -333,12 +336,6 @@ 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\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 \t\t\tdie(_(\"Could not write new index file.\"));\n-- \n1.8.3.698.g079b096\n"},{"id":"220771","messageId":"CANiSa6i5xL-V+W46zWXGzLc+dSO6a+V0BLPt7hFgGZGXpp03BQ@mail.gmail.com","threadId":"34128","inReplyTo":"1371147305-20771-1-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH] reset: trivial refactoring","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-06-13T21:13:17Z","receivedAt":"2013-06-13T21:13:17Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"On Thu, Jun 13, 2013 at 11:15 AM, Felipe Contreras\n wrote:\n> @@ -82,7 +82,7 @@ static int reset_index(const unsigned char *sha1, int reset_type, int quiet)\n> if (unpack_trees(nr, desc, &opts))\n> return -1;\n>\n> - if (reset_type == MIXED || reset_type == HARD) {\n> + if (reset_type == HARD) {\n\nAre you sure that this can not be reached given that...\n\n> @@ -323,8 +323,11 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n> struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n> int newfd = hold_locked_index(lock, 1);\n> if (reset_type == MIXED) {\n> + int flags = quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN;\n> if (read_from_tree(pathspec, sha1))\n> return 1;\n> + refresh_index(&the_index, flags, NULL, NULL,\n> + _(\"Unstaged changes after reset:\"));\n> } else {\n> int err = reset_index(sha1, reset_type, quiet);\n> if (reset_type == KEEP && !err)\n\n...the line after this one reads\n\n err = reset_index(sha1, MIXED, quiet);\n\n? I don't know what the consequence of not calling prime_cache_tree()\nwould be, though.\n\nThe merging of the two if blocks looks good. Thanks.\n"},{"id":"220772","messageId":"CAMP44s17nRzh157hyLMjKWi3ZOLqUui=uMVMwyffcGFFrucd_w@mail.gmail.com","threadId":"34128","inReplyTo":"CANiSa6i5xL-V+W46zWXGzLc+dSO6a+V0BLPt7hFgGZGXpp03BQ@mail.gmail.com","subject":"Re: [PATCH] reset: trivial refactoring","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-06-13T21:20:22Z","receivedAt":"2013-06-13T21:20:22Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, Jun 13, 2013 at 4:13 PM, Martin von Zweigbergk\n wrote:\n> On Thu, Jun 13, 2013 at 11:15 AM, Felipe Contreras\n> wrote:\n>> @@ -82,7 +82,7 @@ static int reset_index(const unsigned char *sha1, int reset_type, int quiet)\n>> if (unpack_trees(nr, desc, &opts))\n>> return -1;\n>>\n>> - if (reset_type == MIXED || reset_type == HARD) {\n>> + if (reset_type == HARD) {\n>\n> Are you sure that this can not be reached given that...\n>\n>> @@ -323,8 +323,11 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n>> struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n>> int newfd = hold_locked_index(lock, 1);\n>> if (reset_type == MIXED) {\n>> + int flags = quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN;\n>> if (read_from_tree(pathspec, sha1))\n>> return 1;\n>> + refresh_index(&the_index, flags, NULL, NULL,\n>> + _(\"Unstaged changes after reset:\"));\n>> } else {\n>> int err = reset_index(sha1, reset_type, quiet);\n>> if (reset_type == KEEP && !err)\n>\n> ...the line after this one reads\n>\n> err = reset_index(sha1, MIXED, quiet);\n\nThat's true. Only the rest of the patch makes sense then. It seems\nthere should be a way to have a single call reset_index(KEEP), so we\ndon't have to call again with MIXED, but perhaps that's for later.\n\n-- \nFelipe Contreras\n"},{"id":"220773","messageId":"7v38slu23u.fsf@alter.siamese.dyndns.org","threadId":"34128","inReplyTo":"CANiSa6i5xL-V+W46zWXGzLc+dSO6a+V0BLPt7hFgGZGXpp03BQ@mail.gmail.com","subject":"Re: [PATCH] reset: trivial refactoring","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-13T21:34:13Z","receivedAt":"2013-06-13T21:34:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk writes:\n\n> ...the line after this one reads\n>\n> err = reset_index(sha1, MIXED, quiet);\n>\n> ? I don't know what the consequence of not calling prime_cache_tree()\n> would be, though.\n\nIt does not affect correctness, but makes the subsequent \"git\nstatus\", the part that internally computes \"diff-cache --index\" to\nsee what changes have been added to the index, more costly.\n\nAfter doing \"reset --hard $commit\" or just \"reset $commit\", we know\nthat the contents of the index must match $commit^{tree}, and\nwriting out any subpart of the index that corresponds to a directory\n(including the top-level one, i.e. the whole index) must match the\ncorresponding subtree of $commit^{tree}. And that is why we prime\nthe cache-tree that was discarded by unpack_trees() at the very end.\nThen incremental \"git add\" to the resulting index after that can\ninvalidate only the parts of the index and cache-tree while\nrelieving the next \"write-tree\" (most often done by the next \"git\ncommit\") from having to compute the tree objects for parts of the\nindex that have not been touched since the \"reset\" operation.\n\nI do not use \"reset --keep $commit\" very often myself, but IIRC, it\nis like \"checkout $commit\" (and not \"checkout -m $commit\") in that\nthe resulting index matches $commit^{tree}, so I think priming the\ncache-tree just like --hard/--mixed is the right thing to do.\n"}]}