Created attachment 112495 [details] [review]
[PATCH] Add NC_ as noop variant of C_. Bug #536996.

 docs/reference/glib/glib-sections.txt |    1 +
 docs/reference/glib/tmpl/i18n.sgml    |   28 ++++++++++++++++++++++++++++
 glib/gi18n-lib.h                      |    1 +
 glib/gi18n.h                          |    1 +
 4 files changed, 31 insertions(+), 0 deletions(-)

That doesn't really look right to me. There is not much point in defining
an NC_ macro that is identical to N_. zShouldn't the macro do

#define NC_(Context, Message) (Context "\004" Message)

?

Then you can either get the translation by using Q_(string) or
manual g_dpgettext (NULL, string, 0), and there is no need to repeat the 
context in two places in the code.

It's not identical to N_. The proposed NC_ is to N_ as C_ is to _; that is it takes 2 arguments instead of just one.

Using your #define for NC_, you will still need to put the context in two places in the code since you need its strlen + 1 as the offset in g_dpgettext.

The point of the no-op NC_ is to annotate the string with context for extraction by intltool, but IMHO the C code should just see the plain string. You want to use it like this:

static const char *some_strings[] = {
  NC_("my context", "thing 0"),
  NC_("my context", "thing 1"),
  ...
};

g_print ("The thing is: %s", pgettext ("my context", some_strings[i]));

i.e. the eventual call to actually translate the string might not involve a string literal for the 2nd argument.

Also with my proposed definition of NC_, the Context string ends up only once in the actual binary, within the pgettext call, instead of N times prepended to the individual NC_-marked strings.

g_dpgettext can handle 0 for the offset, no need for strlen.

Your example keeps using pgettext(), which is not readily available. We should find a solution that allows to get the translation with functions that are available in glib and glibc. 

g_dpgettext only handles 0 offset correctly with '|' separator, not with \004.

I'd like a NC_ variant that doesn't put the context N times into the binary...

pgettext exists since gettext 0.15 which is already 2 years old. However if you're concerned about other platforms not supporting it, we could just move our own copy of it from gtk/gtkbuilderparser.c into glib under a different name...

[mclasen@localhost glib]$ nm -D /lib/libc.so.6  | grep gettext
004523d0 T __dcgettext
00452420 T __dgettext
004523d0 W dcgettext
00453bc0 W dcngettext
00452420 W dgettext
00453c10 W dngettext
00452450 W gettext
00453c50 W ngettext

pgettext only 'exists' in the form of the header /usr/share/gettext/gettext.h

You're right; I should've read the gettext docs more closely. 

So I think a solution should have the following characteristics:
- NC_ should not put the context string into the programme together with the mssage ID, to avoid it occurring N times in the binary if it's repeated (like e.g. in gtk's paper names strings)
- when actually translating the NC_ marked string later, it should allow non-string-literals for either the context and/or the message strings

So how about "#define NC_(Context, String) (String)" as in my previous patch, plus moving the dpgettext function from gtkbuilderparser.c to glib and documenting that you should use that to eventually translate the NC_ marked strings?

> So how about "#define NC_(Context, String) (String)" as in my previous patch,
> plus moving the dpgettext function from gtkbuilderparser.c to glib and
> documenting that you should use that to eventually translate the NC_ marked
> strings?

Sounds like a plan. The one problem is that there is already a g_dpgettext() in gi18n.h, so the obvious name is taken

Oh, also: g_dpgettext (NULL, Context "\004" Msgid, 0) _should_ work, since the initial dgettext call would find the translation. the msgidoffset is only relevant in the fallback case.

While you are at it, both of you should also check bug 503071.  Thanks.

Created attachment 114209 [details] [review]
patch

Unfortunately I couldn't think of a better name than g_dpgettext2 ...

The docs for NC_ should have a paragraph talking about necessary xgettext arguments, similar to what we have for Q_ and C_.


+ * This uses g_dgettext() internally.  See that functions for differences
+ * with dgettext() proper.
+ *

Should probably be "See that function "

Might also be nice to add a sentence about the similarity and differences to g_dpgettext and about the fact that g_dpgettext2 is useful in situations where
you want to keep the message context separate from the message id. Could point to the example in the NC_() docs.

With those changes, and with adding the new symbol to glib.symbols and the new apis to glib-sections.txt, it is good to go in.

In retrospect, it would have been better to only have the g_dpgettext2
variant and avoid g_dpgettext. Too late now...


        Bug 536996 – Missing noop i18n macro equivalent to C_

        * glib/glib.symbols:
        * glib/gstrfuncs.[hc]: Add g_dpgettext2() which is a
        variant of g_dpgettext() taking context and id as separate
        arguments.

        * glib/gi18n-lib.h:
        * glib/gi18n.h: Add an NC_() macro that is to C_() as N_()
        is to _().