Re: [PATCH 08/19] reset.c: share call to die_if_unmerged_cache()

[Martin von Zweigbergk]

On Wed, Jan 9, 2013 at 11:48 AM, Junio C Hamano gits...@pobox.com wrote:
 Martin von Zweigbergk martinv...@gmail.com writes:

 Use a single condition to guard the call to die_if_unmerged_cache for
 both --soft and --keep. This avoids the small distraction of the
 precondition check from the logic following it.

 Also change an instance of

   if (e)
 err = err || f();

 to the almost as short, but clearer

   if (e  !err)
 err = f();

 (which is equivalent since we only care whether exit code is 0)

 It is not just equivalent, but should give us identical result, even
 if we cared the actual value.

If err is initially 0, and f() evaluates to 2, err would be 1 in the
first case, but 2 in the second case, right?

I think the two might be identical in e.g. JavaScript and Python, but
I don't use either much.
--
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 08/19] reset.c: share call to die_if_unmerged_cache()

[Martin von Zweigbergk]

Use a single condition to guard the call to die_if_unmerged_cache for
both --soft and --keep. This avoids the small distraction of the
precondition check from the logic following it.

Also change an instance of

  if (e)
err = err || f();

to the almost as short, but clearer

  if (e  !err)
err = f();

(which is equivalent since we only care whether exit code is 0)
---
 builtin/reset.c | 14 ++
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/builtin/reset.c b/builtin/reset.c
index 4d556e7..42d1563 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -336,15 +336,13 @@ int cmd_reset(int argc, const char **argv, const char 
*prefix)
/* 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. */
-   if (reset_type == SOFT)
+   if (reset_type == SOFT || reset_type == KEEP)
die_if_unmerged_cache(reset_type);
-   else {
-   int err;
-   if (reset_type == KEEP)
-   die_if_unmerged_cache(reset_type);
-   err = reset_index_file(sha1, reset_type, quiet);
-   if (reset_type == KEEP)
-   err = err || reset_index_file(sha1, MIXED, quiet);
+
+   if (reset_type != SOFT) {
+   int err = reset_index_file(sha1, reset_type, quiet);
+   if (reset_type == KEEP  !err)
+   err = reset_index_file(sha1, MIXED, quiet);
if (err)
die(_(Could not reset index file to revision '%s'.), 
rev);
}
-- 
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 08/19] reset.c: share call to die_if_unmerged_cache()

[Junio C Hamano]

Martin von Zweigbergk martinv...@gmail.com writes:

 Use a single condition to guard the call to die_if_unmerged_cache for
 both --soft and --keep. This avoids the small distraction of the
 precondition check from the logic following it.

 Also change an instance of

   if (e)
 err = err || f();

 to the almost as short, but clearer

   if (e  !err)
 err = f();

 (which is equivalent since we only care whether exit code is 0)

It is not just equivalent, but should give us identical result, even
if we cared the actual value.

And I tend to agree that the latter is more readable, especially
when f() can be longer, which is often the case in real life.

Happy to see this change.

 ---
  builtin/reset.c | 14 ++
  1 file changed, 6 insertions(+), 8 deletions(-)

 diff --git a/builtin/reset.c b/builtin/reset.c
 index 4d556e7..42d1563 100644
 --- a/builtin/reset.c
 +++ b/builtin/reset.c
 @@ -336,15 +336,13 @@ int cmd_reset(int argc, const char **argv, const char 
 *prefix)
   /* 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. */
 - if (reset_type == SOFT)
 + if (reset_type == SOFT || reset_type == KEEP)
   die_if_unmerged_cache(reset_type);
 - else {
 - int err;
 - if (reset_type == KEEP)
 - die_if_unmerged_cache(reset_type);
 - err = reset_index_file(sha1, reset_type, quiet);
 - if (reset_type == KEEP)
 - err = err || reset_index_file(sha1, MIXED, quiet);
 +
 + if (reset_type != SOFT) {
 + int err = reset_index_file(sha1, reset_type, quiet);
 + if (reset_type == KEEP  !err)
 + err = reset_index_file(sha1, MIXED, quiet);
   if (err)
   die(_(Could not reset index file to revision '%s'.), 
 rev);
   }
--
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