[PATCH 15/19] reset.c: finish entire cmd_reset() whether or not pathspec is given

2013-01-09 Thread Martin von Zweigbergk 
By not returning from inside the if (pathspec) block, we can let the
pathspec-aware and pathspec-less code share a bit more, making it
easier to make future changes that should affect both cases. This also
highlights the similarity between read_from_tree() and reset_index().
---
Should error reporting be aligned too? Speaking of which,
do_diff_cache() never returns anything by 0. Is the return value for
future-proofing?

 builtin/reset.c | 42 ++
 1 file changed, 18 insertions(+), 24 deletions(-)

diff --git a/builtin/reset.c b/builtin/reset.c
index 254afa9..9bcad29 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -308,19 +308,6 @@ int cmd_reset(int argc, const char **argv, const char 
*prefix)
die(_(%s reset is not allowed in a bare repository),
_(reset_type_names[reset_type]));
 
-   if (pathspec) {
-   struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
-   int index_fd = hold_locked_index(lock, 1);
-   if (read_from_tree(pathspec, sha1))
-   return 1;
-   update_index_refresh(
-   quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
-   if (write_cache(index_fd, active_cache, active_nr) ||
-   commit_locked_index(lock))
-   return error(Could not write new index file.);
-   return 0;
-   }
-
/* Soft reset does not touch the index file nor the working tree
 * at all, but requires them in a good order.  Other resets reset
 * the index file to the tree object we are switching to. */
@@ -330,11 +317,16 @@ int cmd_reset(int argc, const char **argv, const char 
*prefix)
if (reset_type != SOFT) {
struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
int newfd = hold_locked_index(lock, 1);
-   int err = reset_index(sha1, reset_type, quiet);
-   if (reset_type == KEEP  !err)
-   err = reset_index(sha1, MIXED, quiet);
-   if (err)
-   die(_(Could not reset index file to revision '%s'.), 
rev);
+   if (pathspec) {
+   if (read_from_tree(pathspec, sha1))
+   return 1;
+   } else {
+   int err = reset_index(sha1, reset_type, quiet);
+   if (reset_type == KEEP  !err)
+   err = reset_index(sha1, MIXED, quiet);
+   if (err)
+   die(_(Could not reset index file to revision 
'%s'.), rev);
+   }
 
if (reset_type == MIXED) /* Report what has not been updated. */
update_index_refresh(
@@ -345,14 +337,16 @@ int cmd_reset(int argc, const char **argv, const char 
*prefix)
die(_(Could not write new index file.));
}
 
-   /* Any resets update HEAD to the head being switched to,
-* saving the previous head in ORIG_HEAD before. */
-   update_ref_status = update_refs(rev, sha1);
+   if (!pathspec) {
+   /* Any resets without paths update HEAD to the head being
+* switched to, saving the previous head in ORIG_HEAD before. */
+   update_ref_status = update_refs(rev, sha1);
 
-   if (reset_type == HARD  !update_ref_status  !quiet)
-   print_new_head_line(commit);
+   if (reset_type == HARD  !update_ref_status  !quiet)
+   print_new_head_line(commit);
 
-   remove_branch_state();
+   remove_branch_state();
+   }
 
return update_ref_status;
 }
-- 
1.8.1.rc3.331.g1ef2165

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 15/19] reset.c: finish entire cmd_reset() whether or not pathspec is given

2013-01-09 Thread Junio C Hamano 
Martin von Zweigbergk martinv...@gmail.com writes:

 By not returning from inside the if (pathspec) block, we can let the
 pathspec-aware and pathspec-less code share a bit more, making it
 easier to make future changes that should affect both cases. This also
 highlights the similarity between read_from_tree() and reset_index().
 ---
 Should error reporting be aligned too? Speaking of which,
 do_diff_cache() never returns anything by 0. Is the return value for
 future-proofing?

Perhaps, and yes.


  builtin/reset.c | 42 ++
  1 file changed, 18 insertions(+), 24 deletions(-)

 diff --git a/builtin/reset.c b/builtin/reset.c
 index 254afa9..9bcad29 100644
 --- a/builtin/reset.c
 +++ b/builtin/reset.c
 @@ -308,19 +308,6 @@ int cmd_reset(int argc, const char **argv, const char 
 *prefix)
   die(_(%s reset is not allowed in a bare repository),
   _(reset_type_names[reset_type]));
  
 - if (pathspec) {
 - struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
 - int index_fd = hold_locked_index(lock, 1);
 - if (read_from_tree(pathspec, sha1))
 - return 1;
 - update_index_refresh(
 - quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
 - if (write_cache(index_fd, active_cache, active_nr) ||
 - commit_locked_index(lock))
 - return error(Could not write new index file.);
 - return 0;
 - }
 -
   /* Soft reset does not touch the index file nor the working tree
* at all, but requires them in a good order.  Other resets reset
* the index file to the tree object we are switching to. */
 @@ -330,11 +317,16 @@ int cmd_reset(int argc, const char **argv, const char 
 *prefix)
   if (reset_type != SOFT) {
   struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
   int newfd = hold_locked_index(lock, 1);
 - int err = reset_index(sha1, reset_type, quiet);
 - if (reset_type == KEEP  !err)
 - err = reset_index(sha1, MIXED, quiet);
 - if (err)
 - die(_(Could not reset index file to revision '%s'.), 
 rev);
 + if (pathspec) {
 + if (read_from_tree(pathspec, sha1))
 + return 1;
 + } else {
 + int err = reset_index(sha1, reset_type, quiet);
 + if (reset_type == KEEP  !err)
 + err = reset_index(sha1, MIXED, quiet);
 + if (err)
 + die(_(Could not reset index file to revision 
 '%s'.), rev);
 + }
  
   if (reset_type == MIXED) /* Report what has not been updated. */
   update_index_refresh(
 @@ -345,14 +337,16 @@ int cmd_reset(int argc, const char **argv, const char 
 *prefix)
   die(_(Could not write new index file.));
   }
  
 - /* Any resets update HEAD to the head being switched to,
 -  * saving the previous head in ORIG_HEAD before. */
 - update_ref_status = update_refs(rev, sha1);
 + if (!pathspec) {
 + /* Any resets without paths update HEAD to the head being
 +  * switched to, saving the previous head in ORIG_HEAD before. */
 + update_ref_status = update_refs(rev, sha1);
  
 - if (reset_type == HARD  !update_ref_status  !quiet)
 - print_new_head_line(commit);
 + if (reset_type == HARD  !update_ref_status  !quiet)
 + print_new_head_line(commit);
  
 - remove_branch_state();
 + remove_branch_state();
 + }
  
   return update_ref_status;
  }
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html