v3http: Add Accept-Language header if possible

> Add an Accept-Language header which indicates the user's preferred
> languages defined by $LANGUAGE, $LC_ALL, $LC_MESSAGES and $LANG.
> 
> Examples:
>   LANGUAGE= -> ""
>   LANGUAGE=ko:en -> "Accept-Language: ko, en; q=0.9, *; q=0.1"
>   LANGUAGE=ko LANG=en_US.UTF-8 -> "Accept-Language: ko, *; q=0.1"
>   LANGUAGE= LANG=en_US.UTF-8 -> "Accept-Language: en-US, *; q=0.1"
> 
> This gives git servers a chance to display remote error messages in
> the user's preferred language.

> +	retval = getenv("LANGUAGE");
> +	if (retval != NULL && retval[0] != '\0')
> +		return retval;
> +
> +	retval = setlocale(LC_MESSAGES, NULL);
> +	if (retval != NULL && retval[0] != '\0'
> +		&& strcmp(retval, "C") != 0
> +		&& strcmp(retval, "POSIX") != 0)
> +		return retval;

> +	/* Don't add Accept-Language header if no language is preferred. */
> +	if (p1 == NULL || p1[0] == '\0') {
> +		strbuf_release(&buf);
> +		return headers;
> +	}

> +	/* Count number of preferred languages to decide precision of q-factor */
> +	for (p3 = p1; *p3 != '\0'; p3++) {
> +		if (*p3 == ':') {
> +			num_langs++;
> +		}
> +	}

> +	/* Decide the precision for q-factor on number of preferred languages. */
> +	if (num_langs + 1 > 100) { /* +1 is for '*' */
> +		q_precision = 0.001;
> +		q_format = "; q=%.3f";
> +	} else if (num_langs + 1 > 10) { /* +1 is for '*' */
> +		q_precision = 0.01;
> +		q_format = "; q=%.2f";
> +	}

> @@ -1020,6 +1143,8 @@ static int http_request(const char *url,
>  					 fwrite_buffer);
>  	}
>  
> +	headers = add_accept_language(headers);

> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -946,6 +946,8 @@ int main(int argc, const char **argv)
>  	struct strbuf buf = STRBUF_INIT;
>  	int nongit;
>  
> +	git_setup_gettext();

> On Sat, Jul 12, 2014 at 01:52:53AM +0900, Yi EungJun wrote:
>> Add an Accept-Language header which indicates the user's preferred
>> languages defined by $LANGUAGE, $LC_ALL, $LC_MESSAGES and $LANG.
>>
>> Examples:
>>   LANGUAGE= -> ""
>>   LANGUAGE=ko:en -> "Accept-Language: ko, en; q=0.9, *; q=0.1"
>>   LANGUAGE=ko LANG=en_US.UTF-8 -> "Accept-Language: ko, *; q=0.1"
>>   LANGUAGE= LANG=en_US.UTF-8 -> "Accept-Language: en-US, *; q=0.1"
>>
>> This gives git servers a chance to display remote error messages in
>> the user's preferred language.
>
> Thanks, this is looking much nicer. Most of my comments are on style:
>
>> +static const char* get_preferred_languages() {
>> +    const char* retval;
>
> A few style nits:

>   1. We usually put a function's opening brace on a new line.
>
>   2. We usually put the asterisk in a pointer declaration with the
>      variable name ("const char *retval"). This one appears elsewhere in
>      the patch.
>
>   3. This line seems to be indented with spaces instead of tabs.

>> +     /* Decide the precision for q-factor on number of preferred languages. */
>> +     if (num_langs + 1 > 100) { /* +1 is for '*' */
>> +             q_precision = 0.001;
>> +             q_format = "; q=%.3f";
>> +     } else if (num_langs + 1 > 10) { /* +1 is for '*' */
>> +             q_precision = 0.01;
>> +             q_format = "; q=%.2f";
>> +     }
>
> I don't mind this auto-precision too much, but I'm not sure it buys us
> anything. We are still setting a hard-limit at 100, and it just means we
> write "0.1" instead of "0.001" sometimes.

> Add an Accept-Language header which indicates the user's preferred
> languages defined by $LANGUAGE, $LC_ALL, $LC_MESSAGES and $LANG.
>
> Examples:
>   LANGUAGE= -> ""
>   LANGUAGE=ko:en -> "Accept-Language: ko, en; q=0.9, *; q=0.1"
>   LANGUAGE=ko LANG=en_US.UTF-8 -> "Accept-Language: ko, *; q=0.1"
>   LANGUAGE= LANG=en_US.UTF-8 -> "Accept-Language: en-US, *; q=0.1"
>
> This gives git servers a chance to display remote error messages in
> the user's preferred language.
>
> Signed-off-by: Yi EungJun <eungjun.yi@navercorp.com>
> ---
> diff --git a/http.c b/http.c
> index 3a28b21..a20f3e2 100644
> --- a/http.c
> +++ b/http.c
> @@ -983,6 +983,129 @@ static void extract_content_type(struct strbuf *raw, struct strbuf *type,
>                 strbuf_addstr(charset, "ISO-8859-1");
>  }
>
> +/*
> + * Guess the user's preferred languages from the value in LANGUAGE environment
> + * variable and LC_MESSAGES locale category.
> + *
> + * The result can be a colon-separated list like "ko:ja:en".
> + */
> +static const char* get_preferred_languages() {
> +    const char* retval;
> +
> +       retval = getenv("LANGUAGE");
> +       if (retval != NULL && retval[0] != '\0')
> +               return retval;
> +
> +       retval = setlocale(LC_MESSAGES, NULL);
> +       if (retval != NULL && retval[0] != '\0'
> +               && strcmp(retval, "C") != 0
> +               && strcmp(retval, "POSIX") != 0)
> +               return retval;
> +
> +       return NULL;
> +}
> +
> +/*
> + * Add an Accept-Language header which indicates user's preferred languages.
> + *
> + * Examples:
> + *   LANGUAGE= -> ""
> + *   LANGUAGE=ko:en -> "Accept-Language: ko, en; q=0.9, *; q=0.1"
> + *   LANGUAGE=ko_KR.UTF-8:sr@latin -> "Accept-Language: ko-KR, sr; q=0.9, *; q=0.1"
> + *   LANGUAGE=ko LANG=en_US.UTF-8 -> "Accept-Language: ko, *; q=0.1"
> + *   LANGUAGE= LANG=en_US.UTF-8 -> "Accept-Language: en-US, *; q=0.1"
> + *   LANGUAGE= LANG=C -> ""
> + */
> +static struct curl_slist* add_accept_language(struct curl_slist *headers)
> +{
> +       const char *p1, *p2, *p3;
> +       struct strbuf buf = STRBUF_INIT;
> +       float q = 1.0;
> +       float q_precision = 0.1;
> +       int num_langs = 1;
> +       char* q_format = "; q=%.1f";

> +
> +       p1 = get_preferred_languages();
> +
> +       /* Don't add Accept-Language header if no language is preferred. */
> +       if (p1 == NULL || p1[0] == '\0') {
> +               strbuf_release(&buf);
> +               return headers;
> +       }
> +
> +       /* Count number of preferred languages to decide precision of q-factor */
> +       for (p3 = p1; *p3 != '\0'; p3++) {
> +               if (*p3 == ':') {
> +                       num_langs++;
> +               }
> +       }
> +
> +       /* Decide the precision for q-factor on number of preferred languages. */
> +       if (num_langs + 1 > 100) { /* +1 is for '*' */
> +               q_precision = 0.001;
> +               q_format = "; q=%.3f";
> +       } else if (num_langs + 1 > 10) { /* +1 is for '*' */
> +               q_precision = 0.01;
> +               q_format = "; q=%.2f";
> +       }

> +
> +       strbuf_addstr(&buf, "Accept-Language: ");
> +
> +       for (p2 = p1; q > q_precision; p2++) {
> +               if ((*p2 == ':' || *p2 == '\0') && p1 != p2) {
> +                       if (q < 1.0) {
> +                               strbuf_addstr(&buf, ", ");
> +                       }
> +
> +                       for (p3 = p1; p3 < p2; p3++) {
> +                               /* Replace '_' with '-'. */
> +                               if (*p3 == '_') {
> +                                       strbuf_add(&buf, p1, p3 - p1);
> +                                       strbuf_addstr(&buf, "-");
> +                                       p1 = p3 + 1;
> +                               }
> +
> +                               /* Chop off anything after '.' or '@'. */
> +                               if ((*p3 == '.' || *p3 == '@')) {
> +                                       break;
> +                               }
> +                       }
> +
> +                       if (p3 > p1) {
> +                               strbuf_add(&buf, p1, p3 - p1);
> +                       }
> +
> +                       /* Put the q factor if only it is less than 1.0. */
> +                       if (q < 1.0) {
> +                               strbuf_addf(&buf, q_format, q);
> +                       }
> +
> +                       q -= q_precision;
> +                       p1 = p2 + 1;
> +
> +                       if (*p2 == '\0') {
> +                               break;
> +                       }
> +               }
> +       }
> +
> +       /* Don't add Accept-Language header if no language is preferred. */
> +       if (q >= 1.0) {
> +               strbuf_release(&buf);
> +               return headers;
> +       }
> +
> +       /* Add '*' with minimum q-factor greater than 0.0. */
> +       strbuf_addstr(&buf, ", *");
> +       strbuf_addf(&buf, q_format, q_precision);
> +
> +       headers = curl_slist_append(headers, buf.buf);
> +
> +       strbuf_release(&buf);
> +
> +       return headers;
> +}
> +

>> +       /* Decide the precision for q-factor on number of preferred languages. */
>> +       if (num_langs + 1 > 100) { /* +1 is for '*' */
>> +               q_precision = 0.001;
>> +               q_format = "; q=%.3f";
>> +       } else if (num_langs + 1 > 10) { /* +1 is for '*' */
>> +               q_precision = 0.01;
>> +               q_format = "; q=%.2f";
>> +       }
>
> It might make sense to have a final 'else' here which sets these
> variables for the 0.1 case so that the reader of the code doesn't have
> to refer back to the top of the function to figure out what is going
> on.
>
>     } else {
>         q_precision = 0.1;
>         q_format = "; q=%.1f";
>     }
>
> Better yet, would it be possible to compute these values rather than
> having to set them manually via a cascading if-chain?

> I think it is possible like this:
>
>     num_langs += 1; /* for '*' */
>     decimal_places = 1 + (num_langs > 10) + (num_langs > 100);
>     snprintf(q_format, sizeof(q_format), "; q=%%.%df", decimal_places);
>     for (q_precision = 1.0; decimal_places-- > 0;) q_precision /= 10;
>
> Does this one look better than before? I'm not sure which one is better.
>
> ps. The last line can be simpler by using pow() but I'm not sure it is
> okay to include math.h.

>
> If you do not want floating point (and I think we tend to avoid it
> when we do not need it), you can realize that in your use of "0.1"
> and "0.01" and "0.001" there is nothing fundamentally floating-point;
> you can measure how many digits below the two-byte string zero-dot
> you would want upfront (by counting num_langs), and show an integer
> counter zero-padded to the left to that width.
>
> That would avoid having to even worry about a possible funny case
> where subtracting 0.01 ten times from 0.1 may not yield zero (or the
> result of subtracting nine times may not reach 0.01) due to rounding
> errors accumulating, which was the first thing that came to my mind
> when I saw your loop.