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

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Jul 13, 2014, 04:26 UTC
Message-ID: <CAPig+cRwJhyZ=R_HGs9JKUFLuJxoS=abrD9+HuoWDiA5oJrtjw@mail.gmail.com>
In-Reply-To: <1405097573-19239-1-git-send-email-eungjun.yi@navercorp.com>

On Fri, Jul 11, 2014 at 12:52 PM, Yi EungJun <semtlenori@gmail.com> wrote:

Show 63 quoted lines

> 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";

This can be 'const char *'.

Show 24 quoted lines

> +
> +       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";
> +       }

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?

Show 58 quoted lines

> +
> +       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;
> +}
> +