{"thread":{"id":"61701","subject":"[PATCH] submodule--helper: use strvec_pushf() for --super-prefix","startedAt":"2024-06-29T18:01:27Z","lastAt":"2024-07-01T03:42:21Z","messageCount":2,"participants":["René Scharfe","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"497846","messageId":"fb79ebc4-5ecf-4257-ac2e-39f98db5649c@web.de","threadId":"61701","inReplyTo":null,"subject":"[PATCH] submodule--helper: use strvec_pushf() for --super-prefix","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2024-06-29T18:01:24Z","receivedAt":"2024-06-29T18:01:27Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Use the strvec_pushf() call that already appends a slash to also produce\nthe stuck form of the option --super-prefix instead of adding the option\nname in a separate call of strvec_push() or strvec_pushl().  This way we\ncan more easily see that these parts make up a single option with its\nargument and save a function call.\n\nSigned-off-by: René Scharfe <l.s.r@web.de>\n---\n builtin/submodule--helper.c | 17 ++++++-----------\n 1 file changed, 6 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 880ab4456e..f1218a1995 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -376,8 +376,7 @@ static void runcommand_in_submodule_cb(const struct cache_entry *list_item,\n\n \t\tstrvec_pushl(&cpr.args, \"submodule--helper\", \"foreach\", \"--recursive\",\n \t\t\t     NULL);\n-\t\tstrvec_pushl(&cpr.args, \"--super-prefix\", NULL);\n-\t\tstrvec_pushf(&cpr.args, \"%s/\", displaypath);\n+\t\tstrvec_pushf(&cpr.args, \"--super-prefix=%s/\", displaypath);\n\n \t\tif (info->quiet)\n \t\t\tstrvec_push(&cpr.args, \"--quiet\");\n@@ -702,8 +701,7 @@ static void status_submodule(const char *path, const struct object_id *ce_oid,\n\n \t\tstrvec_pushl(&cpr.args, \"submodule--helper\", \"status\",\n \t\t\t     \"--recursive\", NULL);\n-\t\tstrvec_push(&cpr.args, \"--super-prefix\");\n-\t\tstrvec_pushf(&cpr.args, \"%s/\", displaypath);\n+\t\tstrvec_pushf(&cpr.args, \"--super-prefix=%s/\", displaypath);\n\n \t\tif (flags & OPT_CACHED)\n \t\t\tstrvec_push(&cpr.args, \"--cached\");\n@@ -1304,9 +1302,7 @@ static void sync_submodule(const char *path, const char *prefix,\n\n \t\tstrvec_pushl(&cpr.args, \"submodule--helper\", \"sync\",\n \t\t\t     \"--recursive\", NULL);\n-\t\tstrvec_push(&cpr.args, \"--super-prefix\");\n-\t\tstrvec_pushf(&cpr.args, \"%s/\", displaypath);\n-\n+\t\tstrvec_pushf(&cpr.args, \"--super-prefix=%s/\", displaypath);\n\n \t\tif (flags & OPT_QUIET)\n \t\t\tstrvec_push(&cpr.args, \"--quiet\");\n@@ -2534,10 +2530,9 @@ static void update_data_to_args(const struct update_data *update_data,\n \tenum submodule_update_type update_type = update_data->update_default;\n\n \tstrvec_pushl(args, \"submodule--helper\", \"update\", \"--recursive\", NULL);\n-\tif (update_data->displaypath) {\n-\t\tstrvec_push(args, \"--super-prefix\");\n-\t\tstrvec_pushf(args, \"%s/\", update_data->displaypath);\n-\t}\n+\tif (update_data->displaypath)\n+\t\tstrvec_pushf(args, \"--super-prefix=%s/\",\n+\t\t\t     update_data->displaypath);\n \tstrvec_pushf(args, \"--jobs=%d\", update_data->max_jobs);\n \tif (update_data->quiet)\n \t\tstrvec_push(args, \"--quiet\");\n--\n2.45.2\n"},{"id":"497873","messageId":"20240701034220.GD610406@coredump.intra.peff.net","threadId":"61701","inReplyTo":"fb79ebc4-5ecf-4257-ac2e-39f98db5649c@web.de","subject":"Re: [PATCH] submodule--helper: use strvec_pushf() for --super-prefix","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-07-01T03:42:20Z","receivedAt":"2024-07-01T03:42:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jun 29, 2024 at 08:01:24PM +0200, René Scharfe wrote:\n\n> Use the strvec_pushf() call that already appends a slash to also produce\n> the stuck form of the option --super-prefix instead of adding the option\n> name in a separate call of strvec_push() or strvec_pushl().  This way we\n> can more easily see that these parts make up a single option with its\n> argument and save a function call.\n\nI agree it's more readable (both the code and any GIT_TRACE output). But\nalso I think the \"stuck\" form is what we recommend in gitcli(7), since\nit is future-proof against optional arguments. So we'd also be setting a\ngood example.\n\n-Peff\n"}]}