GNOME Bugzilla – Bug 589651
Add a gettext module
Last modified: 2009-08-11 21:56:18 UTC
Bindings for gettext, dgettext, ngettext, dngettext, and pgettext.
Created attachment 139184 [details] [review] Add a gettext module
*** Bug 589496 has been marked as a duplicate of this bug. ***
Larger scale: - Shouldn't you provide bindtextdomain() and textdomain() here? Otherwise, the whole thing isn't usable from a pure GJS program without some C glue. You could make bindtextdomain() automatically call bind_textdomain_codeset(), since everything breaks without that codeset binding - the code assumes UTF-8. - Either by docs or by providing JS glue, it seems like it would be a good idea to standardize how you are supposed use the raw functions. Are you supposed to do: const Gettext = imports.gettext; const _ = function(x) { return Gettext.dgettext("mydomain", x); } Or do we provide some magic glue as Dan suggsted in the gnome-shell localization patch, or... ? Stuff found in the patch (mostly nitpicks about error messages): - there is inconsistency in error messages and variable names about whether it's a "source" or a "msgid" - Similarly there is inconsistency between 'context' and 'domain' - "ngettext() takes three args, msgid1, msgid2, count" "count" isn't really a good description of the "n" in ngettext, is it? In all of these cases, probably stick to whatever the names are in the gettext docs - "dgettext() takes two arguments, domain and source" First comma woul dbe better as a colon "dngettext() takes four args, context msgid1, msgid2, count" A missing comma - "Message id must be a string" If you are going to expand "msgid" - should be "Message ID" + src_context = gjs_string_get_ascii_checked(context, argv[0]); + src_string = gjs_string_get_ascii_checked(context, argv[1]); + if (src_context == NULL) { + gjs_throw(context, "Context must be a string"); + return JS_FALSE; + } + if (src_string == NULL) { + gjs_throw(context, "Source must be a string"); + return JS_FALSE; + } Code oddly checks the two results backwards. (And why not put the checks right after the calls) "dpgettext() takes three arguments, domain context and source" Another missing comma + if (!JS_DefineFunction(context, module_obj, + "dgettext", + gjs_dgettext, + 1, GJS_MODULE_PROP_FLAGS)) + return JS_FALSE; Not sure exactly what nargs (the 1) is used for without looking at the spidermonkey code - it probably sets a minimum. But would be better to pass the right values in in any case.
Created attachment 139316 [details] [review] Add a gettext module Bindings for gettext, dgettext, ngettext, dngettext, and pgettext. We also expose a function "domain" which returns an "instance" of the gettext module specialized for a particular domain, to avoid having to pass the domain to every call to gettext; useful for code which is not in the default domain.
Thanks for the review; the above patch I filed before seeing your comments. Updating it now.
Created attachment 139321 [details] [review] Add a gettext module Bindings for gettext, dgettext, ngettext, dngettext, and pgettext. We also expose a function "domain" which returns an "instance" of the gettext module specialized for a particular domain, to avoid having to pass the domain to every call to gettext; useful for code which is not in the default domain.
In your bindtextdomain: + location = gjs_string_get_ascii_checked(context, argv[1]); you want gjs_string_to_filename(), right? (Note need to free result) I really think "Message id" should be "Message ID" +var domain = function(domain) { + return { + gettext: function(msgid) { + return dgettext(domain, msgid); + }, + + ngettext: function(msgid1, msgid2, n) { + return dngettext(domain, msgid1, msgid2, n); + }, + + pgettext: function(context, msgid) { + return dpgettext(domain, context, msgid); + } + } Indentation here is a bit screwy. I think you should have some docs somewhere. (I've intentionally avoided commenting on lack of test cases, since setting that up seems like a pain)) One place to put docs would just be in a block comment at the top of gettext.js. I'm a little unhappy at the need to define _(), N_(), etc, in every module that uses internationalization, but I don't see an obvious alternative. Could do eval(imports.gettext.defines()); // default domain eval(imports.gettext.defines("gnome-shell")); // specific domain is that better? Or too magic? Other than the small stuff above, I think this is otherwise good, but it needs a sign-off from a gjs maintainer.
Created attachment 139536 [details] [review] Add a gettext module Bindings for gettext, dgettext, ngettext, dngettext, and pgettext. We also expose a function "domain" which returns an "instance" of the gettext module specialized for a particular domain, to avoid having to pass the domain to every call to gettext; useful for code which is not in the default domain.
Added redundant comments just for clarity. diff --git a/modules/gettext-native.c b/modules/gettext-native.c +static JSBool +gjs_bindtextdomain(JSContext *context, + JSObject *obj, + uintN argc, + jsval *argv, + jsval *retval) +{ + const char *domain; + const char *location; Should probably add something like: if (argc != 2 || !JSVAL_IS_STRING(argv[0]) || !JSVAL_IS_STRING(argv[1])) { gjs_throw(context, "bindtextdomain() takes two arguments: the domain, and the locales location"); return JS_FALSE; } + domain = gjs_string_get_ascii_checked(context, argv[0]); + if (domain == NULL) + return JS_FALSE; + location = gjs_string_get_ascii_checked(context, argv[1]); + if (location == NULL) + return JS_FALSE; + bindtextdomain(domain, location); + /* Always use UTF-8; we assume it internally here */ + bind_textdomain_codeset(domain, "UTF-8"); + return JS_FALSE; +} + +static JSBool +gjs_gettext(JSContext *context, + JSObject *obj, + uintN argc, + jsval *argv, + jsval *retval) +{ + const char *msgid; + const char *translated; Check number and type of args here too. Same as above. + msgid = gjs_string_get_ascii_checked(context, argv[0]); + if (msgid == NULL) + return JS_FALSE; + translated = gettext(msgid); + return gjs_string_from_utf8(context, translated, -1, retval); It would be slightly better to do this: if (!gjs_string_from_utf8(context, translated, -1, retval)) return JS_FALSE; return JS_TRUE; As we're returning a JSBool. +} + +static JSBool +gjs_dgettext(JSContext *context, + JSObject *obj, + uintN argc, + jsval *argv, + jsval *retval) +{ + const char *domain; + const char *msgid; + const char *translated; Check number and types of args here. Same as above. + domain = gjs_string_get_ascii_checked(context, argv[0]); + if (domain == NULL) { + gjs_throw(context, "Domain must be a string"); Why are you doing gjs_throw here and not on the other cases? If you add the initial check of number of args and types this doesn't seem to be needed. + return JS_FALSE; + } + msgid = gjs_string_get_ascii_checked(context, argv[1]); + if (msgid == NULL) { + gjs_throw(context, "Message id must be a string"); Why gjs_throw? + return JS_FALSE; + } + + translated = dgettext(domain, msgid); + return gjs_string_from_utf8(context, translated, -1, retval); It would be slightly better to do this: if (!gjs_string_from_utf8(context, translated, -1, retval)) return JS_FALSE; return JS_TRUE; As we're returning a JSBool. +static JSBool +gjs_ngettext(JSContext *context, + JSObject *obj, + uintN argc, + jsval *argv, + jsval *retval) +{ + const char *msgid1; + const char *msgid2; + guint n; + const char *translated; Check number and types of args here. Same as above. + msgid1 = gjs_string_get_ascii_checked(context, argv[0]); + msgid2 = gjs_string_get_ascii_checked(context, argv[1]); + if (msgid1 == NULL || msgid2 == NULL) { + gjs_throw(context, "Message id must be a string"); + return JS_FALSE; + } + + if (!JSVAL_IS_INT(argv[2])) { + gjs_throw(context, "The count must be an integer"); Why gjs_throw? + return JS_FALSE; + } + n = JSVAL_TO_INT(argv[2]); + + translated = ngettext(msgid1, msgid2, n); + return gjs_string_from_utf8(context, translated, -1, retval); It would be slightly better to do this: if (!gjs_string_from_utf8(context, translated, -1, retval)) return JS_FALSE; return JS_TRUE; As we're returning a JSBool. +} + +static JSBool +gjs_dngettext(JSContext *context, + JSObject *obj, + uintN argc, + jsval *argv, + jsval *retval) +{ + const char *domain; + const char *msgid1; + const char *msgid2; + guint n; + const char *translated; Check number and types of args here. Same as above. + domain = gjs_string_get_ascii_checked(context, argv[0]); + if (domain == NULL) { + gjs_throw(context, "Domain must be a string"); Why gjs_throw? + return JS_FALSE; + } + msgid1 = gjs_string_get_ascii_checked(context, argv[1]); + msgid2 = gjs_string_get_ascii_checked(context, argv[2]); + if (msgid1 == NULL || msgid2 == NULL) { + gjs_throw(context, "Message id must be a string"); Why gjs_throw? + return JS_FALSE; + } + + if (!JSVAL_IS_INT(argv[2])) { + gjs_throw(context, "N must be an integer"); + return JS_FALSE; + } + n = JSVAL_TO_INT(argv[2]); + + translated = dngettext(domain, msgid1, msgid2, n); + return gjs_string_from_utf8(context, translated, -1, retval); It would be slightly better to do this: if (!gjs_string_from_utf8(context, translated, -1, retval)) return JS_FALSE; return JS_TRUE; As we're returning a JSBool. +} + +static JSBool +gjs_pgettext(JSContext *context, + JSObject *obj, + uintN argc, + jsval *argv, + jsval *retval) +{ + const char *src_context; + const char *msgid; + const char *translated; Check number and types of args here. Same as above. + src_context = gjs_string_get_ascii_checked(context, argv[0]); + msgid = gjs_string_get_ascii_checked(context, argv[1]); + if (src_context == NULL) { + gjs_throw(context, "Context must be a string"); + return JS_FALSE; + } + if (msgid == NULL) { + gjs_throw(context, "Message id must be a string"); + return JS_FALSE; + } + + translated = g_dpgettext2(NULL, src_context, msgid); + return gjs_string_from_utf8(context, translated, -1, retval); It would be slightly better to do this: if (!gjs_string_from_utf8(context, translated, -1, retval)) return JS_FALSE; return JS_TRUE; As we're returning a JSBool. +} + +static JSBool +gjs_dpgettext(JSContext *context, + JSObject *obj, + uintN argc, + jsval *argv, + jsval *retval) +{ + const char *domain; + const char *src_context; + const char *msgid; + const char *translated; Check number and types of args here. Same as above. + domain = gjs_string_get_ascii_checked(context, argv[0]); + if (domain == NULL) { + gjs_throw(context, "Domain must be a string"); Why gjs_throw? + return JS_FALSE; + } + src_context = gjs_string_get_ascii_checked(context, argv[1]); + if (src_context == NULL) { + gjs_throw(context, "Context must be a string"); Why gjs_throw? + return JS_FALSE; + } + msgid = gjs_string_get_ascii_checked(context, argv[2]); + if (msgid == NULL) { + gjs_throw(context, "Message id must be a string"); Why gjs_throw? + return JS_FALSE; + } + + translated = g_dpgettext2 (domain, src_context, msgid); + return gjs_string_from_utf8(context, translated, -1, retval); It would be slightly better to do this: if (!gjs_string_from_utf8(context, translated, -1, retval)) return JS_FALSE; return JS_TRUE; As we're returning a JSBool. +} + +JSBool +gjs_define_gettext_api(JSContext *context, + JSObject *module_obj) We're naming those functions like gjs_define_gettext_stuff. I tend to prefer gjs_define_*_api but let's keep it consistent. +{ + if (!JS_DefineFunction(context, module_obj, + "bindtextdomain", + gjs_bindtextdomain, + 1, GJS_MODULE_PROP_FLAGS)) + return JS_FALSE; + + if (!JS_DefineFunction(context, module_obj, + "gettext", + gjs_gettext, + 1, GJS_MODULE_PROP_FLAGS)) + return JS_FALSE; + + if (!JS_DefineFunction(context, module_obj, + "dgettext", + gjs_dgettext, + 2, GJS_MODULE_PROP_FLAGS)) + return JS_FALSE; + + if (!JS_DefineFunction(context, module_obj, + "ngettext", + gjs_ngettext, + 3, GJS_MODULE_PROP_FLAGS)) + return JS_FALSE; + + if (!JS_DefineFunction(context, module_obj, + "dngettext", + gjs_dngettext, + 4, GJS_MODULE_PROP_FLAGS)) + return JS_FALSE; + + if (!JS_DefineFunction(context, module_obj, + "pgettext", + gjs_pgettext, + 2, GJS_MODULE_PROP_FLAGS)) + return JS_FALSE; + + if (!JS_DefineFunction(context, module_obj, + "dpgettext", + gjs_dpgettext, + 3, GJS_MODULE_PROP_FLAGS)) + return JS_FALSE; + + return JS_TRUE; +} + +GJS_REGISTER_NATIVE_MODULE("gettextNative", gjs_define_gettext_api) diff --git a/modules/gettext-native.h b/modules/gettext-native.h new file mode 100644 index 0000000..f6bed51 --- /dev/null +++ b/modules/gettext-native.h @@ -0,0 +1,39 @@ +/* -*- mode: C; c-basic-offset: 4; indent-tabs-mode: nil; -*- */ +/* + * Copyright (c) 2009 Red Hat, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to + * deal in the Software without restriction, including without limitation the + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or + * sell copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + */ + +#ifndef __GJS_GETTEXT_H__ +#define __GJS_GETTEXT_H__ + +#include <config.h> +#include <glib.h> + +#include <jsapi.h> + +G_BEGIN_DECLS + +JSBool gjs_define_gettext_api (JSContext *context, + JSObject *in_object); Rename to gjs_define_gettext_stuff. + +G_END_DECLS + +#endif /* __GJS_MAINLOOP_H__ */ diff --git a/modules/gettext.js b/modules/gettext.js new file mode 100644 index 0000000..b0e9fdf --- /dev/null +++ b/modules/gettext.js @@ -0,0 +1,42 @@ +// Copyright 2009 Red Hat, Inc. All Rights Reserved. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to +// deal in the Software without restriction, including without limitation the +// rights to use, copy, modify, merge, publish, distribute, sublicense, and/or +// sell copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS +// IN THE SOFTWARE. + +const Lang = imports.lang; + +// Merge stuff defined in native code +Lang.copyProperties(imports.gettextNative, this); + +// Return an instance of the gettext module set up to use +// a particular translation domain. +var domain = function(domain) { Why not: function domain(domain) { ... } It's a bit confusing that domain is the domain name and a function here. Maybe rename domain arg to something else? Just checking: is it supposed to be used like: const Gettext = imports.gettext; CustomGettext = Gettext.domain("nondefaultdomain"); CustomGettext.gettext("message to be translated"); + return { + gettext: function(msgid) { + return dgettext(domain, msgid); + }, + + ngettext: function(msgid1, msgid2, n) { + return dngettext(domain, msgid1, msgid2, n); + }, + + pgettext: function(context, msgid) { + return dpgettext(domain, context, msgid); + } Should probably indent the function definitions one level up.
Don't understand this comment: > if (!gjs_string_from_utf8(context, translated, -1, retval)) > return JS_FALSE; > > return JS_TRUE; > > As we're returning a JSBool. gjs_string_From_utf8 does return a JSBool.
Created attachment 139987 [details] [review] Add gjs_parse_args Add a utility function to extract arguments from JavaScript to C, generating useful error messages automatically. To implement this, extract out the core of the string conversion function gjs_string_to_utf8 into gjs_try_string_to_utf8, which takes a GError parameter. This allows us to append any conversion error message into our generated exception sanely, i.e. without having to check for and parse the currently pending JS exception. Do similar for gjs_string_to_filename.
Created attachment 139988 [details] [review] Add gjs_parse_args Add a utility function to extract arguments from JavaScript to C, generating useful error messages automatically. To implement this, extract out the core of the string conversion function gjs_string_to_utf8 into gjs_try_string_to_utf8, which takes a GError parameter. This allows us to append any conversion error message into our generated exception sanely, i.e. without having to check for and parse the currently pending JS exception. Do similar for gjs_string_to_filename.
Created attachment 139989 [details] [review] Add a gettext module Bindings for gettext, dgettext, ngettext, dngettext, and pgettext. We also expose a function "domain" which returns an "instance" of the gettext module specialized for a particular domain, to avoid having to pass the domain to every call to gettext; useful for code which is not in the default domain.
In the star trek future a lot of arg.c would be replaced by gjs_try_* functions, but I didn't want to rock the boat hugely right now. We can do that incrementally later.
(In reply to comment #10) > Don't understand this comment: > > > if (!gjs_string_from_utf8(context, translated, -1, retval)) > > return JS_FALSE; > > > > return JS_TRUE; > > > > As we're returning a JSBool. > > gjs_string_From_utf8 does return a JSBool. My bad. Ignore those.
(In reply to comment #12) > Created an attachment (id=139988) [edit] > Add gjs_parse_args > > Add a utility function to extract arguments from JavaScript to > C, generating useful error messages automatically. > > To implement this, extract out the core of the string conversion > function gjs_string_to_utf8 into gjs_try_string_to_utf8, which > takes a GError parameter. This allows us to append any > conversion error message into our generated exception sanely, > i.e. without having to check for and parse the currently > pending JS exception. > > Do similar for gjs_string_to_filename. This looks like a nice utility that should make it a lot easier to write code that does solid checking and throws useful exception on error. +gboolean +gjs_try_string_to_utf8 (JSContext *context, + const jsval string_val, + char **utf8_string_p, + GError **error) I tried to think of a more descriptive name for this, butt gjs_string_to_utf8_with_g_error() is just too clunky, so OK. +void +gjs_throw_gerror (JSContext *context, + GError *error) Probably should be gjs_throw_g_error(). Was inspired to file Bug 591480 – Allow accessing code/domain from thrown GErrors, but that's separate. - GError *error; + GError *convert_error = NULL; [ code passes ] - error = NULL; My impression is that moving the initialization away from the code is considered unstylish in gjs-land, though I'm generally OK with it myself for stuff like this. +gboolean +gjs_try_string_to_filename(JSContext *context, + const jsval filename_val, + char **filename_string_p, + GError **error) +{ [....] + if (!gjs_string_to_utf8(context, filename_val, &tmp)) { + /* exception already set */ + return JS_FALSE; + } This should be changed to gjs_try_string_to_utf8(), right?, since this function is supposed to fail with a GError not throw? JSBool gjs_string_to_filename(JSContext *context, const jsval filename_val, char **filename_string_p) { - GError *error; [ more deleted lines ] - return JS_TRUE; + GError *error = NULL; + if (error) { + gjs_throw(context, + "Could not convert filename to UTF8: '%s'", + error->message); + g_error_free(error); + return JS_FALSE; + } + return JS_TRUE; } Think you forgot to actually do anything in this function :-) +/** + * gjs_parse_args: + * @context: + * @function_name: The name of the function being called + * @format: Printf-like format specifier containing the expected arguments + * @argc: Number of JavaScript arguments + * @argv: JavaScript argument array + * + * This function is inspired by Python's PyArg_ParseTuple for those + * familiar with it. It takes a format specifier which gives the + * types of the expected arguments, and a list of argument name and + * value pairs. The currently accepted format specifiers are: You need to describe the vararg list a bit more here, I couldn't figure it out without looking at the code. Maybe: @Varargs: for each character in @format, a pair of a char * which is the name of the argument, and a pointer to a location to store the value. The type of value stored depends on the format character, as described below. You are missing docs for 'F' + GSList *unwind_strings = NULL; This is performance critical enough (meant to be used everywhere) I think that I'd avoid the GSList allocations. There's no need here to deal with formats of arbitrary length. If you used a heap-allocated array of length 16 that would be *plenty*. (and add an assertion) gpointer arg_location[i] = arg_location; is then enough to do the unwind. + gjs_throw(context, "Error invoking %s: Expected %d arguments, got %d", function_name, + strlen (fmt_iter), argc); + gjs_throw(context, "Error invoking %s, at argument %s: Couldn't convert to integer", I think it might be more useful if all these messages said argument %d (%s) [with %d being 1-based] that will make the case where you got the argument order wrong easier to figure out. + if (!JS_ValueToInt32(context, js_value, (gint32*) arg_location)) { + gjs_throw(context, "Error invoking %s, at argument %s: Couldn't convert to integer", + function_name, argname); + goto error_unwind; + } You need JS_ClearPendingException() here assuming that you don't care that much about why the integer conversion failed - gjs_throw() won't overwrite the exception set by JS_ValueToInt32. [Need to check that JS_ValueToInt32 does throw - the docs sure imply it ] + if (!JS_ValueToNumber(context, js_value, &num)) + goto error_unwind; This has the opposite problem - whatever ValueToNumber threw will be thrown back without any context. I think it's better to clear the exception and throw something more generic than to throw a specific error message without context. + return JS_TRUE; + error_unwind: + va_end (args); I try hard to avoid making comments about the absence of blank in return blocks, since it is (to some extent) personal preference. But this one *badly* needs a blank line before the error_unwind: label.
(In reply to comment #13) > Created an attachment (id=139989) [edit] > Add a gettext module > > Bindings for gettext, dgettext, ngettext, dngettext, and pgettext. > > We also expose a function "domain" which returns an "instance" of > the gettext module specialized for a particular domain, to avoid > having to pass the domain to every call to gettext; useful > for code which is not in the default domain. Certainly much easier to check for correctness of argument parsing this way! + if (!gjs_parse_args (context, "dgettext", "zs", argc, argv, "domain", &domain, + "msgid", &msgid, NULL)) Would be nice to have consistent line-breaking for all gjs_parse_args() rather than ad-hoc "what fits" that's different everywhere. if (!gjs_parse_args (context, "dgettext", "zs", argc, argv, "domain", &domain, "msgid", &msgid, NULL); It's much easier that way to visually match up the format string with the arguments. You don't need the trailing NULLs, though, do you? + guint n; + if (!gjs_parse_args (context, "ngettext", "ssu", argc, argv, "msgid1", &msgid1, + "msgid2", &msgid2, "n", &n, NULL)) You documented in the other patch that 'u' was 'guint32' - I don't think it matters much whether you make i/u gint/guint or gint32/guint32... the intersection of platforms we run on and platforms where they differ is empty and will likely stay that way. But we should be consistent. I still think the block comment of at the top of gettext.js should be expanded to show defining a _() function and using it. Unless we think it's wrong and number should ever use _(). Because if we don't define a style, everybody will do something different, and it will suck. Otherwise looks good to me.
Created attachment 140488 [details] [review] Add gjs_parse_args Add a utility function to extract arguments from JavaScript to C, generating useful error messages automatically. To implement this, extract out the core of the string conversion function gjs_string_to_utf8 into gjs_try_string_to_utf8, which takes a GError parameter. This allows us to append any conversion error message into our generated exception sanely, i.e. without having to check for and parse the currently pending JS exception. Do similar for gjs_string_to_filename.
Created attachment 140489 [details] [review] Add a gettext module Bindings for gettext, dgettext, ngettext, dngettext, and pgettext. We also expose a function "domain" which returns an "instance" of the gettext module specialized for a particular domain, to avoid having to pass the domain to every call to gettext; useful for code which is not in the default domain.
Thanks for the great review, you caught a lot of important issues. These updated patches should fix them all.
Didn't do a full rereview, just looked at the unwind_strings change: + unwind_strings[n_unwind++] = *arg; + g_assert(n_unwind < MAX_UNWIND_STRINGS); That's going to assert if you have MAX_UNWINDOW_STRINGS strings, which is OK. g_assert(n_unwind < MAX_UNWIND_STRINGS); unwind_strings[n_unwind++] = *arg; Seems better.