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

Reply via email to