[PATCH] submodule--helper: use strvec_pushf() for --super-prefix

René Scharfe· Jun 29, 2024, 18:01 UTC · lore

Use the strvec_pushf() call that already appends a slash to also produce the stuck form of the option --super-prefix instead of adding the option name in a separate call of strvec_push() or strvec_pushl(). This way we can more easily see that these parts make up a single option with its argument and save a function call.

Signed-off-by: René Scharfe <l.s.r@web.de>
---
 builtin/submodule--helper.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

Show changes to builtin/submodule--helper.c +6 −12

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 880ab4456e..f1218a1995 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -376,8 +376,7 @@ static void runcommand_in_submodule_cb(const struct cache_entry *list_item,

 		strvec_pushl(&cpr.args, "submodule--helper", "foreach", "--recursive",
 			     NULL);
-		strvec_pushl(&cpr.args, "--super-prefix", NULL);
-		strvec_pushf(&cpr.args, "%s/", displaypath);
+		strvec_pushf(&cpr.args, "--super-prefix=%s/", displaypath);

 		if (info->quiet)
 			strvec_push(&cpr.args, "--quiet");
@@ -702,8 +701,7 @@ static void status_submodule(const char *path, const struct object_id *ce_oid,

 		strvec_pushl(&cpr.args, "submodule--helper", "status",
 			     "--recursive", NULL);
-		strvec_push(&cpr.args, "--super-prefix");
-		strvec_pushf(&cpr.args, "%s/", displaypath);
+		strvec_pushf(&cpr.args, "--super-prefix=%s/", displaypath);

 		if (flags & OPT_CACHED)
 			strvec_push(&cpr.args, "--cached");
@@ -1304,9 +1302,7 @@ static void sync_submodule(const char *path, const char *prefix,

 		strvec_pushl(&cpr.args, "submodule--helper", "sync",
 			     "--recursive", NULL);
-		strvec_push(&cpr.args, "--super-prefix");
-		strvec_pushf(&cpr.args, "%s/", displaypath);
-
+		strvec_pushf(&cpr.args, "--super-prefix=%s/", displaypath);

 		if (flags & OPT_QUIET)
 			strvec_push(&cpr.args, "--quiet");
@@ -2534,10 +2530,9 @@ static void update_data_to_args(const struct update_data *update_data,
 	enum submodule_update_type update_type = update_data->update_default;

 	strvec_pushl(args, "submodule--helper", "update", "--recursive", NULL);
-	if (update_data->displaypath) {
-		strvec_push(args, "--super-prefix");
-		strvec_pushf(args, "%s/", update_data->displaypath);
-	}
+	if (update_data->displaypath)
+		strvec_pushf(args, "--super-prefix=%s/",
+			     update_data->displaypath);
 	strvec_pushf(args, "--jobs=%d", update_data->max_jobs);
 	if (update_data->quiet)
 		strvec_push(args, "--quiet");
--
2.45.2

Jeff King· Jul 1, 2024, 03:42 UTC · re: René Scharfe · lore

Re: [PATCH] submodule--helper: use strvec_pushf() for --super-prefix

On Sat, Jun 29, 2024 at 08:01:24PM +0200, René Scharfe wrote:

Show 5 quoted lines

> Use the strvec_pushf() call that already appends a slash to also produce
> the stuck form of the option --super-prefix instead of adding the option
> name in a separate call of strvec_push() or strvec_pushl().  This way we
> can more easily see that these parts make up a single option with its
> argument and save a function call.

I agree it's more readable (both the code and any GIT_TRACE output). But also I think the "stuck" form is what we recommend in gitcli(7), since it is future-proof against optional arguments. So we'd also be setting a good example.

-Peff

patchsubmodule--helper: use strvec_pushf() for --super-prefix