Re: [PATCHv2 14/15] ident: trim whitespace from default name/email

From: Junio C Hamano <gitster@pobox.com>
Date: May 22, 2012, 16:55 UTC
Message-ID: <7vipfo5f60.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20120521231029.GN10981@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> ... Any other value
> comes from a config file, where we will have cleaned up
> accidental whitespace already.

Are you referring to the behaviour of the config parser that removes leading and trailing whitespaces when reading an unquoted string value?

If the user really wanted to have trailing whitespaces by quoting, we would let it pass, in other words; it probably is a reasonable behaviour.

The same can be said about the environment variable GIT_COMMITTER_NAME and friends, but "accidental whitespaces are cleaned up already" does not apply to them.

So, isn't the real rationale behind this choice to allow users who give leading or trailing whitespaces in the configuration and environment variables on purpose use whatever value they specified?

I agree with the placement of trimming in this patch, but I do not quite get (I do not mean "I do not agree with") what the quoted sentence wanted to say.

Other than that single small "hrm...", the entire series was a pleasant read. Thanks.

Show 30 quoted lines

>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  ident.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/ident.c b/ident.c
> index cefb829..e279039 100644
> --- a/ident.c
> +++ b/ident.c
> @@ -95,8 +95,10 @@ static void copy_email(const struct passwd *pw, struct strbuf *email)
>  
>  const char *ident_default_name(void)
>  {
> -	if (!git_default_name.len)
> +	if (!git_default_name.len) {
>  		copy_gecos(xgetpwuid_self(), &git_default_name);
> +		strbuf_trim(&git_default_name);
> +	}
>  	return git_default_name.buf;
>  }
>  
> @@ -110,6 +112,7 @@ const char *ident_default_email(void)
>  			user_ident_explicitly_given |= IDENT_MAIL_GIVEN;
>  		} else
>  			copy_email(xgetpwuid_self(), &git_default_email);
> +		strbuf_trim(&git_default_email);
>  	}
>  	return git_default_email.buf;
>  }