[1/8] merge: remove always-the-same "verbose" arguments

Commit Message

René Scharfe Oct. 27, 2022, 4:35 p.m. UTC

From: Ævar Arnfjörð Bjarmason <[e-mail]>

Simplify the code that builds the arguments for the "read-tree"
invocation in reset_hard() and read_empty() to remove the "verbose"
parameter.

Before 172b6428d06 (do not overwrite untracked during merge from
unborn branch, 2010-11-14) there was a "reset_hard()" function that
would be called in two places, one of those passed a "verbose=1", the
other a "verbose=0".

After 172b6428d06 when read_empty() was split off from reset_hard()
both of these functions only had one caller. The "verbose" in
read_empty() would always be false, and the one in reset_hard() would
always be true.

There was never a good reason for the code to act this way, it
happened because the read_empty() function was a copy/pasted and
adjusted version of reset_hard().

Since we're no longer conditionally adding the "-v" parameter
here (and we'd only add it for "reset_hard()" we'll be able to move to
a simpler and safer run-command API in the subsequent commit.

Signed-off-by: Ævar Arnfjörð Bjarmason <[e-mail]>
Signed-off-by: René Scharfe <[e-mail]>
---
 builtin/merge.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

--
2.38.1

Comments

Taylor Blau Oct. 29, 2022, 6:12 p.m. UTC | #1

On Thu, Oct 27, 2022 at 06:35:02PM +0200, René Scharfe wrote:
> From: Ævar Arnfjörð Bjarmason <[e-mail]>
>
> Simplify the code that builds the arguments for the "read-tree"
> invocation in reset_hard() and read_empty() to remove the "verbose"
> parameter.
>
> Before 172b6428d06 (do not overwrite untracked during merge from
> unborn branch, 2010-11-14) there was a "reset_hard()" function that
> would be called in two places, one of those passed a "verbose=1", the
> other a "verbose=0".
>
> After 172b6428d06 when read_empty() was split off from reset_hard()
> both of these functions only had one caller. The "verbose" in
> read_empty() would always be false, and the one in reset_hard() would
> always be true.
>
> There was never a good reason for the code to act this way, it
> happened because the read_empty() function was a copy/pasted and
> adjusted version of reset_hard().
>
> Since we're no longer conditionally adding the "-v" parameter
> here (and we'd only add it for "reset_hard()" we'll be able to move to
> a simpler and safer run-command API in the subsequent commit.

All looks reasonable here. Thanks for the detailed explanation of when
and why these changed and at which point they became redundant.

Thanks,
Taylor

Patch

diff --git a/builtin/merge.c b/builtin/merge.c
index 5900b81729..3bb49d805b 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -345,14 +345,12 @@  static int save_state(struct object_id *stash)
 	return rc;
 }

-static void read_empty(const struct object_id *oid, int verbose)
+static void read_empty(const struct object_id *oid)
 {
 	int i = 0;
 	const char *args[7];

 	args[i++] = "read-tree";
-	if (verbose)
-		args[i++] = "-v";
 	args[i++] = "-m";
 	args[i++] = "-u";
 	args[i++] = empty_tree_oid_hex();
@@ -363,14 +361,13 @@  static void read_empty(const struct object_id *oid, int verbose)
 		die(_("read-tree failed"));
 }

-static void reset_hard(const struct object_id *oid, int verbose)
+static void reset_hard(const struct object_id *oid)
 {
 	int i = 0;
 	const char *args[6];

 	args[i++] = "read-tree";
-	if (verbose)
-		args[i++] = "-v";
+	args[i++] = "-v";
 	args[i++] = "--reset";
 	args[i++] = "-u";
 	args[i++] = oid_to_hex(oid);
@@ -385,7 +382,7 @@  static void restore_state(const struct object_id *head,
 {
 	struct strvec args = STRVEC_INIT;

-	reset_hard(head, 1);
+	reset_hard(head);

 	if (is_null_oid(stash))
 		goto refresh_cache;
@@ -1470,7 +1467,7 @@  int cmd_merge(int argc, const char **argv, const char *prefix)
 					       check_trust_level);

 		remote_head_oid = &remoteheads->item->object.oid;
-		read_empty(remote_head_oid, 0);
+		read_empty(remote_head_oid);
 		update_ref("initial pull", "HEAD", remote_head_oid, NULL, 0,
 			   UPDATE_REFS_DIE_ON_ERR);
 		goto done;