From: Junio C Hamano <gitster@pobox.com>
Date: Tue, 19 Apr 2011 04:34:13 GMT
Subject: Re: [PATCH 2/2] i18n.repositoryencoding: a new variable specifying the encoding of blobs in the repository
Message-ID: <7vy6366ere.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1302987004-28134-1-git-send-email-r0bertz@gentoo.org>

"ZHANG, Le" <r0bertz@gentoo.org> writes:

> When not set it defaults to 'verbatim', nothing will be done.
> When set, the encoding of the blobs in repository will be converted to it.
> The original encoding is get from mail header.
>
> Signed-off-by: ZHANG, Le <r0bertz@gentoo.org>

As I suspect that you would need to reroll the [PATCH 1/2], my comment on
this patch might become unapplicable, but anyway...

> @@ -824,8 +825,10 @@ static int handle_commit_msg(struct strbuf *line)
>  	return 0;
>  }
>  
> -static void handle_patch(const struct strbuf *line)
> +static void handle_patch(struct strbuf *line)
>  {
> +	if (strcasecmp(repository_charset, "verbatim"))
> +		convert_to(line, repository_charset, charset.buf);

I really do not want to see you call this strcasecmp for each and every
line of the input.  The majority of the users (read: the current users who
are fine without using this new feature) do not want to pay the overhead.

How about doing it this way instead:

 - Do not define repository_charset variable in this file; do not define
   get_repository_encoding() function in environment.c; just declare
   "const char *repository_encoding" in cache.h (as "extern const ...")
   and define it in environment.c.

 - git_default_i18n_config() in config.c reads i18n.repositoryencoding
   into "repository_encoding".  This variable is initialized to NULL when
   the program is loaded, and as a special case, when the configuration
   variable is "verbatim", this variable is reset to NULL.  Otherwise it
   will hold a copy of the string given by the configuration file (or -c
   option from the command line).

 - This callsite checks if repository_encoding is non-NULL, and if so
   calls convert_to().