bug#61553: 29.0.60; Inconsistent use of dialog boxes by read-multiple-choice

bug#61553: 29.0.60; Inconsistent use of dialog boxes by read-multiple-choice

[Augusto Stoffel]

In the scratch buffer of emacs -Q, type

    (read-multiple-choice "Question" '((?y "yes") (?n "no")))

then click, on the menu bar, "Lisp-Interaction -> Evaluate and Print".
As expected, I see a dialog box.

Now repeat the same using the long-form style:

    (read-multiple-choice "Question" '((?y "yes") (?n "no")) nil nil t)

Then I get a minibuffer query, but I would expect a dialog box in the
case as well.

As a more concrete example, when clicking the "File -> Close" menu item
in a modified buffer, I would expect a to always get a dialog box for
the "buffer modified, kill anyway?" question.  (assuming of course
use-dialog-box is t).  Currently, one gets a minibuffer query by
default.  This change from mouse interaction to keyboard interaction
seems unexpected to me.

bug#61553: 29.0.60; Inconsistent use of dialog boxes by read-multiple-choice

[Eli Zaretskii]

> From: Augusto Stoffel <arstoffel@gmail.com>
> Date: Thu, 16 Feb 2023 17:19:30 +0100
> 
> 
> In the scratch buffer of emacs -Q, type
> 
>     (read-multiple-choice "Question" '((?y "yes") (?n "no")))
> 
> then click, on the menu bar, "Lisp-Interaction -> Evaluate and Print".
> As expected, I see a dialog box.
> 
> Now repeat the same using the long-form style:
> 
>     (read-multiple-choice "Question" '((?y "yes") (?n "no")) nil nil t)
> 
> Then I get a minibuffer query, but I would expect a dialog box in the
> case as well.

The long-form call does a completing-read, and we don't support that
via GUI dialogs (how could we?).

> As a more concrete example, when clicking the "File -> Close" menu item
> in a modified buffer, I would expect a to always get a dialog box for
> the "buffer modified, kill anyway?" question.  (assuming of course
> use-dialog-box is t).  Currently, one gets a minibuffer query by
> default.  This change from mouse interaction to keyboard interaction
> seems unexpected to me.

Does the patch below give good results in that case?

diff --git a/lisp/emacs-lisp/rmc.el b/lisp/emacs-lisp/rmc.el
index 542c965..8ab1f86 100644
--- a/lisp/emacs-lisp/rmc.el
+++ b/lisp/emacs-lisp/rmc.el
@@ -177,8 +177,9 @@ read-multiple-choice
      prompt choices help-string show-help)))
 
 (defun read-multiple-choice--short-answers (prompt choices help-string show-help)
-  (let* ((prompt-choices
-          (if show-help choices (append choices '((?? "?")))))
+  (let* ((dialog-p (use-dialog-box-p))
+         (prompt-choices
+          (if (or show-help dialog-p) choices (append choices '((?? "?")))))
          (altered-names (mapcar #'rmc--add-key-description prompt-choices))
          (full-prompt
           (format
@@ -192,11 +193,12 @@ read-multiple-choice--short-answers
             (setq buf (rmc--show-help prompt help-string show-help
                                       choices altered-names)))
 	(while (not tchar)
-	  (message "%s%s"
-                   (if wrong-char
-                       "Invalid choice.  "
-                     "")
-                   full-prompt)
+          (unless dialog-p
+	    (message "%s%s"
+                     (if wrong-char
+                         "Invalid choice.  "
+                       "")
+                     full-prompt))
           (setq tchar
                 (if (and (display-popup-menus-p)
                          last-input-event ; not during startup
diff --git a/lisp/simple.el b/lisp/simple.el
index c58acfe..d0ba7dc 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -10799,7 +10799,8 @@ kill-buffer--possibly-save
            '((?y "yes" "kill buffer without saving")
              (?n "no" "exit without doing anything")
              (?s "save and then kill" "save the buffer and then kill it"))
-           nil nil (not use-short-answers)))))
+           nil nil (and (not use-short-answers)
+                        (not (use-dialog-box-p)))))))
     (if (equal response "no")
         nil
       (unless (equal response "yes")

bug#61553: 29.0.60; Inconsistent use of dialog boxes by read-multiple-choice

[Augusto Stoffel]

On Thu, 16 Feb 2023 at 19:59, Eli Zaretskii wrote:

>> From: Augusto Stoffel <arstoffel@gmail.com>
>> Date: Thu, 16 Feb 2023 17:19:30 +0100
>> 
>> 
>> In the scratch buffer of emacs -Q, type
>> 
>>     (read-multiple-choice "Question" '((?y "yes") (?n "no")))
>> 
>> then click, on the menu bar, "Lisp-Interaction -> Evaluate and Print".
>> As expected, I see a dialog box.
>> 
>> Now repeat the same using the long-form style:
>> 
>>     (read-multiple-choice "Question" '((?y "yes") (?n "no")) nil nil t)
>> 
>> Then I get a minibuffer query, but I would expect a dialog box in the
>> case as well.
>
> The long-form call does a completing-read, and we don't support that
> via GUI dialogs (how could we?).

Of course.  The point is what takes precedence: the decision to prefer a
dialog over keyboard input, or the decision to do a completing-read
instead of reading a single char?

The purpose of long-form is to protect the user from doing something
dangerous by accidentally pressing a key.  I don't think a mouse
equivalent for that exists or is needed.

So instead of adding a special case for kill-buffer, I would rather
modify the behavior of RMC to just ignore the long-form argument if
(use-dialog-box-p) returns t.  Apart from that, your patch seems fine.

bug#61553: 29.0.60; Inconsistent use of dialog boxes by read-multiple-choice

[Eli Zaretskii]

> From: Augusto Stoffel <arstoffel@gmail.com>
> Cc: 61553@debbugs.gnu.org
> Date: Thu, 16 Feb 2023 19:36:36 +0100
> 
> On Thu, 16 Feb 2023 at 19:59, Eli Zaretskii wrote:
> 
> >>     (read-multiple-choice "Question" '((?y "yes") (?n "no")) nil nil t)
> >> 
> >> Then I get a minibuffer query, but I would expect a dialog box in the
> >> case as well.
> >
> > The long-form call does a completing-read, and we don't support that
> > via GUI dialogs (how could we?).
> 
> Of course.  The point is what takes precedence: the decision to prefer a
> dialog over keyboard input, or the decision to do a completing-read
> instead of reading a single char?

I don't think the function itself can make that decision.  Only the
caller knows what's right for the context.

> The purpose of long-form is to protect the user from doing something
> dangerous by accidentally pressing a key.

That's only one possible cause of using the long form.  There could be
others.

> So instead of adding a special case for kill-buffer, I would rather
> modify the behavior of RMC to just ignore the long-form argument if
> (use-dialog-box-p) returns t.  Apart from that, your patch seems fine.

I disagree that rmc.el should make that decision.  It isn't its call
(pun intended).

bug#61553: 29.0.60; Inconsistent use of dialog boxes by read-multiple-choice

[Robert Pluim]

>>>>> On Thu, 16 Feb 2023 22:17:18 +0200, Eli Zaretskii <eliz@gnu.org> said:

    >> So instead of adding a special case for kill-buffer, I would rather
    >> modify the behavior of RMC to just ignore the long-form argument if
    >> (use-dialog-box-p) returns t.  Apart from that, your patch seems fine.

    Eli> I disagree that rmc.el should make that decision.  It isn't its call
    Eli> (pun intended).

If we do this then we need to modify the docstring of
`read-multiple-choice', which explicitly defines the current
behaviour:

    When `use-dialog-box' is t (the default), and the command using this
    function was invoked via the mouse, this function pops up a GUI dialog
    to collect the user input, but only if Emacs is capable of using GUI
    dialogs.  Otherwise, the function will always use text-mode dialogs.

    The return value is the matching entry from the CHOICES list.

    If LONG-FORM, do a `completing-read' over the NAME elements in
    CHOICES instead.

Although perhaps we could clarify it:

    If LONG-FORM, always do a `completing-read' over the NAME elements in
    CHOICES instead, regardless of the value of `use-dialog-box'.



Robert
-- 

bug#61553: 29.0.60; Inconsistent use of dialog boxes by read-multiple-choice

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: Augusto Stoffel <arstoffel@gmail.com>,  61553@debbugs.gnu.org
> Date: Fri, 17 Feb 2023 11:24:09 +0100
> 
> >>>>> On Thu, 16 Feb 2023 22:17:18 +0200, Eli Zaretskii <eliz@gnu.org> said:
> 
>     >> So instead of adding a special case for kill-buffer, I would rather
>     >> modify the behavior of RMC to just ignore the long-form argument if
>     >> (use-dialog-box-p) returns t.  Apart from that, your patch seems fine.
> 
>     Eli> I disagree that rmc.el should make that decision.  It isn't its call
>     Eli> (pun intended).
> 
> If we do this then we need to modify the docstring of
> `read-multiple-choice', which explicitly defines the current
> behaviour:
> 
>     When `use-dialog-box' is t (the default), and the command using this
>     function was invoked via the mouse, this function pops up a GUI dialog
>     to collect the user input, but only if Emacs is capable of using GUI
>     dialogs.  Otherwise, the function will always use text-mode dialogs.
> 
>     The return value is the matching entry from the CHOICES list.
> 
>     If LONG-FORM, do a `completing-read' over the NAME elements in
>     CHOICES instead.

Where exactly is the above wrong?

The only problem I found in that function is that it evidently was
never actually tested with GUI dialogs, because trying to do that
revealed at least two (albeit minor) issues with what it did in that
case.  Both of them are solved in the patch I proposed.

> Although perhaps we could clarify it:
> 
>     If LONG-FORM, always do a `completing-read' over the NAME elements in
>     CHOICES instead, regardless of the value of `use-dialog-box'.

Oh, you assume that the reader will not understand that
completing-read cannot possibly use GUI dialogs?  I'm okay with saying
that explicitly, although someone who uses these APIs must already
realize that.

bug#61553: 29.0.60; Inconsistent use of dialog boxes by read-multiple-choice

[Robert Pluim]

>>>>> On Fri, 17 Feb 2023 14:31:06 +0200, Eli Zaretskii <eliz@gnu.org> said:

    >> From: Robert Pluim <rpluim@gmail.com>
    >> Cc: Augusto Stoffel <arstoffel@gmail.com>,  61553@debbugs.gnu.org
    >> Date: Fri, 17 Feb 2023 11:24:09 +0100
    >> 
    >> >>>>> On Thu, 16 Feb 2023 22:17:18 +0200, Eli Zaretskii <eliz@gnu.org> said:
    >> 
    >> >> So instead of adding a special case for kill-buffer, I would rather
    >> >> modify the behavior of RMC to just ignore the long-form argument if
    >> >> (use-dialog-box-p) returns t.  Apart from that, your patch seems fine.
    >> 
    Eli> I disagree that rmc.el should make that decision.  It isn't its call
    Eli> (pun intended).
    >> 
    >> If we do this then we need to modify the docstring of
    >> `read-multiple-choice', which explicitly defines the current
    >> behaviour:
    >> 
    >> When `use-dialog-box' is t (the default), and the command using this
    >> function was invoked via the mouse, this function pops up a GUI dialog
    >> to collect the user input, but only if Emacs is capable of using GUI
    >> dialogs.  Otherwise, the function will always use text-mode dialogs.
    >> 
    >> The return value is the matching entry from the CHOICES list.
    >> 
    >> If LONG-FORM, do a `completing-read' over the NAME elements in
    >> CHOICES instead.

    Eli> Where exactly is the above wrong?

Itʼs not wrong, it just could be clearer.

    Eli> The only problem I found in that function is that it evidently was
    Eli> never actually tested with GUI dialogs, because trying to do that
    Eli> revealed at least two (albeit minor) issues with what it did in that
    Eli> case.  Both of them are solved in the patch I proposed.

    >> Although perhaps we could clarify it:
    >> 
    >> If LONG-FORM, always do a `completing-read' over the NAME elements in
    >> CHOICES instead, regardless of the value of `use-dialog-box'.

    Eli> Oh, you assume that the reader will not understand that
    Eli> completing-read cannot possibly use GUI dialogs?  I'm okay with saying
    Eli> that explicitly, although someone who uses these APIs must already
    Eli> realize that.

I had to read it carefully to realize that the 'instead' referred to
the use of dialogs.

Robert
-- 

bug#61553: 29.0.60; Inconsistent use of dialog boxes by read-multiple-choice

[Eli Zaretskii]

> Cc: 61553@debbugs.gnu.org
> Date: Thu, 16 Feb 2023 22:17:18 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > From: Augusto Stoffel <arstoffel@gmail.com>
> > Cc: 61553@debbugs.gnu.org
> > Date: Thu, 16 Feb 2023 19:36:36 +0100
> > 
> > On Thu, 16 Feb 2023 at 19:59, Eli Zaretskii wrote:
> > 
> > >>     (read-multiple-choice "Question" '((?y "yes") (?n "no")) nil nil t)
> > >> 
> > >> Then I get a minibuffer query, but I would expect a dialog box in the
> > >> case as well.
> > >
> > > The long-form call does a completing-read, and we don't support that
> > > via GUI dialogs (how could we?).
> > 
> > Of course.  The point is what takes precedence: the decision to prefer a
> > dialog over keyboard input, or the decision to do a completing-read
> > instead of reading a single char?
> 
> I don't think the function itself can make that decision.  Only the
> caller knows what's right for the context.
> 
> > The purpose of long-form is to protect the user from doing something
> > dangerous by accidentally pressing a key.
> 
> That's only one possible cause of using the long form.  There could be
> others.
> 
> > So instead of adding a special case for kill-buffer, I would rather
> > modify the behavior of RMC to just ignore the long-form argument if
> > (use-dialog-box-p) returns t.  Apart from that, your patch seems fine.
> 
> I disagree that rmc.el should make that decision.  It isn't its call
> (pun intended).

No further comments, so I've now installed the proposed change on the
emacs-29 branch, and I'm closing this bug.

> From: Robert Pluim <rpluim@gmail.com>
> Cc: arstoffel@gmail.com,  61553@debbugs.gnu.org
> Date: Fri, 17 Feb 2023 13:42:35 +0100
> 
>     Eli> Oh, you assume that the reader will not understand that
>     Eli> completing-read cannot possibly use GUI dialogs?  I'm okay with saying
>     Eli> that explicitly, although someone who uses these APIs must already
>     Eli> realize that.
> 
> I had to read it carefully to realize that the 'instead' referred to
> the use of dialogs.

I've made this aspect more explicit in the doc string, thanks.