Created attachment 61575 [details] [review]
proposed fix

I also fixed a bug I spotted where bindtextdomain was called for the wrong domain name.

Ok to commit?

Created attachment 61576 [details] [review]
alternative patch

bindtextdomain is going to be called more than once otherwise, and it's not exactly a no-op. So this patch makes sure to only init once.

I'm not totally sure we need to not bind the codeset in some cases, but I remember sth about a problem with popt otherwise? Best to just bind it when using goption.

There are dozen calls of bindtextdomain over the code, isn't it better to drop them completely and use normal initialization of gettext bonobo_activation_init instead?

nsh: The problem is that if you use popt, you must NOT bind the textdomain codeset before parsing options, but only after; while with goption you must do so. So we need to ensure it's done correctly on every entry path...

I'd just trust Christian on this one :-) I commited a similar fix from him in libgnomeui.

Ping? Is is ok to commit this?

Comment on attachment 61576 [details] [review]
alternative patch

>? P
>? goption.diff
>? libbonobo-zip
>? p
>? doc/api/tmpl/libbonobo-unused.sgml
>? po/stamp-it
>Index: bonobo-activation/bonobo-activation-init.c
>===================================================================
>RCS file: /cvs/gnome/libbonobo/bonobo-activation/bonobo-activation-init.c,v
>retrieving revision 1.67
>diff -p -u -u -p -r1.67 bonobo-activation-init.c
>--- bonobo-activation/bonobo-activation-init.c	7 Feb 2006 12:24:04 -0000	1.67
>+++ bonobo-activation/bonobo-activation-init.c	19 Mar 2006 22:11:46 -0000
>@@ -366,11 +366,35 @@ static const GOptionEntry bonobo_activat
>         { NULL }
> };
> 
>+static void
>+init_gettext (gboolean bind_codeset)
>+{
>+	static gboolean initialized = FALSE;
>+#ifdef HAVE_BIND_TEXTDOMAIN_CODESET	
>+	static gboolean codeset_bound = FALSE;
>+#endif
>+
>+	if (!initialized)
>+	{
>+		bindtextdomain (GETTEXT_PACKAGE, BONOBO_ACTIVATION_LOCALEDIR);
>+		initialized = TRUE;
>+	}
>+#ifdef HAVE_BIND_TEXTDOMAIN_CODESET	
>+	if (bind_codeset && !codeset_bound)
>+	{
>+		bind_textdomain_codeset (GETTEXT_PACKAGE, "UTF-8");
>+		codeset_bound = TRUE;
>+	}
>+#endif
>+}
>+
> GOptionGroup *
> bonobo_activation_get_goption_group (void)
> {
> 	GOptionGroup *group;
> 
>+	init_gettext (TRUE);
>+
> 	group = g_option_group_new ("bonobo-activation",
> 				    N_("Bonobo Activation"),
> 				    N_("Show Bonobo Activation options"),
>@@ -405,7 +429,7 @@ bonobo_activation_ior_fd_get (void)
> void
> bonobo_activation_preinit (gpointer app, gpointer mod_info)
> {
>-        bindtextdomain (GETTEXT_PACKAGE, BONOBO_ACTIVATION_LOCALEDIR);
>+	init_gettext (FALSE);
> }
> 
> void
>@@ -467,7 +491,7 @@ bonobo_activation_is_initialized (void)
> char *
> bonobo_activation_get_popt_table_name (void)
> {
>-        bindtextdomain (PACKAGE, BONOBO_ACTIVATION_LOCALEDIR);
>+	init_gettext (FALSE);
>         return _("Bonobo activation options");
> }
> 
Other than the two tabs vs spaces things in the last two modified lines it looks good. Please change that and commit.

Looks like predominant style in that file is to use tabs, so I fixed up the surrounding line (which used spaces) instead :)

2006-05-03  Christian Persch  <chpe@cvs.gnome.org>

        * bonobo-activation/bonobo-activation-init.c: (init_gettext),
        (bonobo_activation_get_goption_group), (bonobo_activation_preinit),
        (bonobo_activation_get_popt_table_name): Make sure to initialise
        the textdomain codeset if we're using goption. Bug #335159.