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

[Junio C Hamano <gitster@pobox.com>]

Sebastian Schuberth <sschuberth@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 <sschuberth@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.