{"thread":{"id":"37113","subject":"[PATCH v3] http: Add Accept-Language header if possible","startedAt":"2014-07-11T16:52:53Z","lastAt":"2014-07-14T00:42:49Z","messageCount":9,"participants":["Yi EungJun","Jeff King","Eric Sunshine","Peter Krefting","Yi, EungJun","Junio C Hamano"],"isPatch":true,"patchVersion":3,"patchTotal":null},"messages":[{"id":"245795","messageId":"1405097573-19239-1-git-send-email-eungjun.yi@navercorp.com","threadId":"37113","inReplyTo":null,"subject":"[PATCH v3] http: Add Accept-Language header if possible","fromName":"Yi EungJun","fromEmail":"semtlenori@gmail.com","sentAt":"2014-07-11T16:52:53Z","receivedAt":"2014-07-11T16:52:53Z","isPatch":true,"sender":{"key":"semtlenori@gmail.com","avatar":"https://gravatar.com/avatar/8363435d2badb3450df0dd7c4ec2113f6e1d62c44dd3cf7dfe83c5a389b8a9bf?d=mp&s=160"},"body":"Add an Accept-Language header which indicates the user's preferred\nlanguages defined by $LANGUAGE, $LC_ALL, $LC_MESSAGES and $LANG.\n\nExamples:\n LANGUAGE= -> \"\"\n LANGUAGE=ko:en -> \"Accept-Language: ko, en; q=0.9, *; q=0.1\"\n LANGUAGE=ko LANG=en_US.UTF-8 -> \"Accept-Language: ko, *; q=0.1\"\n LANGUAGE= LANG=en_US.UTF-8 -> \"Accept-Language: en-US, *; q=0.1\"\n\nThis gives git servers a chance to display remote error messages in\nthe user's preferred language.\n\nSigned-off-by: Yi EungJun \n---\n http.c | 125 +++++++++++++++++++++++++++++++++++++++++++++\n remote-curl.c | 2 +\n t/t5550-http-fetch-dumb.sh | 21 ++++++++\n 3 files changed, 148 insertions(+)\n\ndiff --git a/http.c b/http.c\nindex 3a28b21..a20f3e2 100644\n--- a/http.c\n+++ b/http.c\n@@ -983,6 +983,129 @@ static void extract_content_type(struct strbuf *raw, struct strbuf *type,\n \t\tstrbuf_addstr(charset, \"ISO-8859-1\");\n }\n \n+/*\n+ * Guess the user's preferred languages from the value in LANGUAGE environment\n+ * variable and LC_MESSAGES locale category.\n+ *\n+ * The result can be a colon-separated list like \"ko:ja:en\".\n+ */\n+static const char* get_preferred_languages() {\n+ const char* retval;\n+\n+\tretval = getenv(\"LANGUAGE\");\n+\tif (retval != NULL && retval[0] != '\\0')\n+\t\treturn retval;\n+\n+\tretval = setlocale(LC_MESSAGES, NULL);\n+\tif (retval != NULL && retval[0] != '\\0'\n+\t\t&& strcmp(retval, \"C\") != 0\n+\t\t&& strcmp(retval, \"POSIX\") != 0)\n+\t\treturn retval;\n+\n+\treturn NULL;\n+}\n+\n+/*\n+ * Add an Accept-Language header which indicates user's preferred languages.\n+ *\n+ * Examples:\n+ * LANGUAGE= -> \"\"\n+ * LANGUAGE=ko:en -> \"Accept-Language: ko, en; q=0.9, *; q=0.1\"\n+ * LANGUAGE=ko_KR.UTF-8:sr@latin -> \"Accept-Language: ko-KR, sr; q=0.9, *; q=0.1\"\n+ * LANGUAGE=ko LANG=en_US.UTF-8 -> \"Accept-Language: ko, *; q=0.1\"\n+ * LANGUAGE= LANG=en_US.UTF-8 -> \"Accept-Language: en-US, *; q=0.1\"\n+ * LANGUAGE= LANG=C -> \"\"\n+ */\n+static struct curl_slist* add_accept_language(struct curl_slist *headers)\n+{\n+\tconst char *p1, *p2, *p3;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tfloat q = 1.0;\n+\tfloat q_precision = 0.1;\n+\tint num_langs = 1;\n+\tchar* q_format = \"; q=%.1f\";\n+\n+\tp1 = get_preferred_languages();\n+\n+\t/* Don't add Accept-Language header if no language is preferred. */\n+\tif (p1 == NULL || p1[0] == '\\0') {\n+\t\tstrbuf_release(&buf);\n+\t\treturn headers;\n+\t}\n+\n+\t/* Count number of preferred languages to decide precision of q-factor */\n+\tfor (p3 = p1; *p3 != '\\0'; p3++) {\n+\t\tif (*p3 == ':') {\n+\t\t\tnum_langs++;\n+\t\t}\n+\t}\n+\n+\t/* Decide the precision for q-factor on number of preferred languages. */\n+\tif (num_langs + 1 > 100) { /* +1 is for '*' */\n+\t\tq_precision = 0.001;\n+\t\tq_format = \"; q=%.3f\";\n+\t} else if (num_langs + 1 > 10) { /* +1 is for '*' */\n+\t\tq_precision = 0.01;\n+\t\tq_format = \"; q=%.2f\";\n+\t}\n+\n+\tstrbuf_addstr(&buf, \"Accept-Language: \");\n+\n+\tfor (p2 = p1; q > q_precision; p2++) {\n+\t\tif ((*p2 == ':' || *p2 == '\\0') && p1 != p2) {\n+\t\t\tif (q < 1.0) {\n+\t\t\t\tstrbuf_addstr(&buf, \", \");\n+\t\t\t}\n+\n+\t\t\tfor (p3 = p1; p3 < p2; p3++) {\n+\t\t\t\t/* Replace '_' with '-'. */\n+\t\t\t\tif (*p3 == '_') {\n+\t\t\t\t\tstrbuf_add(&buf, p1, p3 - p1);\n+\t\t\t\t\tstrbuf_addstr(&buf, \"-\");\n+\t\t\t\t\tp1 = p3 + 1;\n+\t\t\t\t}\n+\n+\t\t\t\t/* Chop off anything after '.' or '@'. */\n+\t\t\t\tif ((*p3 == '.' || *p3 == '@')) {\n+\t\t\t\t\tbreak;\n+\t\t\t\t}\n+\t\t\t}\n+\n+\t\t\tif (p3 > p1) {\n+\t\t\t\tstrbuf_add(&buf, p1, p3 - p1);\n+\t\t\t}\n+\n+\t\t\t/* Put the q factor if only it is less than 1.0. */\n+\t\t\tif (q < 1.0) {\n+\t\t\t\tstrbuf_addf(&buf, q_format, q);\n+\t\t\t}\n+\n+\t\t\tq -= q_precision;\n+\t\t\tp1 = p2 + 1;\n+\n+\t\t\tif (*p2 == '\\0') {\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\t/* Don't add Accept-Language header if no language is preferred. */\n+\tif (q >= 1.0) {\n+\t\tstrbuf_release(&buf);\n+\t\treturn headers;\n+\t}\n+\n+\t/* Add '*' with minimum q-factor greater than 0.0. */\n+\tstrbuf_addstr(&buf, \", *\");\n+\tstrbuf_addf(&buf, q_format, q_precision);\n+\n+\theaders = curl_slist_append(headers, buf.buf);\n+\n+\tstrbuf_release(&buf);\n+\n+\treturn headers;\n+}\n+\n /* http_request() targets */\n #define HTTP_REQUEST_STRBUF\t0\n #define HTTP_REQUEST_FILE\t1\n@@ -1020,6 +1143,8 @@ static int http_request(const char *url,\n \t\t\t\t\t fwrite_buffer);\n \t}\n \n+\theaders = add_accept_language(headers);\n+\n \tstrbuf_addstr(&buf, \"Pragma:\");\n \tif (options && options->no_cache)\n \t\tstrbuf_addstr(&buf, \" no-cache\");\ndiff --git a/remote-curl.c b/remote-curl.c\nindex 4493b38..07f2a5d 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -946,6 +946,8 @@ int main(int argc, const char **argv)\n \tstruct strbuf buf = STRBUF_INIT;\n \tint nongit;\n \n+\tgit_setup_gettext();\n+\n \tgit_extract_argv0_path(argv[0]);\n \tsetup_git_directory_gently(&nongit);\n \tif (argc < 2) {\ndiff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh\nindex ac71418..2cc41d2 100755\n--- a/t/t5550-http-fetch-dumb.sh\n+++ b/t/t5550-http-fetch-dumb.sh\n@@ -196,5 +196,26 @@ test_expect_success 'reencoding is robust to whitespace oddities' '\n \tgrep \"this is the error message\" stderr\n '\n \n+test_expect_success 'git client sends Accept-Language based on LANGUAGE, LC_ALL, LC_MESSAGES and LANG' '\n+\ttest_must_fail env GIT_CURL_VERBOSE=1 LANGUAGE=ko_KR.UTF-8 LC_ALL=de_DE.UTF-8 LC_MESSAGES=ja_JP.UTF-8 LANG=en_US.UTF-8 git clone \"$HTTPD_URL/accept/language\" 2>stderr &&\n+\tgrep \"^Accept-Language: ko-KR, \\\\*; q=0.1\" stderr &&\n+\ttest_must_fail env GIT_CURL_VERBOSE=1 LANGUAGE= LC_ALL=de_DE.UTF-8 LC_MESSAGES=ja_JP.UTF-8 LANG=en_US.UTF-8 git clone \"$HTTPD_URL/accept/language\" 2>stderr &&\n+\tgrep \"^Accept-Language: de-DE, \\\\*; q=0.1\" stderr &&\n+\ttest_must_fail env GIT_CURL_VERBOSE=1 LANGUAGE= LC_ALL= LC_MESSAGES=ja_JP.UTF-8 LANG=en_US.UTF-8 git clone \"$HTTPD_URL/accept/language\" 2>stderr &&\n+\tgrep \"^Accept-Language: ja-JP, \\\\*; q=0.1\" stderr &&\n+\ttest_must_fail env GIT_CURL_VERBOSE=1 LANGUAGE= LC_ALL= LC_MESSAGES= LANG=en_US.UTF-8 git clone \"$HTTPD_URL/accept/language\" 2>stderr &&\n+\tgrep \"^Accept-Language: en-US, \\\\*; q=0.1\" stderr\n+'\n+\n+test_expect_success 'git client sends Accept-Language with many preferred languages' '\n+\ttest_must_fail env GIT_CURL_VERBOSE=1 LANGUAGE=ko_KR.EUC-KR:en_US.UTF-8:fr_CA:de.UTF-8@euro:sr@latin:ja:zh:sv:pt:nb git clone \"$HTTPD_URL/accept/language\" 2>stderr &&\n+\tgrep \"^Accept-Language: ko-KR, en-US; q=0.99, fr-CA; q=0.98, de; q=0.97, sr; q=0.96, ja; q=0.95, zh; q=0.94, sv; q=0.93, pt; q=0.92, nb; q=0.91, \\\\*; q=0.01\" stderr\n+'\n+\n+test_expect_success 'git client does not send Accept-Language' '\n+\ttest_must_fail env GIT_CURL_VERBOSE=1 LANGUAGE= git clone \"$HTTPD_URL/accept/language\" 2>stderr &&\n+\t! grep \"^Accept-Language:\" stderr\n+'\n+\n stop_httpd\n test_done\n-- \n2.0.1.473.g4b69a73.dirty\n\nThe only difference from the patch v2 is \\-escape in the tests. Thanks to Eric Sunshine.\n"},{"id":"245803","messageId":"20140711173544.GB7856@sigill.intra.peff.net","threadId":"37113","inReplyTo":"1405097573-19239-1-git-send-email-eungjun.yi@navercorp.com","subject":"Re: [PATCH v3] http: Add Accept-Language header if possible","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-07-11T17:35:44Z","receivedAt":"2014-07-11T17:35:44Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jul 12, 2014 at 01:52:53AM +0900, Yi EungJun wrote:\n\n> Add an Accept-Language header which indicates the user's preferred\n> languages defined by $LANGUAGE, $LC_ALL, $LC_MESSAGES and $LANG.\n> \n> Examples:\n> LANGUAGE= -> \"\"\n> LANGUAGE=ko:en -> \"Accept-Language: ko, en; q=0.9, *; q=0.1\"\n> LANGUAGE=ko LANG=en_US.UTF-8 -> \"Accept-Language: ko, *; q=0.1\"\n> LANGUAGE= LANG=en_US.UTF-8 -> \"Accept-Language: en-US, *; q=0.1\"\n> \n> This gives git servers a chance to display remote error messages in\n> the user's preferred language.\n\nThanks, this is looking much nicer. Most of my comments are on style:\n\n> +static const char* get_preferred_languages() {\n> + const char* retval;\n\nA few style nits:\n\n 1. We usually put a function's opening brace on a new line.\n\n 2. We usually put the asterisk in a pointer declaration with the\n variable name (\"const char *retval\"). This one appears elsewhere in\n the patch.\n\n 3. This line seems to be indented with spaces instead of tabs.\n\n> +\tretval = getenv(\"LANGUAGE\");\n> +\tif (retval != NULL && retval[0] != '\\0')\n> +\t\treturn retval;\n> +\n> +\tretval = setlocale(LC_MESSAGES, NULL);\n> +\tif (retval != NULL && retval[0] != '\\0'\n> +\t\t&& strcmp(retval, \"C\") != 0\n> +\t\t&& strcmp(retval, \"POSIX\") != 0)\n> +\t\treturn retval;\n\nMore style nits: we usually avoid writing out explicit comparisons with\nNULL (and often with zero). So I'd write this as:\n\n if (retval && *retval &&\n strcmp(retval, \"C\") &&\n strcmp(retval, \"POSIX\"))\n\nbut I think the NULL one is the only one we usually enforce explicitly.\n\n> +\tconst char *p1, *p2, *p3;\n\nI had trouble following the logic with these variable names. Is it\npossible to give them more descriptive names?\n\n> +\t/* Don't add Accept-Language header if no language is preferred. */\n> +\tif (p1 == NULL || p1[0] == '\\0') {\n> +\t\tstrbuf_release(&buf);\n> +\t\treturn headers;\n> +\t}\n\nNo need to strbuf_release a buffer that hasn't been used (assigning\nSTRBUF_INIT does not count as use).\n\n> +\t/* Count number of preferred languages to decide precision of q-factor */\n> +\tfor (p3 = p1; *p3 != '\\0'; p3++) {\n> +\t\tif (*p3 == ':') {\n> +\t\t\tnum_langs++;\n> +\t\t}\n> +\t}\n\nStyle: we usually omit braces for one-liners. So:\n\n for (p3 = p1; *p3; p3++)\n\tif (*p3 == ':')\n\t\tnum_langs++;\n\n(and elsewhere).\n\n> +\t/* Decide the precision for q-factor on number of preferred languages. */\n> +\tif (num_langs + 1 > 100) { /* +1 is for '*' */\n> +\t\tq_precision = 0.001;\n> +\t\tq_format = \"; q=%.3f\";\n> +\t} else if (num_langs + 1 > 10) { /* +1 is for '*' */\n> +\t\tq_precision = 0.01;\n> +\t\tq_format = \"; q=%.2f\";\n> +\t}\n\nI don't mind this auto-precision too much, but I'm not sure it buys us\nanything. We are still setting a hard-limit at 100, and it just means we\nwrite \"0.1\" instead of \"0.001\" sometimes.\n\n> +\theaders = curl_slist_append(headers, buf.buf);\n> +\n> +\tstrbuf_release(&buf);\n\nDo all versions of curl make a copy of buf.buf here?\n\nI seem to recall that older versions want pointers passed to\ncurl_easy_setopt to stay around for the duration of the request (I do\nnot know about curl_slist, though).\n\n> @@ -1020,6 +1143,8 @@ static int http_request(const char *url,\n> \t\t\t\t\t fwrite_buffer);\n> \t}\n> \n> +\theaders = add_accept_language(headers);\n\nThis means we do the whole parsing routine for each request we make (for\ndumb-http, this can be quite a lot, though I suppose the parsing is not\nespecially expensive). Should we perhaps just cache the result in a\nstatic strbuf? That would also make the pointer-lifetime issue above go\naway.\n\n> --- a/remote-curl.c\n> +++ b/remote-curl.c\n> @@ -946,6 +946,8 @@ int main(int argc, const char **argv)\n> \tstruct strbuf buf = STRBUF_INIT;\n> \tint nongit;\n> \n> +\tgit_setup_gettext();\n\nOops. We probably should have been doing this all along to localize the\nmessages on our side.\n\n> +test_expect_success 'git client sends Accept-Language based on LANGUAGE, LC_ALL, LC_MESSAGES and LANG' '\n> +\ttest_must_fail env GIT_CURL_VERBOSE=1 LANGUAGE=ko_KR.UTF-8 LC_ALL=de_DE.UTF-8 LC_MESSAGES=ja_JP.UTF-8 LANG=en_US.UTF-8 git clone \"$HTTPD_URL/accept/language\" 2>stderr &&\n\nWe usually try to avoid long lines (you can break them up with \"\\\").\n\n> +\tgrep \"^Accept-Language: ko-KR, \\\\*; q=0.1\" stderr &&\n\nI see you noticed the extra level of quoting necessary between v2 and\nv3. :)\n\nI wonder if these test cases might be more readable with a helper\nfunction like:\n\n check_language () {\n\techo \"Accept-Language: $1\" >expect &&\n\ttest_must_fail env \\\n\t\tGIT_CURL_VERBOSE=1 \\\n\t\tLANGUAGE=$2 \\\n\t\tLC_ALL=$3 \\\n\t\tLC_MESSAGES=$4 \\\n\t\tLANG=$5 \\\n\t\tgit clone \"$HTTPD_URL/accept/language\" 2>stderr &&\n\tgrep -i ^Accept-Language: stderr >actual &&\n\ttest_cmp expect actual\n }\n\nwhich lets you write:\n\n check_language \"de-DE, *; q=0.1\" \"\" de_DE.UTF-8 ja_JP.UTF-8 en_US.UTF-8\n\nand so on. I dunno if that is more readable or not.\n\n-Peff\n"},{"id":"245840","messageId":"CAPig+cTHnhJjMimeHyJYm0vOb0PwNWtQArAcPZ3tPkvaGUyMzA@mail.gmail.com","threadId":"37113","inReplyTo":"20140711173544.GB7856@sigill.intra.peff.net","subject":"Re: [PATCH v3] http: Add Accept-Language header if possible","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-07-11T22:29:03Z","receivedAt":"2014-07-11T22:29:03Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Jul 11, 2014 at 1:35 PM, Jeff King wrote:\n> On Sat, Jul 12, 2014 at 01:52:53AM +0900, Yi EungJun wrote:\n>> Add an Accept-Language header which indicates the user's preferred\n>> languages defined by $LANGUAGE, $LC_ALL, $LC_MESSAGES and $LANG.\n>>\n>> Examples:\n>> LANGUAGE= -> \"\"\n>> LANGUAGE=ko:en -> \"Accept-Language: ko, en; q=0.9, *; q=0.1\"\n>> LANGUAGE=ko LANG=en_US.UTF-8 -> \"Accept-Language: ko, *; q=0.1\"\n>> LANGUAGE= LANG=en_US.UTF-8 -> \"Accept-Language: en-US, *; q=0.1\"\n>>\n>> This gives git servers a chance to display remote error messages in\n>> the user's preferred language.\n>\n> Thanks, this is looking much nicer. Most of my comments are on style:\n>\n>> +static const char* get_preferred_languages() {\n>> + const char* retval;\n>\n> A few style nits:\n\nAlso, this is C, not C++, so don't forget void:\n\n static const char *get_preferred_languages(void)\n {\n\n> 1. We usually put a function's opening brace on a new line.\n>\n> 2. We usually put the asterisk in a pointer declaration with the\n> variable name (\"const char *retval\"). This one appears elsewhere in\n> the patch.\n>\n> 3. This line seems to be indented with spaces instead of tabs.\n"},{"id":"245891","messageId":"alpine.DEB.2.00.1407121115110.18850@ds9.cixit.se","threadId":"37113","inReplyTo":"1405097573-19239-1-git-send-email-eungjun.yi@navercorp.com","subject":"Re: [PATCH v3] http: Add Accept-Language header if possible","fromName":"Peter Krefting","fromEmail":"peter@softwolves.pp.se","sentAt":"2014-07-12T10:16:02Z","receivedAt":"2014-07-12T10:16:02Z","isPatch":true,"sender":{"key":"peter@softwolves.pp.se","avatar":"https://avatars.githubusercontent.com/u/990764?v=4"},"body":"Yi EungJun:\n\n> Add an Accept-Language header which indicates the user's preferred\n> languages defined by $LANGUAGE, $LC_ALL, $LC_MESSAGES and $LANG.\n\nThis one seems to fix all the issues I had with the first patch, \nthanks!\n\n-- \n\\\\// Peter - http://www.softwolves.pp.se/\n"},{"id":"245900","messageId":"CAFT+Tg8HfoFO8T+EJ6DiMj6kRO8h0P7tO2PHTK9BFMMe+Ld_Vg@mail.gmail.com","threadId":"37113","inReplyTo":"20140711173544.GB7856@sigill.intra.peff.net","subject":"Re: [PATCH v3] http: Add Accept-Language header if possible","fromName":"Yi, EungJun","fromEmail":"semtlenori@gmail.com","sentAt":"2014-07-12T18:52:39Z","receivedAt":"2014-07-12T18:52:39Z","isPatch":true,"sender":{"key":"semtlenori@gmail.com","avatar":"https://gravatar.com/avatar/8363435d2badb3450df0dd7c4ec2113f6e1d62c44dd3cf7dfe83c5a389b8a9bf?d=mp&s=160"},"body":"Thanks for your detailed review and nice suggestions. I will accept\nmost of them.\n\n2014-07-12 2:35 GMT+09:00 Jeff King :\n>> + /* Decide the precision for q-factor on number of preferred languages. */\n>> + if (num_langs + 1 > 100) { /* +1 is for '*' */\n>> + q_precision = 0.001;\n>> + q_format = \"; q=%.3f\";\n>> + } else if (num_langs + 1 > 10) { /* +1 is for '*' */\n>> + q_precision = 0.01;\n>> + q_format = \"; q=%.2f\";\n>> + }\n>\n> I don't mind this auto-precision too much, but I'm not sure it buys us\n> anything. We are still setting a hard-limit at 100, and it just means we\n> write \"0.1\" instead of \"0.001\" sometimes.\n\nIt means we use \"0.1\" if possible.\n\n>From my observation, many major web browsers doesn't or didn't send\nq-factor of 2 or 3 decimal places. Google chrome doesn't currently and\nMozilla firefox also didn't before 2012 [1]. I think it means some old\nand naive web servers may not support q-factor of 2 or 3 decimal\nplaces because major web browsers don't send it. So I think we should\nuse \"0.1\" if possible for interoperability with the buggy servers.\n\nBut, quite frankly, it is just a possibility and I have no evidence\nwhich proves that such kind of buggy servers really exist. Please let\nme know if anybody know about it.\n\n[1]: http://hg.mozilla.org/integration/mozilla-inbound/rev/b0b07ef904ea\n"},{"id":"245903","messageId":"CAPig+cRwJhyZ=R_HGs9JKUFLuJxoS=abrD9+HuoWDiA5oJrtjw@mail.gmail.com","threadId":"37113","inReplyTo":"1405097573-19239-1-git-send-email-eungjun.yi@navercorp.com","subject":"Re: [PATCH v3] http: Add Accept-Language header if possible","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-07-13T04:26:58Z","receivedAt":"2014-07-13T04:26:58Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Jul 11, 2014 at 12:52 PM, Yi EungJun wrote:\n> Add an Accept-Language header which indicates the user's preferred\n> languages defined by $LANGUAGE, $LC_ALL, $LC_MESSAGES and $LANG.\n>\n> Examples:\n> LANGUAGE= -> \"\"\n> LANGUAGE=ko:en -> \"Accept-Language: ko, en; q=0.9, *; q=0.1\"\n> LANGUAGE=ko LANG=en_US.UTF-8 -> \"Accept-Language: ko, *; q=0.1\"\n> LANGUAGE= LANG=en_US.UTF-8 -> \"Accept-Language: en-US, *; q=0.1\"\n>\n> This gives git servers a chance to display remote error messages in\n> the user's preferred language.\n>\n> Signed-off-by: Yi EungJun \n> ---\n> diff --git a/http.c b/http.c\n> index 3a28b21..a20f3e2 100644\n> --- a/http.c\n> +++ b/http.c\n> @@ -983,6 +983,129 @@ static void extract_content_type(struct strbuf *raw, struct strbuf *type,\n> strbuf_addstr(charset, \"ISO-8859-1\");\n> }\n>\n> +/*\n> + * Guess the user's preferred languages from the value in LANGUAGE environment\n> + * variable and LC_MESSAGES locale category.\n> + *\n> + * The result can be a colon-separated list like \"ko:ja:en\".\n> + */\n> +static const char* get_preferred_languages() {\n> + const char* retval;\n> +\n> + retval = getenv(\"LANGUAGE\");\n> + if (retval != NULL && retval[0] != '\\0')\n> + return retval;\n> +\n> + retval = setlocale(LC_MESSAGES, NULL);\n> + if (retval != NULL && retval[0] != '\\0'\n> + && strcmp(retval, \"C\") != 0\n> + && strcmp(retval, \"POSIX\") != 0)\n> + return retval;\n> +\n> + return NULL;\n> +}\n> +\n> +/*\n> + * Add an Accept-Language header which indicates user's preferred languages.\n> + *\n> + * Examples:\n> + * LANGUAGE= -> \"\"\n> + * LANGUAGE=ko:en -> \"Accept-Language: ko, en; q=0.9, *; q=0.1\"\n> + * LANGUAGE=ko_KR.UTF-8:sr@latin -> \"Accept-Language: ko-KR, sr; q=0.9, *; q=0.1\"\n> + * LANGUAGE=ko LANG=en_US.UTF-8 -> \"Accept-Language: ko, *; q=0.1\"\n> + * LANGUAGE= LANG=en_US.UTF-8 -> \"Accept-Language: en-US, *; q=0.1\"\n> + * LANGUAGE= LANG=C -> \"\"\n> + */\n> +static struct curl_slist* add_accept_language(struct curl_slist *headers)\n> +{\n> + const char *p1, *p2, *p3;\n> + struct strbuf buf = STRBUF_INIT;\n> + float q = 1.0;\n> + float q_precision = 0.1;\n> + int num_langs = 1;\n> + char* q_format = \"; q=%.1f\";\n\nThis can be 'const char *'.\n\n> +\n> + p1 = get_preferred_languages();\n> +\n> + /* Don't add Accept-Language header if no language is preferred. */\n> + if (p1 == NULL || p1[0] == '\\0') {\n> + strbuf_release(&buf);\n> + return headers;\n> + }\n> +\n> + /* Count number of preferred languages to decide precision of q-factor */\n> + for (p3 = p1; *p3 != '\\0'; p3++) {\n> + if (*p3 == ':') {\n> + num_langs++;\n> + }\n> + }\n> +\n> + /* Decide the precision for q-factor on number of preferred languages. */\n> + if (num_langs + 1 > 100) { /* +1 is for '*' */\n> + q_precision = 0.001;\n> + q_format = \"; q=%.3f\";\n> + } else if (num_langs + 1 > 10) { /* +1 is for '*' */\n> + q_precision = 0.01;\n> + q_format = \"; q=%.2f\";\n> + }\n\nIt might make sense to have a final 'else' here which sets these\nvariables for the 0.1 case so that the reader of the code doesn't have\nto refer back to the top of the function to figure out what is going\non.\n\n } else {\n q_precision = 0.1;\n q_format = \"; q=%.1f\";\n }\n\nBetter yet, would it be possible to compute these values rather than\nhaving to set them manually via a cascading if-chain?\n\n> +\n> + strbuf_addstr(&buf, \"Accept-Language: \");\n> +\n> + for (p2 = p1; q > q_precision; p2++) {\n> + if ((*p2 == ':' || *p2 == '\\0') && p1 != p2) {\n> + if (q < 1.0) {\n> + strbuf_addstr(&buf, \", \");\n> + }\n> +\n> + for (p3 = p1; p3 < p2; p3++) {\n> + /* Replace '_' with '-'. */\n> + if (*p3 == '_') {\n> + strbuf_add(&buf, p1, p3 - p1);\n> + strbuf_addstr(&buf, \"-\");\n> + p1 = p3 + 1;\n> + }\n> +\n> + /* Chop off anything after '.' or '@'. */\n> + if ((*p3 == '.' || *p3 == '@')) {\n> + break;\n> + }\n> + }\n> +\n> + if (p3 > p1) {\n> + strbuf_add(&buf, p1, p3 - p1);\n> + }\n> +\n> + /* Put the q factor if only it is less than 1.0. */\n> + if (q < 1.0) {\n> + strbuf_addf(&buf, q_format, q);\n> + }\n> +\n> + q -= q_precision;\n> + p1 = p2 + 1;\n> +\n> + if (*p2 == '\\0') {\n> + break;\n> + }\n> + }\n> + }\n> +\n> + /* Don't add Accept-Language header if no language is preferred. */\n> + if (q >= 1.0) {\n> + strbuf_release(&buf);\n> + return headers;\n> + }\n> +\n> + /* Add '*' with minimum q-factor greater than 0.0. */\n> + strbuf_addstr(&buf, \", *\");\n> + strbuf_addf(&buf, q_format, q_precision);\n> +\n> + headers = curl_slist_append(headers, buf.buf);\n> +\n> + strbuf_release(&buf);\n> +\n> + return headers;\n> +}\n> +\n"},{"id":"245946","messageId":"CAFT+Tg8pJrdoD4M1uy6A=PPJaSrq_QcRUQdxdrA-T5sJiZ7j1Q@mail.gmail.com","threadId":"37113","inReplyTo":"CAPig+cRwJhyZ=R_HGs9JKUFLuJxoS=abrD9+HuoWDiA5oJrtjw@mail.gmail.com","subject":"Re: [PATCH v3] http: Add Accept-Language header if possible","fromName":"Yi, EungJun","fromEmail":"semtlenori@gmail.com","sentAt":"2014-07-13T09:51:57Z","receivedAt":"2014-07-13T09:51:57Z","isPatch":true,"sender":{"key":"semtlenori@gmail.com","avatar":"https://gravatar.com/avatar/8363435d2badb3450df0dd7c4ec2113f6e1d62c44dd3cf7dfe83c5a389b8a9bf?d=mp&s=160"},"body":"2014-07-13 13:26 GMT+09:00 Eric Sunshine :\n>> + /* Decide the precision for q-factor on number of preferred languages. */\n>> + if (num_langs + 1 > 100) { /* +1 is for '*' */\n>> + q_precision = 0.001;\n>> + q_format = \"; q=%.3f\";\n>> + } else if (num_langs + 1 > 10) { /* +1 is for '*' */\n>> + q_precision = 0.01;\n>> + q_format = \"; q=%.2f\";\n>> + }\n>\n> It might make sense to have a final 'else' here which sets these\n> variables for the 0.1 case so that the reader of the code doesn't have\n> to refer back to the top of the function to figure out what is going\n> on.\n>\n> } else {\n> q_precision = 0.1;\n> q_format = \"; q=%.1f\";\n> }\n>\n> Better yet, would it be possible to compute these values rather than\n> having to set them manually via a cascading if-chain?\n\nI think it is possible like this:\n\n num_langs += 1; /* for '*' */\n decimal_places = 1 + (num_langs > 10) + (num_langs > 100);\n snprintf(q_format, sizeof(q_format), \"; q=%%.%df\", decimal_places);\n for (q_precision = 1.0; decimal_places-- > 0;) q_precision /= 10;\n\nDoes this one look better than before? I'm not sure which one is better.\n\nps. The last line can be simpler by using pow() but I'm not sure it is\nokay to include math.h.\n"},{"id":"245950","messageId":"xmqqoawt19jr.fsf@gitster.dls.corp.google.com","threadId":"37113","inReplyTo":"CAFT+Tg8pJrdoD4M1uy6A=PPJaSrq_QcRUQdxdrA-T5sJiZ7j1Q@mail.gmail.com","subject":"Re: [PATCH v3] http: Add Accept-Language header if possible","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-07-13T16:57:12Z","receivedAt":"2014-07-13T16:57:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Yi, EungJun\" writes:\n\n> I think it is possible like this:\n>\n> num_langs += 1; /* for '*' */\n> decimal_places = 1 + (num_langs > 10) + (num_langs > 100);\n> snprintf(q_format, sizeof(q_format), \"; q=%%.%df\", decimal_places);\n> for (q_precision = 1.0; decimal_places-- > 0;) q_precision /= 10;\n>\n> Does this one look better than before? I'm not sure which one is better.\n>\n> ps. The last line can be simpler by using pow() but I'm not sure it is\n> okay to include math.h.\n\nIf you do not want floating point (and I think we tend to avoid it\nwhen we do not need it), you can realize that in your use of \"0.1\"\nand \"0.01\" and \"0.001\" there is nothing fundamentally floating-point;\nyou can measure how many digits below the two-byte string zero-dot\nyou would want upfront (by counting num_langs), and show an integer\ncounter zero-padded to the left to that width.\n\nThat would avoid having to even worry about a possible funny case\nwhere subtracting 0.01 ten times from 0.1 may not yield zero (or the\nresult of subtracting nine times may not reach 0.01) due to rounding\nerrors accumulating, which was the first thing that came to my mind\nwhen I saw your loop.\n"},{"id":"245962","messageId":"CAFT+Tg-FmtfmhuTr8-tbbOtDs73p0qzQoT+UVdu3bZLizJbWOQ@mail.gmail.com","threadId":"37113","inReplyTo":"xmqqoawt19jr.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3] http: Add Accept-Language header if possible","fromName":"Yi, EungJun","fromEmail":"semtlenori@gmail.com","sentAt":"2014-07-14T00:42:49Z","receivedAt":"2014-07-14T00:42:49Z","isPatch":true,"sender":{"key":"semtlenori@gmail.com","avatar":"https://gravatar.com/avatar/8363435d2badb3450df0dd7c4ec2113f6e1d62c44dd3cf7dfe83c5a389b8a9bf?d=mp&s=160"},"body":"2014-07-14 1:57 GMT+09:00 Junio C Hamano :\n>\n> If you do not want floating point (and I think we tend to avoid it\n> when we do not need it), you can realize that in your use of \"0.1\"\n> and \"0.01\" and \"0.001\" there is nothing fundamentally floating-point;\n> you can measure how many digits below the two-byte string zero-dot\n> you would want upfront (by counting num_langs), and show an integer\n> counter zero-padded to the left to that width.\n>\n> That would avoid having to even worry about a possible funny case\n> where subtracting 0.01 ten times from 0.1 may not yield zero (or the\n> result of subtracting nine times may not reach 0.01) due to rounding\n> errors accumulating, which was the first thing that came to my mind\n> when I saw your loop.\n\nYou're right; We don't need floating point numbers. I'll try to fix it.\n"}]}