[GSoC][PATCH 1/8] submodule--helper: introduce get_submodule_displaypath()

[Prathamesh Chavan]

Introduce function get_submodule_displaypath() to replace the code
occurring in submodule_init() for generating displaypath of the
submodule with a call to it.

This new function will also be used in other parts of the system
in later patches.

Mentored-by: Christian Couder 
Mentored-by: Stefan Beller 
Signed-off-by: Prathamesh Chavan 
---
 builtin/submodule--helper.c | 33 ++---
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 6abdad329..7af4de09b 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -220,6 +220,27 @@ static int resolve_relative_url_test(int argc, const char 
**argv, const char *pr
return 0;
 }
 
+static char *get_submodule_displaypath(const char *path, const char *prefix)
+{
+   const char *super_prefix = get_super_prefix();
+
+   if (prefix && super_prefix) {
+   BUG("cannot have prefix '%s' and superprefix '%s'",
+   prefix, super_prefix);
+   } else if (prefix) {
+   struct strbuf sb = STRBUF_INIT;
+   char *displaypath = xstrdup(relative_path(path, prefix, ));
+   strbuf_release();
+   return displaypath;
+   } else if (super_prefix) {
+   int len = strlen(super_prefix);
+   const char *format = is_dir_sep(super_prefix[len - 1]) ? "%s%s" 
: "%s/%s";
+   return xstrfmt(format, super_prefix, path);
+   } else {
+   return xstrdup(path);
+   }
+}
+
 struct module_list {
const struct cache_entry **entries;
int alloc, nr;
@@ -339,16 +360,7 @@ static void init_submodule(const char *path, const char 
*prefix, int quiet)
 
/* Only loads from .gitmodules, no overlay with .git/config */
gitmodules_config();
-
-   if (prefix && get_super_prefix())
-   die("BUG: cannot have prefix and superprefix");
-   else if (prefix)
-   displaypath = xstrdup(relative_path(path, prefix, ));
-   else if (get_super_prefix()) {
-   strbuf_addf(, "%s%s", get_super_prefix(), path);
-   displaypath = strbuf_detach(, NULL);
-   } else
-   displaypath = xstrdup(path);
+   displaypath = get_submodule_displaypath(path, prefix);
 
sub = submodule_from_path(null_sha1, path);
 
@@ -363,7 +375,6 @@ static void init_submodule(const char *path, const char 
*prefix, int quiet)
 * Set active flag for the submodule being initialized
 */
if (!is_submodule_active(the_repository, path)) {
-   strbuf_reset();
strbuf_addf(, "submodule.%s.active", sub->name);
git_config_set_gently(sb.buf, "true");
}
-- 
2.13.0

Re: [GSoC][PATCH 1/8] submodule--helper: introduce get_submodule_displaypath()

[Brandon Williams]

On 07/10, Stefan Beller wrote:
> On Mon, Jul 10, 2017 at 4:32 PM, Brandon Williams  wrote:
> >>   if (!is_submodule_active(the_repository, path)) {
> >> - strbuf_reset();
> >
> > Is this line removal intended?  It doesn't look related to the rest of
> > this patch.
> 
> It is, as  is re-used and has to be cleared first.
> With the code above removed,  is unmodified since
> struct strbuf sb = STRBUF_INIT; hence the removal is ok here.
> It is related, but only when looking at the entirety of the code. :-/

Ah I see. Thanks!

-- 
Brandon Williams

Re: [GSoC][PATCH 1/8] submodule--helper: introduce get_submodule_displaypath()

[Stefan Beller]

On Mon, Jul 10, 2017 at 4:32 PM, Brandon Williams  wrote:
>>   if (!is_submodule_active(the_repository, path)) {
>> - strbuf_reset();
>
> Is this line removal intended?  It doesn't look related to the rest of
> this patch.

It is, as  is re-used and has to be cleared first.
With the code above removed,  is unmodified since
struct strbuf sb = STRBUF_INIT; hence the removal is ok here.
It is related, but only when looking at the entirety of the code. :-/

Re: [GSoC][PATCH 1/8] submodule--helper: introduce get_submodule_displaypath()

[Brandon Williams]

On 07/11, Prathamesh Chavan wrote:
> Introduce function get_submodule_displaypath() to replace the code
> occurring in submodule_init() for generating displaypath of the
> submodule with a call to it.
> 
> This new function will also be used in other parts of the system
> in later patches.
> 
> Mentored-by: Christian Couder 
> Mentored-by: Stefan Beller 
> Signed-off-by: Prathamesh Chavan 
> ---
>  builtin/submodule--helper.c | 33 ++---
>  1 file changed, 22 insertions(+), 11 deletions(-)
> 
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 6abdad329..7af4de09b 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -220,6 +220,27 @@ static int resolve_relative_url_test(int argc, const 
> char **argv, const char *pr
>   return 0;
>  }
>  
> +static char *get_submodule_displaypath(const char *path, const char *prefix)
> +{
> + const char *super_prefix = get_super_prefix();
> +
> + if (prefix && super_prefix) {
> + BUG("cannot have prefix '%s' and superprefix '%s'",
> + prefix, super_prefix);
> + } else if (prefix) {
> + struct strbuf sb = STRBUF_INIT;
> + char *displaypath = xstrdup(relative_path(path, prefix, ));
> + strbuf_release();
> + return displaypath;
> + } else if (super_prefix) {
> + int len = strlen(super_prefix);
> + const char *format = is_dir_sep(super_prefix[len - 1]) ? "%s%s" 
> : "%s/%s";
> + return xstrfmt(format, super_prefix, path);
> + } else {
> + return xstrdup(path);
> + }
> +}
> +
>  struct module_list {
>   const struct cache_entry **entries;
>   int alloc, nr;
> @@ -339,16 +360,7 @@ static void init_submodule(const char *path, const char 
> *prefix, int quiet)
>  
>   /* Only loads from .gitmodules, no overlay with .git/config */
>   gitmodules_config();
> -
> - if (prefix && get_super_prefix())
> - die("BUG: cannot have prefix and superprefix");
> - else if (prefix)
> - displaypath = xstrdup(relative_path(path, prefix, ));
> - else if (get_super_prefix()) {
> - strbuf_addf(, "%s%s", get_super_prefix(), path);
> - displaypath = strbuf_detach(, NULL);
> - } else
> - displaypath = xstrdup(path);
> + displaypath = get_submodule_displaypath(path, prefix);
>  
>   sub = submodule_from_path(null_sha1, path);
>  
> @@ -363,7 +375,6 @@ static void init_submodule(const char *path, const char 
> *prefix, int quiet)
>* Set active flag for the submodule being initialized
>*/
>   if (!is_submodule_active(the_repository, path)) {
> - strbuf_reset();

Is this line removal intended?  It doesn't look related to the rest of
this patch.

>   strbuf_addf(, "submodule.%s.active", sub->name);
>   git_config_set_gently(sb.buf, "true");
>   }
> -- 
> 2.13.0
> 

-- 
Brandon Williams

[GSoC][PATCH 1/8] submodule--helper: introduce get_submodule_displaypath()

[Prathamesh Chavan]

Introduce function get_submodule_displaypath() to replace the code
occurring in submodule_init() for generating displaypath of the
submodule with a call to it.

This new function will also be used in other parts of the system
in later patches.

Mentored-by: Christian Couder 
Mentored-by: Stefan Beller 
Signed-off-by: Prathamesh Chavan 
---
 builtin/submodule--helper.c | 33 ++---
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 6abdad329..7af4de09b 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -220,6 +220,27 @@ static int resolve_relative_url_test(int argc, const char 
**argv, const char *pr
return 0;
 }
 
+static char *get_submodule_displaypath(const char *path, const char *prefix)
+{
+   const char *super_prefix = get_super_prefix();
+
+   if (prefix && super_prefix) {
+   BUG("cannot have prefix '%s' and superprefix '%s'",
+   prefix, super_prefix);
+   } else if (prefix) {
+   struct strbuf sb = STRBUF_INIT;
+   char *displaypath = xstrdup(relative_path(path, prefix, ));
+   strbuf_release();
+   return displaypath;
+   } else if (super_prefix) {
+   int len = strlen(super_prefix);
+   const char *format = is_dir_sep(super_prefix[len - 1]) ? "%s%s" 
: "%s/%s";
+   return xstrfmt(format, super_prefix, path);
+   } else {
+   return xstrdup(path);
+   }
+}
+
 struct module_list {
const struct cache_entry **entries;
int alloc, nr;
@@ -339,16 +360,7 @@ static void init_submodule(const char *path, const char 
*prefix, int quiet)
 
/* Only loads from .gitmodules, no overlay with .git/config */
gitmodules_config();
-
-   if (prefix && get_super_prefix())
-   die("BUG: cannot have prefix and superprefix");
-   else if (prefix)
-   displaypath = xstrdup(relative_path(path, prefix, ));
-   else if (get_super_prefix()) {
-   strbuf_addf(, "%s%s", get_super_prefix(), path);
-   displaypath = strbuf_detach(, NULL);
-   } else
-   displaypath = xstrdup(path);
+   displaypath = get_submodule_displaypath(path, prefix);
 
sub = submodule_from_path(null_sha1, path);
 
@@ -363,7 +375,6 @@ static void init_submodule(const char *path, const char 
*prefix, int quiet)
 * Set active flag for the submodule being initialized
 */
if (!is_submodule_active(the_repository, path)) {
-   strbuf_reset();
strbuf_addf(, "submodule.%s.active", sub->name);
git_config_set_gently(sb.buf, "true");
}
-- 
2.13.0