Re: [Libexif-devel] Changes to libexif-gtk for migration to gtk3

[Dan F. <da...@co...>]

On Mon, Nov 14, 2011 at 10:16:03PM +0000, Valere Monseur wrote:
> As discussed with Dan, here are the different diff files to apply to the cvs
> tree and a description of the content of each file.
> The diff files are located here: http://sourceforge.net/projects/vmonseur.u/
> files/libexif-gtk/

Thanks, Valere, for taking this on.

> Description updates1.diff
> 
>  summary: technical changes + internationalisation changes

This patch is the only one I can meaningfully review.  I'm hoping others on 
this list who are familiar with GTK will give the other patches.

After taking a look at this first patch, I have some questions about a few of
the hunks. 

diff -aurpN libexif-gtk-0.3.5-cvs/cvs/libexif-gtk/gtk-extensions/Makefile.am libexif-gtk-0.3.5-diff1/cvs/libexif-gtk/gtk-extensions/Makefile.am
 libgtk_extensions_la_CFLAGS = \
        $(AM_CFLAGS) $(CFLAGS)          \
-       -I$(top_srcdir)                 \
-       $(GTK_CFLAGS)
+       -I$(top_srcdir)                         \
+       $(GTK_CFLAGS)                           \
+       -DLOCALEDIR=\""$(localedir)"\"
+

-DLOCALEDIR is already set in configure.ac by GP_GETTEXT_HACK, isn't it?

-       -DG_LOG_DOMAIN=\"libexif\"
+       -DG_LOG_DOMAIN=\"libexif-gtk\"  \

Won't this break backward compatibility?  Or is this just for debugging?
Could one expect changing it to break any software?

diff -aurpN libexif-gtk-0.3.5-cvs/cvs/libexif-gtk/tests/test-libexif-gtk.c libexif-gtk-0.3.5-diff1/cvs/libexif-gtk/tests/test-libexif-gtk.c
+       bind_textdomain_codeset (GETTEXT_PACKAGE, "UTF-8");

Does GTK not operate in the current locale's character set? Is it fixed to
use UTF-8 no matter what the environment?  If so, then this should really
be called everywhere that bindtextdomain(GETTEXT_PACKAGE, LOCALEDIR)
is called from within libexif-gtk. Apps won't know this textual domain name,
anyway, since it's really an internal detail to libexif-gtk.

-       gtk_set_locale ();
-       textdomain (PACKAGE);
+       bind_textdomain_codeset (GETTEXT_PACKAGE, "UTF-8");
+       textdomain (GETTEXT_PACKAGE);

Since test-libexif-gtk.c itself doesn't use gettext, I wouldn't expect it to
be calling textdomain().  In fact, it seems like a better test to NOT
call it, so you can better ensure that the libexif-gtk library is doing
the right thing with gettext on its own. In addition, the whole #ifdef
ENABLE_NLS block at the head of that file should go.

However, I believe you'll need a call to setlocale(LC_ALL,"") to set
the locale. I'm guessing that's what gtk_set_locale() did, so that
part should probably stay. I think this diff chunk should probably look
like this:

        gtk_set_locale ();
-       textdomain (PACKAGE);

>>> Dan