Re: Suggested patch for improved locale handling on Win32

[Owen Taylor <otaylor redhat com>]

On Thu, 2003-09-18 at 20:27, Tor Lillqvist wrote:
> Basically this just splits out the ifdefified code snipped already
> present in gtk_get_default_language() into a new internal GTK
> function, _gtk_get_lc_ctype(). That function is then also called from
> gtk_im_multicontext_get_slave() instead of setlocale().
> 
> (On Unix, the code snippet calls setlocale(LC_CTYPE, NULL), and on
> Windows it checks for LC_ALL, LC_CTYPE and LANG environment variables
> (for Unix compatibilty; those envvars aren't used by the C library on
> Windows), and if not found, then calls g_win32_getlocale() which uses
> the Win32 API to get the thread's locale and translates it into an
> en_US -style locale string.)
> 
> Without this, the locale-specific "slave" input methods (is that the
> correct terminology?)

Well, yes, it's right terminology for gtkimmulticontext. I wouldn't
really use it in the context of all of GTK+.

> , mainly the imcedilla module, won't work. (A
> small fix to gdk/win32/gdkkeys-win32.c is also needed for it to work,
> otherwise the single/double quote key on the US-International keyboard
> won't be correctly translated into the correct dead GDK keysyms.)
> 
> g_win32_getlocale() currently looks for LC_MESSAGES (After LC_ALL and
> before LANG). it probably should be changed to look for LC_CTYPE
> instead. Then _gtk_get_lc_ctype() could just call it, and not look
> itself for the envvars.

I guess it depends on what it is used for. To change it from returning
the message locale to returning the CTYPE locale seems a bit dubious
to me in terms of compatibility.

> OK to commit?

I think there are memory leak problems with your patch. Your new
function returns memory that must be freed, which is different
from setlocale(LC_CTYPE, NULL), so you can't just make a single
line substitution.

> Index: gtkimmulticontext.c
> ===================================================================
> RCS file: /cvs/gnome/gtk+/gtk/gtkimmulticontext.c,v
> retrieving revision 1.25.2.3
> diff -u -4 -r1.25.2.3 gtkimmulticontext.c
> --- gtkimmulticontext.c	19 Aug 2003 19:24:50 -0000	1.25.2.3
> +++ gtkimmulticontext.c	18 Sep 2003 22:20:54 -0000
> @@ -23,8 +23,9 @@
>  #include <locale.h>
>  
>  #include "gtkimmulticontext.h"
>  #include "gtkimmodule.h"
> +#include "gtkmain.h"
>  #include "gtkradiomenuitem.h"
>  #include "gtkintl.h"
>  
>  struct _GtkIMMulticontextPrivate
> @@ -254,9 +255,9 @@
>        if (!global_context_id)
>  	{
>  	  const char *locale;
>  	  
> -	  locale = setlocale (LC_CTYPE, NULL);
> +	  locale = _gtk_get_lc_ctype ();
>  
>  	  global_context_id = _gtk_im_module_get_default_context_id (locale);
>  	}
>  	
> Index: gtkmain.c
> ===================================================================
> RCS file: /cvs/gnome/gtk+/gtk/gtkmain.c,v
> retrieving revision 1.222.2.2
> diff -u -4 -r1.222.2.2 gtkmain.c
> --- gtkmain.c	24 Feb 2003 20:29:39 -0000	1.222.2.2
> +++ gtkmain.c	18 Sep 2003 22:20:56 -0000
> @@ -988,67 +988,105 @@
>   * program environment. This is the same as calling the C library function
>   * <literal>setlocale (LC_ALL, "")</literal> but also takes care of the 
>   * locale specific setup of the windowing system used by GDK.
>   * 
> - * Return value: a string corresponding to the locale set, as with the
> - * C library function <function>setlocale()</function>.
> + * Return value: a string corresponding to the locale set. On Unix,
> + * this is as with the C library function
> + * <function>setlocale()</function>, typically consisting of an
> + * ISO3166 language code and optionally an ISO639 country code. Also
> + * on Windows the return value is a Unix style string with language
> + * and country codes, even if the C library uses a different
> + * convention, with language and country names spelled out in English.

The language here needs some help. Can I suggest:

 Return: a string corresponding to the locale set, typically in the 
 form lang_COUNTRY, where lang is an ISO-3166 language code, and 
 COUNTRY an ISO-639 country code. On Unix, this form matches the
 result of the <function>setlocale()</function>; it is also used
 on other machines, such as Windows, where the C library returns
 a different result.

> + * Return value: a string private to GTK, must not be tampered with.

The typical language for this is 'should not be modified or freed'

>   **/
>  gchar *
>  gtk_set_locale (void)
>  {
>    return gdk_set_locale ();
>  }

Basically looks OK to commit, other than the memory leak issue.

Regards,
						Owen