Re: [PATCH 15/19] reset.c: finish entire cmd_reset() whether or not pathspec is given
Martin von Zweigbergk 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
[PATCH 15/19] reset.c: finish entire cmd_reset() whether or not pathspec is given
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
