bug#45412: File ... is large (... MiB), really open? (y)es or (n)o or (l)iterally

[Eli Zaretskii <eliz@gnu.org>]

> Date: Thu, 22 Apr 2021 00:12:54 +0200
> From:  Daniel Martín via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> -(defun read-multiple-choice (prompt choices)
> +(defun read-multiple-choice (prompt choices &optional help-string)
>    "Ask user a multiple choice question.
>  PROMPT should be a string that will be displayed as the prompt.
>  
> @@ -35,15 +35,18 @@ read-multiple-choice
>  be displayed while prompting (if there's room, it might be
>  shortened).  DESCRIPTION is an optional longer explanation that
>  will be displayed in a help buffer if the user requests more
> -help.
> +help.  This help description has a fixed format in columns, but
> +the user can control the text that is displayed and how it is
> +formatted with optional argument HELP-STRING.

This leaves it unsaid what HELP-STRING is (is it a string? a list? a
function? something else?) and how it is used by the function.

> +If the user enters `recenter', `scroll-up', or `scroll-down'
> +responses, perform the requested window recentering or scrolling
> +and ask again.  If the user enters `edit', a recursive edit is
> +started.

The last sentence (ab)uses the passive tense, and thus reads
awkwardly.  Can you reword not to use passive tense?

Also, how about saying what happens when the user exits the
recursive-edit?

> @@ -133,6 +136,10 @@ read-multiple-choice
>                    (ignore-errors (scroll-other-window)) t)
>                   ((eq answer 'scroll-other-window-down)
>                    (ignore-errors (scroll-other-window-down)) t)
> +                 ((eq answer 'edit)
> +                  (save-excursion
> +		    (save-window-excursion
> +		      (recursive-edit))))

It is customary to display an echo-area message when entering
recursive-edit, telling the user how to exit when done editing.

> -	    (setq wrong-char (not (memq tchar '(?? ?\C-h)))
> +	    (setq wrong-char (not (memq tchar '(?? help-char)))

What about F1?

> +              (if help-string
> +                  (with-help-window buf
> +                    (with-current-buffer buf
> +                      (insert help-string)))

Should this verify that HELP-STRING is a string?

> +---
> +** New help window when Emacs prompts before opening a large file.
> ++Press '?' or 'help-char' (by default, 'C-h') to display this new help window.

I think this should mention the situations where this happens.
"Opening large files" is not detailed enough: I suggest to mention a
couple of commands that trigger this, and perhaps also what is
considered a "large file".

>  (defun files--ask-user-about-large-file (size op-type filename offer-raw)
>    (let ((prompt (format "File %s is large (%s), really %s?"
>  		        (file-name-nondirectory filename)
>  		        (funcall byte-count-to-string-function size) op-type)))
>      (if (not offer-raw)
>          (if (y-or-n-p prompt) nil 'abort)
> -      (let* ((use-dialog (and (display-popup-menus-p)
> -                              last-input-event
> -	                      (listp last-nonmenu-event)
> -	                      use-dialog-box))
> +      (let* ((prompt (format "File %s is large (%s), really %s?"
> +		             (file-name-nondirectory filename)
> +		             (funcall byte-count-to-string-function size) op-type))
>               (choice
> -              (if use-dialog
> -                  (x-popup-dialog t `(,prompt
> -                                      ("Yes" . ?y)
> -                                      ("No" . ?n)
> -                                      ("Open literally" . ?l)))

Why are we losing the feature whereby we use GUI dialogs to ask the
question?  This sounds like a regression to me, at least for users who
use the mouse and menu-bar in preference to the keyboard.  And the doc
string still says we do support the dialogs.  Am I missing something?

Thanks.