Re: [PATCH] http: Add Accept-Language header if possible

From: Yi, EungJun <semtlenori@gmail.com>
Date: Jul 11, 2014, 09:05 UTC
Message-ID: <CAFT+Tg8Z5z=vTcCTbwKMQeTAaqpTYzr5OzdfedV0q8J5KzG-Yg@mail.gmail.com>
In-Reply-To: <alpine.DEB.2.00.1407091127490.22132@ds9.cixit.se>

2014-07-09 19:40 GMT+09:00 Peter Krefting <peter@softwolves.pp.se>:

Show 12 quoted lines

> Yi EungJun:
>
>
>> Example:
>>  LANGUAGE= -> ""
>>  LANGUAGE=ko -> "Accept-Language: ko; q=1.000, *; q=0.001"
>>  LANGUAGE=ko:en -> "Accept-Language: ko; q=1.000, en; q=0.999, *; q=0.001"
>
>
> Avoid adding "q=1.000". It is redundant (the default for any unqualified
> language names is 1.0, and additionally there has historically been some
> buggy servers that failed if it was included.

Ok, I'll fix it.

Show 15 quoted lines

>
>
>> +       p1 = getenv("LANGUAGE");
>
>
> You need a fallback mechanism here to parse all the possible language
> variables. I would use the first one I find of these:
>
>  1. LANGUAGE
>  2. LC_ALL
>  3. LC_MESSAGES
>  4. LANG
>
> Only "LANGUAGE" holds a colon-separated list, but the same code can parse
> all of them, just yielding a single entry for the others.

I'll use setlocale(LC_MESSAGES, NULL) as well as getenv("LANGUAGE").

Show 16 quoted lines

>
>
>> +                               strbuf_add(buf, p1, p2 - p1);
>
>
> The tokens are on the form language_COUNTRY.encoding@identifier, whereas
> Accept-Language wants language-COUNTRY, so you need to a) replace "_" with
> "-", and b) chop off anything following a "." or "@".
>
>
>> +                               strbuf_addf(buf, "; q=%.3f", q);
>> +                               q -= 0.001;
>
>
> Three decimals seems a bit overkill, but some experimentation might be
> necessary.

I'll use three decimals only if there are 100 or more preferred languages.

Show 7 quoted lines

>
>
>> +               strbuf_addstr(buf, "*; q=0.001\r\n");
>
>
> You should probably also add an explicit "en" here, if none was already
> included. I've seen some servers break horribly if "en" isn't included.

I'll send Accept-Language only if there is at least one preferred language. Is it enough?

Thanks for your review.