Re: [PATCH] emacs should use GtkFileChooser widget

[Jan D.]

Some comments on the patch.


> Hi,
>
> Please excuse if this is the wrong way to submit patches. I just
> recently started to use the CVS build of emacs because of its use of
> the GTK+-2.x toolkit. I find it convenient but I think that there are
> a couple of issues and would like to submit patches for them. As a
> first start, here's a patch against CVS that makes emacs use the
> GtkFileChooser widget if compiled against GTK+ version 2.4 or newer.
>
>
> -----------------------------------------------------------------------
> Index: src/gtkutil.c
> ===================================================================
> RCS file: /cvsroot/emacs/emacs/src/gtkutil.c,v
> retrieving revision 1.41
> diff -u -p -r1.41 gtkutil.c
> --- src/gtkutil.c       28 Jun 2004 14:36:48 -0000      1.41
> +++ src/gtkutil.c       18 Jul 2004 20:21:08 -0000
> @@ -1159,6 +1159,10 @@ create_dialog (wv, select_cb, deactivate
>  }
...
> +
> +  dialog = gtk_file_chooser_dialog_new (prompt,
> +                                        GTK_WINDOW 
> (FRAME_GTK_OUTER_WIDGET (f)),
> +                                        (mustmatch_p ?
> +                                         GTK_FILE_CHOOSER_ACTION_OPEN 
> :
> +                                         
> GTK_FILE_CHOOSER_ACTION_SAVE),
> +
> +                                        GTK_STOCK_CANCEL, 
> GTK_RESPONSE_CANCEL,
> +                                        GTK_STOCK_SAVE,   
> GTK_RESPONSE_OK,
> +
> +                                        NULL);
> +

If mustmatch_p is true the file is required to exist, but in Emacs this 
does
not imply that an open operation is taking place.  It is common to open
a non-existing file, and the file is saved later.  Here mustmatch_p is
false, so GTK_STOCK_SAVE is inappropriate.  The best thing to do is to
use GTK_STOCK_OK regardless of mustmatch_p, since the GtkFileChooser has
no way to implement mustmatch_p the way Emacs wants.  The only file 
dialog
that does this correctly is Lesstif/Motif.  Mustmatch usually means that
Emacs knows the file name, but can't find it, so the user is supposed
to go to the directory where the file is.

But then there are other problems.  If we are to open a non existing 
file
we can't say ACTION_OPEN, because then there is no way for the user to
enter the name of the file, except to do Ctrl-L.  But Ctrl-L is 
unintuitive
so the user might not know about it.  Also, the dialog that pops up on
Ctrl-L is very annoying (Miles explained that very well in another 
mail).

So, how about using ACTION_SAVE then?  Well, then there is text in the 
dialog
that says "Save", and we are not saving, we are opening a non existent 
file.

All this is due to the bad design of the dialog.  I now remember why I 
put
off adding this new dialog to later.

Also, I can't get rid of the annoying

(emacs:21803): Gtk-CRITICAL **: file gtksettings.c: line 447 
(gtk_settings_get_for_screen): assertion `GDK_IS_SCREEN (screen)' 
failed

(emacs:21803): GLib-GObject-WARNING **: invalid (NULL) pointer instance

(emacs:21803): GLib-GObject-CRITICAL **: file gsignal.c: line 1726 
(g_signal_handler_disconnect): assertion `G_TYPE_CHECK_INSTANCE 
(instance)' failed

that GTK writes everytime the dialog is displayed.  I'm sure there is a 
GTK
bug in there somewhere, but I have put off finding a workaround until 
the
next Emacs release.

Emacs wants to use the file dialog as a tool to get a file name, and the
tool should not care if we are opening a new or old file, or saving to
a new or old file.  It is just a way for the user to enter a file name,
possibly assisted by the display of existing files.
The GtkFileChooser is not designed with this simple
thought in mind, so that is why it fails to be a good UI component.

        Jan D.