Re: [PATCH 2/2] i18n.repositoryencoding: a new variable specifying the encoding of blobs in the repository
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:51:03
"ZHANG, Le" [off-list ref] 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 <redacted>
As I suspect that you would need to reroll the [PATCH 1/2], my comment on this patch might become unapplicable, but anyway...
quoted hunk
@@ -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().