Re: Using Qunsupported__w32_dialog

[Eli Zaretskii <eliz@gnu.org>]

> Date: Wed, 04 Jun 2014 17:37:46 +0400
> From: Dmitry Antipov <dmantipov@yandex.ru>
> CC: emacs-devel@gnu.org
> 
> On 06/04/2014 05:09 PM, Eli Zaretskii wrote:
> 
> > Therefore, the snippet above should instead say something like this:
> >
> >    /* Display the popup dialog by a terminal-specific hook ... */
> >    if (FRAME_TERMINAL (f)->popup_dialog_hook)
> >      {
> >        Lisp_Object val =
> >          FRAME_TERMINAL (f)->popup_dialog_hook (f, header, contents);
> >
> >        if (!EQ (val, Qunsupported__w32_dialog))
> >          return val;
> >      }
> >
> >    /* ... or emulate it with a menu.  */
> >    return emulate_dialog_with_menu (f, contents);
> >
> > And I think this means Qunsupported__w32_dialog cannot be static.
> 
> Hm... I would like to see Qunsupported__w32_dialog as a local
> Windows-specific workaround, and let w32_popup_dialog call
> emulate_dialog_with_menu in case of unsupported dialog structure.

Why?  What does this gain us?

> So, to preserve the proposed end of Fx_popup_dialog:
> 
> /* Display the popup dialog by a terminal-specific hook ... */
> if (FRAME_TERMINAL (f)->popup_dialog_hook)
>    return FRAME_TERMINAL (f)->popup_dialog_hook (f, header, contents);
> 
> /* ... or emulate it with a menu.  */
> return emulate_dialog_with_menu (f, contents);
> 
> MS-Windows stuff should be:
> 
> Lisp_Object
> w32_popup_dialog (struct frame *f, Lisp_Object header, Lisp_Object contents)
> {
>    Lisp_Object selection;
> 
>    check_window_system (f);
> 
> #ifndef HAVE_DIALOGS
>    /* Handle simple Yes/No choices as MessageBox popups.  */
>    if (is_simple_dialog (contents))
>      selection = simple_dialog_show (f, contents, header);
>    else
>      selection = Qunsupported__w32_dialog;
> #else  /* HAVE_DIALOGS */
>      {
>        Lisp_Object title;
>        char *error_name;
> 
>        /* Decode the dialog items from what was specified.  */
>        title = Fcar (contents);
>        CHECK_STRING (title);
> 
>        list_of_panes (Fcons (contents, Qnil));
> 
>        /* Display them in a dialog box.  */
>        block_input ();
>        selection = w32_dialog_show (f, title, header, &error_name);
>        unblock_input ();
> 
>        discard_menu_items ();
>        FRAME_DISPLAY_INFO (f)->grabbed = 0;
> 
>        if (error_name) error (error_name);
>      }
> #endif /* HAVE_DIALOGS */
>      return (EQ (selection, Qunsupported__w32_dialog) ?
> 	    emulate_dialog_with_menu (f, contents) : selection);
> }
> 
> IIUC this should have the same behavior as the old code, isn't it?

More or less, but how is this better?  For starters, you now have
emulate_dialog_with_menu extern instead of static, and it is called
from 2 places rather than one.  I don't see the gain here.

I can put the explanation of why we need Qunsupported__w32_dialog in
comments, if that would help.

Btw, the part of your patch that does this:

@@ -148,6 +149,8 @@
       FRAME_DISPLAY_INFO (f)->grabbed = 0;
 
       if (error_name) error (error_name);
+      if (EQ (selection, Qunsupported__w32_dialog))
+	return emulate_dialog_with_menu (f, contents);
       return selection;
     }
 #endif /* HAVE_DIALOGS */

is unnecessary: the HAVE_DIALOGS code will never return this special
value.