Thread (1 message) 1 message, 1 author, 2019-07-08

Re: [PATCH v2 1/1] gettext: always use UTF-8 on native Windows

From: Junio C Hamano <hidden>
Date: 2019-07-08 18:31:04

Ævar Arnfjörð Bjarmason [off-list ref] writes:
quoted
-#	ifdef HAVE_LIBCHARSET_H
+#	ifdef GIT_WINDOWS_NATIVE
+ ... new windows-only code ...
+#	elif defined HAVE_LIBCHARSET_H
 #		include <libcharset.h>
 #	else
 #		include <langinfo.h>
...
It looks to me that with this patch the HAVE_LIBCHARSET_H docs in
"Makefile" become wrong. Shouldn't those be updated too?
I do not think this change has much to do with HAVE_LIBCHARSET_H; it
inserts "regardless of what we have been doing, do this new thing
only and always on windows (persumably '... because libcharset would
not be useful on that platform')".  

Existing users of HAVE_LIBCHARSET_H and existing non-windows users
that did not use HAVE_LIBCHARSET_H are not affected, and whatever
Makefile documents the macro as still applies to them.
I wonder if it wouldn't be better to always compile this function, and
just have init_gettext_charset() switch between the two. We've moved
more towards that sort of thing (e.g. with pthreads). I.e. prefer
redundant compilation to ifdefing platform-only code (which then only
gets compiled there). See "HAVE_THREADS" in the code.
OK, so init_gettext_charset() is the only caller of locale_charset()
in our codebase, and we supply our own locale_charset() if we do not
have <libcharset.h>, either with nl_langinfo(), or with the code
introduced by the patch in question for windows.  Your suggestion is
to add a block of #ifdef cascade in init_gettext_charset() to call
locale_charset(), nl_langinfo(), or the windows-only code (perhaps
inlined right there)?

I am not sure.  We'd need the conditional inclusion of header files
depending on HAVE_LIBCHARSET_H etc. anyway, so...
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help