diff mbox series

[v7,03/20] submodule--helper: allow setting superprefix for init_submodule()

Message ID 20220210092833.55360-4-chooglen@google.com (mailing list archive)
State Accepted
Commit 3ce52cba5b283af1f5dbebfe43aeea5d3421dac6
Headers show
Series submodule: convert the rest of 'update' to C | expand

Commit Message

Glen Choo Feb. 10, 2022, 9:28 a.m. UTC
From: Atharva Raykar <raykar.ath@gmail.com>

We allow callers of the `init_submodule()` function to optionally
override the superprefix from the environment.

We need to enable this option because in our conversion of the update
command that will follow, the '--init' option will be handled through
this API. We will need to change the superprefix at that time to ensure
the display paths show correctly in the output messages.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Shourya Shukla <periperidip@gmail.com>
Signed-off-by: Atharva Raykar <raykar.ath@gmail.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/submodule--helper.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Comments

Ævar Arnfjörð Bjarmason Feb. 12, 2022, 2:30 p.m. UTC | #1
On Thu, Feb 10 2022, Glen Choo wrote:

> From: Atharva Raykar <raykar.ath@gmail.com>
>
> We allow callers of the `init_submodule()` function to optionally
> override the superprefix from the environment.
>
> We need to enable this option because in our conversion of the update
> command that will follow, the '--init' option will be handled through
> this API. We will need to change the superprefix at that time to ensure
> the display paths show correctly in the output messages.
>
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by: Shourya Shukla <periperidip@gmail.com>
> Signed-off-by: Atharva Raykar <raykar.ath@gmail.com>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  builtin/submodule--helper.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 5efceb9d46..09cda67c1e 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -606,18 +606,22 @@ static int module_foreach(int argc, const char **argv, const char *prefix)
>  
>  struct init_cb {
>  	const char *prefix;
> +	const char *superprefix;
>  	unsigned int flags;
>  };
>  #define INIT_CB_INIT { 0 }
>  
>  static void init_submodule(const char *path, const char *prefix,
> -			   unsigned int flags)
> +			   const char *superprefix, unsigned int flags)
>  {
>  	const struct submodule *sub;
>  	struct strbuf sb = STRBUF_INIT;
>  	char *upd = NULL, *url = NULL, *displaypath;
>  
> -	displaypath = get_submodule_displaypath(path, prefix);
> +	/* try superprefix from the environment, if it is not passed explicitly */
> +	if (!superprefix)
> +		superprefix = get_super_prefix();
> +	displaypath = do_get_submodule_displaypath(path, prefix, superprefix);
>  
>  	sub = submodule_from_path(the_repository, null_oid(), path);
>  
> @@ -691,7 +695,7 @@ static void init_submodule(const char *path, const char *prefix,
>  static void init_submodule_cb(const struct cache_entry *list_item, void *cb_data)
>  {
>  	struct init_cb *info = cb_data;
> -	init_submodule(list_item->name, info->prefix, info->flags);
> +	init_submodule(list_item->name, info->prefix, info->superprefix, info->flags);
>  }
>  
>  static int module_init(int argc, const char **argv, const char *prefix)

Note/nit on existing (pre this series) code, I wonder why we ended up
with this init_submodule() v.s. init_submodule_cb() indirection,
v.s. just doing:

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 09cda67c1ea..aa82abeb37a 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -611,9 +611,14 @@ struct init_cb {
 };
 #define INIT_CB_INIT { 0 }
 
-static void init_submodule(const char *path, const char *prefix,
-			   const char *superprefix, unsigned int flags)
+static void init_submodule_cb(const struct cache_entry *list_item, void *cb_data)
 {
+	const char *path = list_item->name;
+	struct init_cb *info = cb_data;
+	const char *prefix = info->prefix;
+	const char *superprefix = info->superprefix ? info->superprefix :
+		get_super_prefix();
+	unsigned int flags = info->flags;
 	const struct submodule *sub;
 	struct strbuf sb = STRBUF_INIT;
 	char *upd = NULL, *url = NULL, *displaypath;
@@ -692,12 +697,6 @@ static void init_submodule(const char *path, const char *prefix,
 	free(upd);
 }
 
-static void init_submodule_cb(const struct cache_entry *list_item, void *cb_data)
-{
-	struct init_cb *info = cb_data;
-	init_submodule(list_item->name, info->prefix, info->superprefix, info->flags);
-}
-
 static int module_init(int argc, const char **argv, const char *prefix)
 {
 	struct init_cb info = INIT_CB_INIT;

Maybe it's worth it to declare the variables in the argument list
v.s. the function.
diff mbox series

Patch

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 5efceb9d46..09cda67c1e 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -606,18 +606,22 @@  static int module_foreach(int argc, const char **argv, const char *prefix)
 
 struct init_cb {
 	const char *prefix;
+	const char *superprefix;
 	unsigned int flags;
 };
 #define INIT_CB_INIT { 0 }
 
 static void init_submodule(const char *path, const char *prefix,
-			   unsigned int flags)
+			   const char *superprefix, unsigned int flags)
 {
 	const struct submodule *sub;
 	struct strbuf sb = STRBUF_INIT;
 	char *upd = NULL, *url = NULL, *displaypath;
 
-	displaypath = get_submodule_displaypath(path, prefix);
+	/* try superprefix from the environment, if it is not passed explicitly */
+	if (!superprefix)
+		superprefix = get_super_prefix();
+	displaypath = do_get_submodule_displaypath(path, prefix, superprefix);
 
 	sub = submodule_from_path(the_repository, null_oid(), path);
 
@@ -691,7 +695,7 @@  static void init_submodule(const char *path, const char *prefix,
 static void init_submodule_cb(const struct cache_entry *list_item, void *cb_data)
 {
 	struct init_cb *info = cb_data;
-	init_submodule(list_item->name, info->prefix, info->flags);
+	init_submodule(list_item->name, info->prefix, info->superprefix, info->flags);
 }
 
 static int module_init(int argc, const char **argv, const char *prefix)