Sebastian Schuberth <sschube...@gmail.com> writes:

> Subject: Re: [PATCH v2] clone: Simplify string handling in guess_dir_name()

We seem not to capitalize the first word on the subject line.

> Content-Type: multipart/mixed;  
> boundary="----=_Part_8_836493213.1436462597065"

Please don't.

> Signed-off-by: Sebastian Schuberth <sschube...@gmail.com>
> ---
>  builtin/clone.c | 16 +++-------------
>  1 file changed, 3 insertions(+), 13 deletions(-)
>
> diff --git a/builtin/clone.c b/builtin/clone.c
> index 00535d0..afdc004 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -147,6 +147,7 @@ static char *get_repo_path(const char *repo, int 
> *is_bundle)
>  static char *guess_dir_name(const char *repo, int is_bundle, int is_bare)
>  {
>       const char *end = repo + strlen(repo), *start;
> +     size_t len;
>       char *dir;
>  
>       /*
> @@ -173,20 +174,9 @@ static char *guess_dir_name(const char *repo, int 
> is_bundle, int is_bare)
>       /*
>        * Strip .{bundle,git}.
>        */
> -     if (is_bundle) {
> -             if (end - start > 7 && !strncmp(end - 7, ".bundle", 7))
> -                     end -= 7;
> -     } else {
> -             if (end - start > 4 && !strncmp(end - 4, ".git", 4))
> -                     end -= 4;
> -     }
> +     strip_suffix(start, is_bundle ? ".bundle" : ".git" , &len);

This looks vastly nicer than the original.

> -     if (is_bare) {
> -             struct strbuf result = STRBUF_INIT;
> -             strbuf_addf(&result, "%.*s.git", (int)(end - start), start);
> -             dir = strbuf_detach(&result, NULL);
> -     } else
> -             dir = xstrndup(start, end - start);
> +     dir = is_bare ? xstrfmt("%.*s.git", (int)len, start) : xstrndup(start, 
> len);

This however I had to read twice.  I'd say

        if (is_bare)
                dir = xstrfmt(...);
        else
                dir = xstrndup(...);

is much easier to read.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to