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

Jeff King [off-list ref] writes:

On Thu, Feb 26, 2015 at 12:59:56PM -0800, Junio C Hamano wrote:

quoted

Jeff King [off-list ref] writes:

quoted

Perhaps it would be less risky to stick get_preferred_languages() into
gettext.c, like the patch below. Then we do not have to worry about
locale.h introducing other disruptive includes. The function is not
technically about gettext, but it seems reasonable to me to stuff all of
the i18n code together.

Yeah, I like that a lot better.  Thanks.

Are you just using it for inspiration, or did you want me to wrap it up
with a commit message?

Here is what I queued.  Thanks.

-- >8 --
From: Jeff King <redacted>
Date: Wed, 25 Feb 2015 22:04:16 -0500
Subject: [PATCH] gettext.c: move get_preferred_languages() from http.c

Calling setlocale(LC_MESSAGES, ...) directly from http.c, without
including <locale.h>, was causing compilation warnings.  Move the
helper function to gettext.c that already includes the header and
where locale-related issues are handled.

Signed-off-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
---
 gettext.c | 25 +++++++++++++++++++++++++
 gettext.h |  2 ++
 http.c    |  1 +
 3 files changed, 28 insertions(+)

diff --git a/gettext.c b/gettext.c
index 8b2da46..7378ba2 100644
--- a/gettext.c
+++ b/gettext.c

@@ -18,6 +18,31 @@
 #	endif
 #endif
 
+/*
+ * Guess the user's preferred languages from the value in LANGUAGE environment
+ * variable and LC_MESSAGES locale category if NO_GETTEXT is not defined.
+ *
+ * The result can be a colon-separated list like "ko:ja:en".
+ */
+const char *get_preferred_languages(void)
+{
+	const char *retval;
+
+	retval = getenv("LANGUAGE");
+	if (retval && *retval)
+		return retval;
+
+#ifndef NO_GETTEXT
+	retval = setlocale(LC_MESSAGES, NULL);
+	if (retval && *retval &&
+		strcmp(retval, "C") &&
+		strcmp(retval, "POSIX"))
+		return retval;
+#endif
+
+	return NULL;
+}
+
 #ifdef GETTEXT_POISON
 int use_gettext_poison(void)
 {

diff --git a/gettext.h b/gettext.h
index 7671d09..e539482 100644
--- a/gettext.h
+++ b/gettext.h

@@ -65,4 +65,6 @@ const char *Q_(const char *msgid, const char *plu, unsigned long n)
 /* Mark msgid for translation but do not translate it. */
 #define N_(msgid) msgid
 
+const char *get_preferred_languages(void);
+
 #endif

diff --git a/http.c b/http.c
index 8b659b6..71ed418 100644
--- a/http.c
+++ b/http.c

@@ -8,6 +8,7 @@
 #include "credential.h"
 #include "version.h"
 #include "pkt-line.h"
+#include "gettext.h"
 
 int active_requests;
 int http_is_verbose;

-- 
2.3.1-280-g2531f2d

On Thu, Feb 26, 2015 at 1:42 PM, Junio C Hamano [off-list ref] wrote:>

Here is what I queued.  Thanks.

I did not follow the thread if there are any intermediate patches,
though it applied cleanly.

Applying this on top of f18604bbf2c391c689a41fca14cbaeff5e106255
(http: add Accept-Language header if possible) still doesn't compile for me.

http.c:1001:20: error: static declaration of 'get_preferred_languages'
follows non-static declaration
 static const char *get_preferred_languages(void)
                    ^
In file included from cache.h:8:0,
                 from http.h:4,
                 from http.c:2:
gettext.h:68:13: note: previous declaration of
'get_preferred_languages' was here
 const char *get_preferred_languages(void);
             ^
http.c: In function 'get_preferred_languages':
http.c:1010:2: warning: implicit declaration of function 'setlocale'
[-Wimplicit-function-declaration]
  retval = setlocale(LC_MESSAGES, NULL);
  ^
http.c:1010:21: error: 'LC_MESSAGES' undeclared (first use in this function)
  retval = setlocale(LC_MESSAGES, NULL);
                     ^
http.c:1010:21: note: each undeclared identifier is reported only once
for each function it appears in

Rebasing this on top of current master (Post 2.3 cyle (batch #5)) also fails:

http.c:1013:20: error: static declaration of 'get_preferred_languages'
follows non-static declaration
 static const char *get_preferred_languages(void)
                    ^
In file included from cache.h:8:0,
                 from http.h:4,
                 from http.c:2:
gettext.h:92:13: note: previous declaration of
'get_preferred_languages' was here
 const char *get_preferred_languages(void);
             ^
http.c: In function 'get_preferred_languages':
http.c:1022:2: warning: implicit declaration of function 'setlocale'
[-Wimplicit-function-declaration]
  retval = setlocale(LC_MESSAGES, NULL);
  ^
http.c:1022:21: error: 'LC_MESSAGES' undeclared (first use in this function)
  retval = setlocale(LC_MESSAGES, NULL);
                     ^
http.c:1022:21: note: each undeclared identifier is reported only once
for each function it appears in

On Thu, Feb 26, 2015 at 01:47:34PM -0800, Stefan Beller wrote:

On Thu, Feb 26, 2015 at 1:42 PM, Junio C Hamano [off-list ref] wrote:>

quoted

Here is what I queued.  Thanks.

I did not follow the thread if there are any intermediate patches,
though it applied cleanly.

What Junio posted is missing the hunk to drop the old static definition
of get_preferred_languages from http.c.

-Peff

On Thu, Feb 26, 2015 at 05:06:10PM -0500, Jeff King wrote:

On Thu, Feb 26, 2015 at 01:47:34PM -0800, Stefan Beller wrote:

quoted

On Thu, Feb 26, 2015 at 1:42 PM, Junio C Hamano [off-list ref] wrote:>

quoted

Here is what I queued.  Thanks.

I did not follow the thread if there are any intermediate patches,
though it applied cleanly.

What Junio posted is missing the hunk to drop the old static definition
of get_preferred_languages from http.c.

Here it is, with the commit message and the missing hunk. This works for
me both with and without NO_GETTEXT defined.

-- >8 --
Subject: [PATCH] gettext.c: move get_preferred_languages() from http.c

Calling setlocale(LC_MESSAGES, ...) directly from http.c,
without including <locale.h>, was causing compilation
warnings.  Move the helper function to gettext.c that
already includes the header and where locale-related issues
are handled.

Signed-off-by: Jeff King <redacted>
---
 gettext.c | 25 +++++++++++++++++++++++++
 gettext.h |  2 ++
 http.c    | 27 +--------------------------
 3 files changed, 28 insertions(+), 26 deletions(-)

diff --git a/gettext.c b/gettext.c
index 8b2da46..7378ba2 100644
--- a/gettext.c
+++ b/gettext.c

@@ -18,6 +18,31 @@
 #	endif
 #endif
 
+/*
+ * Guess the user's preferred languages from the value in LANGUAGE environment
+ * variable and LC_MESSAGES locale category if NO_GETTEXT is not defined.
+ *
+ * The result can be a colon-separated list like "ko:ja:en".
+ */
+const char *get_preferred_languages(void)
+{
+	const char *retval;
+
+	retval = getenv("LANGUAGE");
+	if (retval && *retval)
+		return retval;
+
+#ifndef NO_GETTEXT
+	retval = setlocale(LC_MESSAGES, NULL);
+	if (retval && *retval &&
+		strcmp(retval, "C") &&
+		strcmp(retval, "POSIX"))
+		return retval;
+#endif
+
+	return NULL;
+}
+
 #ifdef GETTEXT_POISON
 int use_gettext_poison(void)
 {

diff --git a/gettext.h b/gettext.h
index dc1722d..5d8d2df 100644
--- a/gettext.h
+++ b/gettext.h

@@ -89,4 +89,6 @@ const char *Q_(const char *msgid, const char *plu, unsigned long n)
 #define N_(msgid) (msgid)
 #endif
 
+const char *get_preferred_languages();
+
 #endif

diff --git a/http.c b/http.c
index 0153fb0..9c825af 100644
--- a/http.c
+++ b/http.c

@@ -8,6 +8,7 @@
 #include "credential.h"
 #include "version.h"
 #include "pkt-line.h"
+#include "gettext.h"
 
 int active_requests;
 int http_is_verbose;

@@ -1002,32 +1003,6 @@ 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 if NO_GETTEXT is not defined.
- *
- * The result can be a colon-separated list like "ko:ja:en".
- */
-static const char *get_preferred_languages(void)
-{
-	const char *retval;
-
-	retval = getenv("LANGUAGE");
-	if (retval && *retval)
-		return retval;
-
-#ifndef NO_GETTEXT
-	retval = setlocale(LC_MESSAGES, NULL);
-	if (retval && *retval &&
-		strcmp(retval, "C") &&
-		strcmp(retval, "POSIX"))
-		return retval;
-#endif
-
-	return NULL;
-}
-
 static void write_accept_language(struct strbuf *buf)
 {
 	/*

-- 
2.3.0.449.g1690e78

On Thu, Feb 26, 2015 at 2:07 PM, Jeff King [off-list ref] wrote:

Here it is, with the commit message and the missing hunk. This works for
me both with and without NO_GETTEXT defined.

This compiles here though a warning is spit:
In file included from cache.h:8:0,
                 from userdiff.c:1:
gettext.h:92:1: warning: function declaration isn't a prototype
[-Wstrict-prototypes]
 const char *get_preferred_languages();
 ^
so I guess I can still add a
Tested-by: Stefan Beller <redacted>

On Thu, Feb 26, 2015 at 02:26:05PM -0800, Stefan Beller wrote:

On Thu, Feb 26, 2015 at 2:07 PM, Jeff King [off-list ref] wrote:

quoted

Here it is, with the commit message and the missing hunk. This works for
me both with and without NO_GETTEXT defined.

This compiles here though a warning is spit:
In file included from cache.h:8:0,
                 from userdiff.c:1:
gettext.h:92:1: warning: function declaration isn't a prototype
[-Wstrict-prototypes]
 const char *get_preferred_languages();
 ^

Hmph. The compiler is right that it should be:

 const char *get_preferred_languages(void);

but my gcc (4.9.2, with -Wstrict_prototypes) does not seem to notice it!
Weird.

-Peff

On Thu, Feb 26, 2015 at 05:36:03PM -0500, Jeff King wrote:

quoted

[-Wstrict-prototypes]
 const char *get_preferred_languages();
 ^

Hmph. The compiler is right that it should be:

 const char *get_preferred_languages(void);

but my gcc (4.9.2, with -Wstrict_prototypes) does not seem to notice it!
Weird.

Ugh. I have a snippet in my config.mak that relaxes the warnings on older
versions of git, and it was accidentally triggering due to a typo. :(

So that explains that. Junio, do you mind squashing in:

diff --git a/gettext.h b/gettext.h
index 5d8d2df..33696a4 100644
--- a/gettext.h
+++ b/gettext.h

@@ -89,6 +89,6 @@ const char *Q_(const char *msgid, const char *plu, unsigned long n)
 #define N_(msgid) (msgid)
 #endif
 
-const char *get_preferred_languages();
+const char *get_preferred_languages(void);
 
 #endif