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

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

[積丹尼 Dan Jacobson]

File 06-7377E-001A.jpg is large (14.4 MiB), really open? (y)es or (n)o or (l)iterally y
File 06-7377E-001B.jpg is large (14.4 MiB), really open? (y)es or (n)o or (l)iterally y
File 06-7377E-001C.jpg is large (14.4 MiB), really open? (y)es or (n)o or (l)iterally y
...
Maybe add choices:
"(a)lways" (for the rest of the session).
"(n)ever" (for the rest of the session).

Also the user has no clue about what to change to change whatever is
asking this. So maybe add (c)ustomize.

Seen with
v runs the command dired-view-file (found in dired-mode-map).

emacs-version "27.1"

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

[Unknown]

積丹尼 Dan Jacobson <jidanni@jidanni.org> writes:

Thank you for your feature request to improve Emacs.

> File 06-7377E-001A.jpg is large (14.4 MiB), really open? (y)es or (n)o or (l)iterally y
> File 06-7377E-001B.jpg is large (14.4 MiB), really open? (y)es or (n)o or (l)iterally y
> File 06-7377E-001C.jpg is large (14.4 MiB), really open? (y)es or (n)o or (l)iterally y
> ...
> Maybe add choices:
> "(a)lways" (for the rest of the session).

I think users would be confused about this option.  Would it always open
the file regularly or literally?  A minor thing, I think it's customary
in Emacs to use "!" for this "don't ask me again" feature (see
save-some-buffers or hack-local-variables-confirm, for example).

> "(n)ever" (for the rest of the session).

Same here, it's not totally clear to me if this option would never open
a large file, or never ask for confirmation before opening a large
file.  The dialog message ends with "really open?".

>
> Also the user has no clue about what to change to change whatever is
> asking this. So maybe add (c)ustomize.

Makes sense, but I don't know how well it'd fit the rest of Emacs.
AFAIK, customization points are not usually advertised in Emacs prompts.
It's more typical to expect that users "ask Emacs" to discover them, for
example by issuing M-x customize-apropos large file RET or M-x
customize-group files RET.

Instead, one possibility could be to offer a "!" option that would
always open big files non-literally in the current Emacs session.  Its
implementation would basically open the large file and set
large-file-warning-threshold to nil.  However, I foresee problems if
users accidentally press "!" in this prompt.

Another approach could be to offer a C-h binding that opens a help
window that describes the possible options and briefly links to the
customization entry point that removes the prompt permanently.

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

[積丹尼 Dan Jacobson]

>>>>> "DM" == Daniel Martín <mardani29@yahoo.es> writes:
DM> Another approach could be to offer a C-h binding

Indeed. Then all options of what the user could do could be explained in
detail there without any size limit.

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

[Lars Ingebrigtsen]

Daniel Martín <mardani29@yahoo.es> writes:

> I think users would be confused about this option.  Would it always open
> the file regularly or literally?

Yes, it's rife with confusion.

> Instead, one possibility could be to offer a "!" option that would
> always open big files non-literally in the current Emacs session.  Its
> implementation would basically open the large file and set
> large-file-warning-threshold to nil.  However, I foresee problems if
> users accidentally press "!" in this prompt.

Whether to open a large file (and how) is so situation-dependent that I
think adding a "!" option here would be a disservice.

> Another approach could be to offer a C-h binding that opens a help
> window that describes the possible options and briefly links to the
> customization entry point that removes the prompt permanently.

Yeah, that sounds like a good idea.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Juri Linkov]

>> Another approach could be to offer a C-h binding that opens a help
>> window that describes the possible options and briefly links to the
>> customization entry point that removes the prompt permanently.
>
> Yeah, that sounds like a good idea.

I agree, this is really the best idea.

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

[Daniel Martín via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 405 bytes --]

Juri Linkov <juri@linkov.net> writes:

>>> Another approach could be to offer a C-h binding that opens a help
>>> window that describes the possible options and briefly links to the
>>> customization entry point that removes the prompt permanently.
>>
>> Yeah, that sounds like a good idea.
>
> I agree, this is really the best idea.

OK, I've implemented it in the attached patch.  Suggestions
welcome.


[-- Attachment #2: 0001-Add-a-help-option-in-the-open-large-files-prompt.patch --]
[-- Type: text/x-patch, Size: 5246 bytes --]

From 8ae34d5bca1a8fc13f2388842880d4234ab28165 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Mart=C3=ADn?= <mardani29@yahoo.es>
Date: Sun, 11 Apr 2021 00:16:38 +0200
Subject: [PATCH] Add a help option in the open large files prompt

* lisp/files.el (files--ask-user-about-large-file-help): New function
that displays a Help buffer with information about opening large files
in Emacs.  (Bug#45412)
(files--ask-user-about-large-file): Add a ?/C-h option to the prompt.
* etc/NEWS: Advertise the change.
---
 etc/NEWS      |  6 ++++
 lisp/files.el | 76 +++++++++++++++++++++++++++++++++++++--------------
 2 files changed, 61 insertions(+), 21 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index a0f05d8cf1..b1bd9689e2 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -261,6 +261,12 @@ commands.  The new keystrokes are 'C-x x g' ('revert-buffer'),
 ** Commands 'set-frame-width' and 'set-frame-height' can now get their
 input using the minibuffer.
 
+---
+** When Emacs prompts before opening a large file, a new help window
+has been added.  This help window describes the available options and
+how to disable the prompt by customizing the
+'large-file-warning-threshold' variable.
+
 \f
 * Editing Changes in Emacs 28.1
 
diff --git a/lisp/files.el b/lisp/files.el
index 60d6034011..49224e4b1e 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -2123,28 +2123,62 @@ out-of-memory-warning-percentage
 
 (declare-function x-popup-dialog "menu.c" (position contents &optional header))
 
+(defun files--ask-user-about-large-file-help (op-type size)
+  "Show a buffer explaining the options to open large files in Emacs."
+  (with-output-to-temp-buffer "*Help*"
+    (with-current-buffer standard-output
+      (insert
+       (format
+        "The file that you want to %s is large (%s).
+
+Large files may be slow to edit or navigate so Emacs asks you
+before you try to open such files.
+
+You can press ‘y‘ to open the file.
+You can press ‘n‘ to abort.
+You can press ‘l‘ to open the file literally, which means that
+Emacs will open the file without doing any format or character code
+conversion and in Fundamental mode, without loading any potentially
+expensive feature.
+
+You can customize the option ‘large-file-warning-threshold‘ to be
+the file size, in bytes, from which Emacs will ask for
+confirmation.  Set it to ‘nil‘ to never request
+confirmation."
+        op-type
+        size))
+      (save-excursion
+	(re-search-backward "\\(customize\\)" nil t)
+	(help-xref-button 1 'help-customize-variable 'large-file-warning-threshold)))))
+
 (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))
-             (choice
-              (if use-dialog
-                  (x-popup-dialog t `(,prompt
-                                      ("Yes" . ?y)
-                                      ("No" . ?n)
-                                      ("Open literally" . ?l)))
-                (read-char-choice
-                 (concat prompt " (y)es or (n)o or (l)iterally ")
-                 '(?y ?Y ?n ?N ?l ?L)))))
-        (cond ((memq choice '(?y ?Y)) nil)
-              ((memq choice '(?l ?L)) 'raw)
-              (t 'abort))))))
+  (save-window-excursion
+    (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))
+               choice)
+          (if use-dialog
+              (setq choice (x-popup-dialog t `(,prompt
+                                               ("Yes" . ?y)
+                                               ("No" . ?n)
+                                               ("Open literally" . ?l))))
+            (while (null choice)
+              (setq choice (read-char-choice
+                            (concat prompt " (y)es, (n)o, (l)iterally, (?)")
+                            '(?y ?Y ?n ?N ?l ?L ?? ?\C-h)))
+              (when (memq choice '(?? ?\C-h))
+                (files--ask-user-about-large-file-help
+                 op-type (funcall byte-count-to-string-function size))
+                (setq choice nil))))
+          (cond ((memq choice '(?y ?Y)) nil)
+                ((memq choice '(?l ?L)) 'raw)
+                (t 'abort)))))))
 
 (defun abort-if-file-too-large (size op-type filename &optional offer-raw)
   "If file SIZE larger than `large-file-warning-threshold', allow user to abort.
-- 
2.31.0

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

[Juri Linkov]

>>>> Another approach could be to offer a C-h binding that opens a help
>>>> window that describes the possible options and briefly links to the
>>>> customization entry point that removes the prompt permanently.
>>>
>>> Yeah, that sounds like a good idea.
>>
>> I agree, this is really the best idea.
>
> OK, I've implemented it in the attached patch.  Suggestions
> welcome.
>
> +            (while (null choice)
> +              (setq choice (read-char-choice
> +                            (concat prompt " (y)es, (n)o, (l)iterally, (?)")
> +                            '(?y ?Y ?n ?N ?l ?L ?? ?\C-h)))
> +              (when (memq choice '(?? ?\C-h))
> +                (files--ask-user-about-large-file-help
> +                 op-type (funcall byte-count-to-string-function size))

To avoid the additional loop, maybe it's easier just to let-bind
the variable 'help-form' that is handled by 'read-char-choice'.

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

[Eli Zaretskii]

> Cc: 45412@debbugs.gnu.org, Lars Ingebrigtsen <larsi@gnus.org>,
>  積丹尼 Dan Jacobson <jidanni@jidanni.org>
> Date: Sun, 11 Apr 2021 00:32:34 +0200
> From:  Daniel Martín via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> >>> Another approach could be to offer a C-h binding that opens a help
> >>> window that describes the possible options and briefly links to the
> >>> customization entry point that removes the prompt permanently.
> >>
> >> Yeah, that sounds like a good idea.
> >
> > I agree, this is really the best idea.
> 
> OK, I've implemented it in the attached patch.  Suggestions
> welcome.

Thanks, see below for some comments.

> +---
> +** When Emacs prompts before opening a large file, a new help window
> +has been added.  This help window describes the available options and
> +how to disable the prompt by customizing the
> +'large-file-warning-threshold' variable.

This should mention the C-h key which triggers the help display:
IMO that's the most important piece of information users should know
about the new feature.

NEWS is visited in Outline mode, so each entry should have a heading
line which is a complete sentence with a concise description of the
change.

> +
>  \f
>  * Editing Changes in Emacs 28.1
>  
> diff --git a/lisp/files.el b/lisp/files.el
> index 60d6034011..49224e4b1e 100644
> --- a/lisp/files.el
> +++ b/lisp/files.el
> @@ -2123,28 +2123,62 @@ out-of-memory-warning-percentage
>  
>  (declare-function x-popup-dialog "menu.c" (position contents &optional header))
>  
> +(defun files--ask-user-about-large-file-help (op-type size)
> +  "Show a buffer explaining the options to open large files in Emacs."
> +  (with-output-to-temp-buffer "*Help*"
> +    (with-current-buffer standard-output
> +      (insert
> +       (format
> +        "The file that you want to %s is large (%s).

I suggest to say here what is the threshold above which Emacs asks the
question (because users may wish to customize it, as mentioned below).

> +You can press ‘y‘ to open the file.

  Press 'y' to open the file.

And please also use our style of quoting in documentation and strings,
'like this'.

> +You can press ‘n‘ to abort.

This should explain what "abort" means in this case.

> +You can press ‘l‘ to open the file literally, which means that

Suggest to say "(the letter ell)" here, to avoid confusion with the
digit 1.

Also, the text says literally "open", but the function actually
accepts an argument that describes the operation, which could be
something else.

> +Emacs will open the file without doing any format or character code
> +conversion and in Fundamental mode, without loading any potentially
> +expensive feature.
             ^^^^^^^
"features", in plural.

> +You can customize the option ‘large-file-warning-threshold‘ to be
                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Quoting style again.  Please quote `like this' here, so that Emacs
creates a hyperlink to the variable's doc.

> +the file size, in bytes, from which Emacs will ask for
> +confirmation.  Set it to ‘nil‘ to never request

We don't quote nil and t in this context.

> +          (if use-dialog
> +              (setq choice (x-popup-dialog t `(,prompt
> +                                               ("Yes" . ?y)
> +                                               ("No" . ?n)
> +                                               ("Open literally" . ?l))))

A literal "Open" again.

Also, don't we want to add a Help option to the dialog?  If not, why
not?

> +            (while (null choice)
> +              (setq choice (read-char-choice
> +                            (concat prompt " (y)es, (n)o, (l)iterally, (?)")
> +                            '(?y ?Y ?n ?N ?l ?L ?? ?\C-h)))
> +              (when (memq choice '(?? ?\C-h))

Why only ? or C-h?  What about F1? what about the case that help-char
is something else?

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

[Lars Ingebrigtsen]

Daniel Martín <mardani29@yahoo.es> writes:

> OK, I've implemented it in the attached patch.  Suggestions
> welcome.

Hm...  wouldn't just using `read-multiple-choice' be simpler here? 

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Daniel Martín via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Daniel Martín <mardani29@yahoo.es> writes:
>
>> OK, I've implemented it in the attached patch.  Suggestions
>> welcome.
>
> Hm...  wouldn't just using `read-multiple-choice' be simpler here? 

I've given `read-multiple-choice' a try and the help text doesn't look
very good if any of the descriptions is a bit long.  Another problem I
saw is that if the help text contains a reference to a variable, the
rendered hyperlink doesn't work.

It's a pity because I think this function is a good way to simplify and
homogenize this use case in Emacs.

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

[Lars Ingebrigtsen]

Daniel Martín <mardani29@yahoo.es> writes:

> I've given `read-multiple-choice' a try and the help text doesn't look
> very good if any of the descriptions is a bit long.  Another problem I
> saw is that if the help text contains a reference to a variable, the
> rendered hyperlink doesn't work.
>
> It's a pity because I think this function is a good way to simplify and
> homogenize this use case in Emacs.

Well, neither problem seem insurmountable to fix.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Daniel Martín via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 1420 bytes --]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Daniel Martín <mardani29@yahoo.es> writes:
>
>> I've given `read-multiple-choice' a try and the help text doesn't look
>> very good if any of the descriptions is a bit long.  Another problem I
>> saw is that if the help text contains a reference to a variable, the
>> rendered hyperlink doesn't work.
>>
>> It's a pity because I think this function is a good way to simplify and
>> homogenize this use case in Emacs.
>
> Well, neither problem seem insurmountable to fix.

OK, I've refactored the feature on top of a patched version of
`read-multiple-choice' that adds support for overriding the help text.
I've also handled the `edit' action, so that a user can enter a
recursive edit to select the help link.  This API change is
backwards-compatible.  As one change depends on the previous one, but
they are otherwise orthogonal, I've made two separate commits.

These new patches address the following review comments:

- Simplify the call site by avoiding the character loop.

- The help description now appears on `help-char', ?, or F1.

- Improves and fixes the help text and NEWS description, per Eli's
  suggestions.

The idea to show the help text also for graphical prompts is a good
idea, but as there's no command already in Emacs that supports it,
AFAIK, it could be a separate feature request.  What do you think?

Thanks.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Extend-read-multiple-choice-to-support-free-form-hel.patch --]
[-- Type: text/x-patch, Size: 8316 bytes --]

From d6c124930a7b54f222d3cef79e11b66eb4a188f5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Mart=C3=ADn?= <mardani29@yahoo.es>
Date: Wed, 21 Apr 2021 23:14:27 +0200
Subject: [PATCH 1/2] Extend read-multiple-choice to support free-form help
 descriptions

* lisp/emacs-lisp/rmc.el (read-multiple-choice): Add a new argument to
override the default help description in `read-multiple-choice'.  Use
the `help-char' variable instead of ?\C-h.  Also support the `edit'
action from `query-replace-map', so that help links can be visited by
entering a recursive edit.
---
 lisp/emacs-lisp/rmc.el | 119 ++++++++++++++++++++++-------------------
 1 file changed, 65 insertions(+), 54 deletions(-)

diff --git a/lisp/emacs-lisp/rmc.el b/lisp/emacs-lisp/rmc.el
index bedf598d44..2c4c13ae1c 100644
--- a/lisp/emacs-lisp/rmc.el
+++ b/lisp/emacs-lisp/rmc.el
@@ -26,7 +26,7 @@
 (require 'seq)
 
 ;;;###autoload
-(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 function translates user input into responses by consulting
 the bindings in `query-replace-map'; see the documentation of
 that variable for more information.  In this case, the useful
-bindings are `recenter', `scroll-up', and `scroll-down'.  If the
-user enters `recenter', `scroll-up', or `scroll-down' responses,
-perform the requested window recentering or scrolling and ask
-again.
+bindings are `recenter', `scroll-up', `scroll-down', and `edit'.
+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.
 
 When `use-dialog-box' is t (the default), this function can pop
 up a dialog window to collect the user input.  That functionality
@@ -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))))
                  (t tchar)))
           (when (eq tchar t)
             (setq wrong-char nil
@@ -141,57 +148,61 @@ read-multiple-choice
           ;; help messages.
           (when (and (not (eq tchar nil))
                      (not (assq tchar choices)))
-	    (setq wrong-char (not (memq tchar '(?? ?\C-h)))
+	    (setq wrong-char (not (memq tchar '(?? help-char)))
                   tchar nil)
             (when wrong-char
               (ding))
-            (with-help-window (setq buf (get-buffer-create
-                                         "*Multiple Choice Help*"))
-              (with-current-buffer buf
-                (erase-buffer)
-                (pop-to-buffer buf)
-                (insert prompt "\n\n")
-                (let* ((columns (/ (window-width) 25))
-                       (fill-column 21)
-                       (times 0)
-                       (start (point)))
-                  (dolist (elem choices)
-                    (goto-char start)
-                    (unless (zerop times)
-                      (if (zerop (mod times columns))
-                          ;; Go to the next "line".
-                          (goto-char (setq start (point-max)))
-                        ;; Add padding.
-                        (while (not (eobp))
-                          (end-of-line)
-                          (insert (make-string (max (- (* (mod times columns)
-                                                          (+ fill-column 4))
-                                                       (current-column))
-                                                    0)
-                                               ?\s))
-                          (forward-line 1))))
-                    (setq times (1+ times))
-                    (let ((text
-                           (with-temp-buffer
-                             (insert (format
-                                      "%c: %s\n"
-                                      (car elem)
-                                      (cdr (assq (car elem) altered-names))))
-                             (fill-region (point-min) (point-max))
-                             (when (nth 2 elem)
-                               (let ((start (point)))
-                                 (insert (nth 2 elem))
-                                 (unless (bolp)
-                                   (insert "\n"))
-                                 (fill-region start (point-max))))
-                             (buffer-string))))
-                      (goto-char start)
-                      (dolist (line (split-string text "\n"))
-                        (end-of-line)
-                        (if (bolp)
-                            (insert line "\n")
-                          (insert line))
-                        (forward-line 1)))))))))))
+            (let ((buf (get-buffer-create "*Multiple Choice Help*")))
+              (if help-string
+                  (with-help-window buf
+                    (with-current-buffer buf
+                      (insert help-string)))
+                (with-help-window buf
+                  (with-current-buffer buf
+                    (erase-buffer)
+                    (pop-to-buffer buf)
+                    (insert prompt "\n\n")
+                    (let* ((columns (/ (window-width) 25))
+                           (fill-column 21)
+                           (times 0)
+                           (start (point)))
+                      (dolist (elem choices)
+                        (goto-char start)
+                        (unless (zerop times)
+                          (if (zerop (mod times columns))
+                              ;; Go to the next "line".
+                              (goto-char (setq start (point-max)))
+                            ;; Add padding.
+                            (while (not (eobp))
+                              (end-of-line)
+                              (insert (make-string (max (- (* (mod times columns)
+                                                              (+ fill-column 4))
+                                                           (current-column))
+                                                        0)
+                                                   ?\s))
+                              (forward-line 1))))
+                        (setq times (1+ times))
+                        (let ((text
+                               (with-temp-buffer
+                                 (insert (format
+                                          "%c: %s\n"
+                                          (car elem)
+                                          (cdr (assq (car elem) altered-names))))
+                                 (fill-region (point-min) (point-max))
+                                 (when (nth 2 elem)
+                                   (let ((start (point)))
+                                     (insert (nth 2 elem))
+                                     (unless (bolp)
+                                       (insert "\n"))
+                                     (fill-region start (point-max))))
+                                 (buffer-string))))
+                          (goto-char start)
+                          (dolist (line (split-string text "\n"))
+                            (end-of-line)
+                            (if (bolp)
+                                (insert line "\n")
+                              (insert line))
+                            (forward-line 1)))))))))))))
     (when (buffer-live-p buf)
       (kill-buffer buf))
     (assq tchar choices)))
-- 
2.31.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Add-a-help-option-to-the-open-large-files-prompt.patch --]
[-- Type: text/x-patch, Size: 4367 bytes --]

From 4f3e308b372cfea728211421679ad00d5a8b64df Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Mart=C3=ADn?= <mardani29@yahoo.es>
Date: Wed, 21 Apr 2021 23:27:13 +0200
Subject: [PATCH 2/2] Add a help option to the open large files prompt

* lisp/files.el (files--ask-user-about-large-file-help-text): New
function that returns information about opening large files in
Emacs.  (Bug#45412)
(files--ask-user-about-large-file): Use read-multiple-choice to
display the available actions.
* etc/NEWS: Advertise the new feature.
---
 etc/NEWS      |  4 ++++
 lisp/files.el | 57 ++++++++++++++++++++++++++++++++++++++-------------
 2 files changed, 47 insertions(+), 14 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index b641e8d95f..7905c936a3 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -272,6 +272,10 @@ commands.  The new keystrokes are 'C-x x g' ('revert-buffer'),
 ** Commands 'set-frame-width' and 'set-frame-height' can now get their
 input using the minibuffer.
 
+---
+** 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.
+
 \f
 * Editing Changes in Emacs 28.1
 
diff --git a/lisp/files.el b/lisp/files.el
index 7440c11a21..fe3722b07d 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -2123,27 +2123,56 @@ out-of-memory-warning-percentage
 
 (declare-function x-popup-dialog "menu.c" (position contents &optional header))
 
+(defun files--ask-user-about-large-file-help-text (op-type size)
+  "Format the text that explains the options to open large files in Emacs.
+OP-TYPE contains the kind of file operation that will be
+performed.  SIZE is the size of the large file."
+  (format
+   "The file that you want to %s is large (%s), which exceeds the
+ threshold above which Emacs asks for confirmation (%s).
+
+ Large files may be slow to edit or navigate so Emacs asks you
+ before you try to %s such files.
+
+ You can press:
+ 'y' to %s the file.
+ 'n' to abort, and not %s the file.
+ 'l' (the letter ell) to %s the file literally, which means that
+ Emacs will %s the file without doing any format or character code
+ conversion and in Fundamental mode, without loading any potentially
+ expensive features.
+
+ You can customize the option `large-file-warning-threshold' to be the
+ file size, in bytes, from which Emacs will ask for confirmation.  Set
+ it to nil to never request confirmation."
+   op-type
+   size
+   (funcall byte-count-to-string-function large-file-warning-threshold)
+   op-type
+   op-type
+   op-type
+   op-type
+   op-type))
+
 (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)))
-                (read-char-choice
-                 (concat prompt " (y)es or (n)o or (l)iterally ")
-                 '(?y ?Y ?n ?N ?l ?L)))))
-        (cond ((memq choice '(?y ?Y)) nil)
-              ((memq choice '(?l ?L)) 'raw)
+              (car
+               (read-multiple-choice prompt '((?y "yes")
+                                              (?n "no")
+                                              (?l "literally"))
+                                     (files--ask-user-about-large-file-help-text
+                                      op-type
+                                      (funcall byte-count-to-string-function size))))))
+        (cond ((eq choice ?y) nil)
+              ((eq choice ?l) 'raw)
               (t 'abort))))))
 
 (defun abort-if-file-too-large (size op-type filename &optional offer-raw)
-- 
2.31.0

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

[Eli Zaretskii]

> 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.

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

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> -              (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?

The support for x-popup-dialog is in `read-multiple-choice', so I assume
that it's just using that instead?  But I haven't tried the patch.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Daniel Martín via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> -              (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?
>
> The support for x-popup-dialog is in `read-multiple-choice', so I assume
> that it's just using that instead?  But I haven't tried the patch.

Yes, if you click on a large file in Dired, you'll see a graphical
prompt, as always.

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

[Eli Zaretskii]

> From: Daniel Martín <mardani29@yahoo.es>
> Cc: Eli Zaretskii <eliz@gnu.org>,  45412@debbugs.gnu.org,
>   jidanni@jidanni.org,  juri@linkov.net
> Date: Sun, 25 Apr 2021 23:23:02 +0200
> 
> Lars Ingebrigtsen <larsi@gnus.org> writes:
> 
> > Eli Zaretskii <eliz@gnu.org> writes:
> >
> >>> -              (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?
> >
> > The support for x-popup-dialog is in `read-multiple-choice', so I assume
> > that it's just using that instead?  But I haven't tried the patch.
> 
> Yes, if you click on a large file in Dired, you'll see a graphical
> prompt, as always.

With which buttons and which text?

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

[Lars Ingebrigtsen]

Daniel Martín <mardani29@yahoo.es> writes:

> Yes, if you click on a large file in Dired, you'll see a graphical
> prompt, as always.

Right.  Eli had other comments on the patch, though -- would it be
possible for you to rework the patch, and then we can get it applied?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Daniel Martín via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 419 bytes --]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Daniel Martín <mardani29@yahoo.es> writes:
>
>> Yes, if you click on a large file in Dired, you'll see a graphical
>> prompt, as always.
>
> Right.  Eli had other comments on the patch, though -- would it be
> possible for you to rework the patch, and then we can get it applied?

I think I've applied all code review suggestions (patches attached).
Thanks.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Extend-read-multiple-choice-to-support-free-form-hel.patch --]
[-- Type: text/x-patch, Size: 8895 bytes --]

From b5a49005af33d8fcd35ca034949920f3ff8c36d6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Mart=C3=ADn?= <mardani29@yahoo.es>
Date: Wed, 21 Apr 2021 23:14:27 +0200
Subject: [PATCH 1/2] Extend read-multiple-choice to support free-form help
 descriptions

* lisp/emacs-lisp/rmc.el (read-multiple-choice): Add a new argument to
override the default help description in `read-multiple-choice'.  Use
the `help-char' variable instead of ?\C-h.  Also support the `edit'
action from `query-replace-map', so that help links can be visited by
entering a recursive edit.
---
 lisp/emacs-lisp/rmc.el | 129 +++++++++++++++++++++++------------------
 1 file changed, 74 insertions(+), 55 deletions(-)

diff --git a/lisp/emacs-lisp/rmc.el b/lisp/emacs-lisp/rmc.el
index bedf598d44..6aa169c032 100644
--- a/lisp/emacs-lisp/rmc.el
+++ b/lisp/emacs-lisp/rmc.el
@@ -26,24 +26,32 @@
 (require 'seq)
 
 ;;;###autoload
-(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.
 
 CHOICES is a list of (KEY NAME [DESCRIPTION]).  KEY is a
 character to be entered.  NAME is a short name for the entry to
 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.
+shortened).  DESCRIPTION is an optional longer explanation for
+the entry that will be displayed in a help buffer if the user
+requests more help.  This help description has a fixed format in
+columns, but, for greater flexibility, instead of passing a
+DESCRIPTION, the user can use the optional argument HELP-STRING.
+This argument is a string that contains the text with the
+complete description of all choices.  `read-multiple-choice' will
+display that description in a help buffer if the user requests
+it.
 
 This function translates user input into responses by consulting
 the bindings in `query-replace-map'; see the documentation of
 that variable for more information.  In this case, the useful
-bindings are `recenter', `scroll-up', and `scroll-down'.  If the
-user enters `recenter', `scroll-up', or `scroll-down' responses,
-perform the requested window recentering or scrolling and ask
-again.
+bindings are `recenter', `scroll-up', `scroll-down', and `edit'.
+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', start a recursive
+edit.  When the user exit the recursive edit, the multiple choice
+prompt gains focus again.
 
 When `use-dialog-box' is t (the default), this function can pop
 up a dialog window to collect the user input.  That functionality
@@ -133,6 +141,13 @@ 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-match-data
+                    (save-excursion
+                      (message "%s"
+                               (substitute-command-keys
+                                "Recursive edit.  Resume with \\[exit-recursive-edit]"))
+                      (recursive-edit))))
                  (t tchar)))
           (when (eq tchar t)
             (setq wrong-char nil
@@ -141,57 +156,61 @@ read-multiple-choice
           ;; help messages.
           (when (and (not (eq tchar nil))
                      (not (assq tchar choices)))
-	    (setq wrong-char (not (memq tchar '(?? ?\C-h)))
+	    (setq wrong-char (not (memq tchar `(?? ,help-char)))
                   tchar nil)
             (when wrong-char
               (ding))
-            (with-help-window (setq buf (get-buffer-create
-                                         "*Multiple Choice Help*"))
-              (with-current-buffer buf
-                (erase-buffer)
-                (pop-to-buffer buf)
-                (insert prompt "\n\n")
-                (let* ((columns (/ (window-width) 25))
-                       (fill-column 21)
-                       (times 0)
-                       (start (point)))
-                  (dolist (elem choices)
-                    (goto-char start)
-                    (unless (zerop times)
-                      (if (zerop (mod times columns))
-                          ;; Go to the next "line".
-                          (goto-char (setq start (point-max)))
-                        ;; Add padding.
-                        (while (not (eobp))
-                          (end-of-line)
-                          (insert (make-string (max (- (* (mod times columns)
-                                                          (+ fill-column 4))
-                                                       (current-column))
-                                                    0)
-                                               ?\s))
-                          (forward-line 1))))
-                    (setq times (1+ times))
-                    (let ((text
-                           (with-temp-buffer
-                             (insert (format
-                                      "%c: %s\n"
-                                      (car elem)
-                                      (cdr (assq (car elem) altered-names))))
-                             (fill-region (point-min) (point-max))
-                             (when (nth 2 elem)
-                               (let ((start (point)))
-                                 (insert (nth 2 elem))
-                                 (unless (bolp)
-                                   (insert "\n"))
-                                 (fill-region start (point-max))))
-                             (buffer-string))))
+            (setq buf (get-buffer-create "*Multiple Choice Help*"))
+            (if (stringp help-string)
+                (with-help-window buf
+                  (with-current-buffer buf
+                    (insert help-string)))
+              (with-help-window buf
+                (with-current-buffer buf
+                  (erase-buffer)
+                  (pop-to-buffer buf)
+                  (insert prompt "\n\n")
+                  (let* ((columns (/ (window-width) 25))
+                         (fill-column 21)
+                         (times 0)
+                         (start (point)))
+                    (dolist (elem choices)
                       (goto-char start)
-                      (dolist (line (split-string text "\n"))
-                        (end-of-line)
-                        (if (bolp)
-                            (insert line "\n")
-                          (insert line))
-                        (forward-line 1)))))))))))
+                      (unless (zerop times)
+                        (if (zerop (mod times columns))
+                            ;; Go to the next "line".
+                            (goto-char (setq start (point-max)))
+                          ;; Add padding.
+                          (while (not (eobp))
+                            (end-of-line)
+                            (insert (make-string (max (- (* (mod times columns)
+                                                            (+ fill-column 4))
+                                                         (current-column))
+                                                      0)
+                                                 ?\s))
+                            (forward-line 1))))
+                      (setq times (1+ times))
+                      (let ((text
+                             (with-temp-buffer
+                               (insert (format
+                                        "%c: %s\n"
+                                        (car elem)
+                                        (cdr (assq (car elem) altered-names))))
+                               (fill-region (point-min) (point-max))
+                               (when (nth 2 elem)
+                                 (let ((start (point)))
+                                   (insert (nth 2 elem))
+                                   (unless (bolp)
+                                     (insert "\n"))
+                                   (fill-region start (point-max))))
+                               (buffer-string))))
+                        (goto-char start)
+                        (dolist (line (split-string text "\n"))
+                          (end-of-line)
+                          (if (bolp)
+                              (insert line "\n")
+                            (insert line))
+                          (forward-line 1))))))))))))
     (when (buffer-live-p buf)
       (kill-buffer buf))
     (assq tchar choices)))
-- 
2.31.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Add-a-help-option-to-the-open-large-files-prompt.patch --]
[-- Type: text/x-patch, Size: 4693 bytes --]

From 406990011fe26ee681597e25155ab4dfa4f6ee42 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Mart=C3=ADn?= <mardani29@yahoo.es>
Date: Wed, 21 Apr 2021 23:27:13 +0200
Subject: [PATCH 2/2] Add a help option to the open large files prompt

* lisp/files.el (files--ask-user-about-large-file-help-text): New
function that returns information about opening large files in
Emacs.  (Bug#45412)
(files--ask-user-about-large-file): Use read-multiple-choice to
display the available actions.
* etc/NEWS: Advertise the new feature.
---
 etc/NEWS      |  8 +++++++
 lisp/files.el | 60 ++++++++++++++++++++++++++++++++++++++-------------
 2 files changed, 53 insertions(+), 15 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index c2db98ae45..f99dcae536 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -276,6 +276,14 @@ commands.  The new keystrokes are 'C-x x g' ('revert-buffer'),
 ** Commands 'set-frame-width' and 'set-frame-height' can now get their
 input using the minibuffer.
 
+---
+** New help window when Emacs prompts before opening a large file.
+Commands like 'find-file' or 'visit-tags-table' ask to visit a file
+normally or literally when the file is larger than a certain size (by
+default, 9.5 MiB).  Press '?' or 'C-h' in that prompt to read more
+about the different options to visit a file, how you can disable the
+prompt, and how you can tweak the file size threshold.
+
 \f
 * Editing Changes in Emacs 28.1
 
diff --git a/lisp/files.el b/lisp/files.el
index 8e8fbac8dc..c1857ba032 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -2129,27 +2129,57 @@ out-of-memory-warning-percentage
 
 (declare-function x-popup-dialog "menu.c" (position contents &optional header))
 
+(defun files--ask-user-about-large-file-help-text (op-type size)
+  "Format the text that explains the options to open large files in Emacs.
+OP-TYPE contains the kind of file operation that will be
+performed.  SIZE is the size of the large file."
+  (format
+   "The file that you want to %s is large (%s), which exceeds the
+ threshold above which Emacs asks for confirmation (%s).
+
+ Large files may be slow to edit or navigate so Emacs asks you
+ before you try to %s such files.
+
+ You can press:
+ 'y' to %s the file.
+ 'n' to abort, and not %s the file.
+ 'l' (the letter ell) to %s the file literally, which means that
+ Emacs will %s the file without doing any format or character code
+ conversion and in Fundamental mode, without loading any potentially
+ expensive features.
+
+ You can customize the option `large-file-warning-threshold' to be the
+ file size, in bytes, from which Emacs will ask for confirmation.  Set
+ it to nil to never request confirmation."
+   op-type
+   size
+   (funcall byte-count-to-string-function large-file-warning-threshold)
+   op-type
+   op-type
+   op-type
+   op-type
+   op-type))
+
 (defun files--ask-user-about-large-file (size op-type filename offer-raw)
+  "If file SIZE larger than `large-file-warning-threshold', allow user to abort.
+OP-TYPE specifies the file operation being performed on FILENAME.
+If OFFER-RAW is true, give user the additional option to open the
+file literally."
   (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))
-             (choice
-              (if use-dialog
-                  (x-popup-dialog t `(,prompt
-                                      ("Yes" . ?y)
-                                      ("No" . ?n)
-                                      ("Open literally" . ?l)))
-                (read-char-choice
-                 (concat prompt " (y)es or (n)o or (l)iterally ")
-                 '(?y ?Y ?n ?N ?l ?L)))))
-        (cond ((memq choice '(?y ?Y)) nil)
-              ((memq choice '(?l ?L)) 'raw)
+      (let* ((choice
+              (car
+               (read-multiple-choice prompt '((?y "yes")
+                                              (?n "no")
+                                              (?l "literally"))
+                                     (files--ask-user-about-large-file-help-text
+                                      op-type
+                                      (funcall byte-count-to-string-function size))))))
+        (cond ((eq choice ?y) nil)
+              ((eq choice ?l) 'raw)
               (t 'abort))))))
 
 (defun abort-if-file-too-large (size op-type filename &optional offer-raw)
-- 
2.31.0

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

[Lars Ingebrigtsen]

Daniel Martín <mardani29@yahoo.es> writes:

> I think I've applied all code review suggestions (patches attached).
> Thanks.

Thanks; applied to Emacs 28 (with some minor changes).  Seems to work
great.  :-)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Eli Zaretskii]

> From: Daniel Martín <mardani29@yahoo.es>
> Cc: Eli Zaretskii <eliz@gnu.org>,  45412@debbugs.gnu.org,
>   jidanni@jidanni.org,  juri@linkov.net
> Date: Thu, 06 May 2021 01:10:08 +0200
> 
> I think I've applied all code review suggestions (patches attached).

Thanks, there are still a couple of nits left:

> * lisp/emacs-lisp/rmc.el (read-multiple-choice): Add a new argument to
> override the default help description in `read-multiple-choice'.  Use
> the `help-char' variable instead of ?\C-h.  Also support the `edit'
> action from `query-replace-map', so that help links can be visited by
> entering a recursive edit.

In commit log messages and in NEWS we nowadays prefer quoting 'like
this', not `like this'.

> +shortened).  DESCRIPTION is an optional longer explanation for
> +the entry that will be displayed in a help buffer if the user
> +requests more help.  This help description has a fixed format in
> +columns, but, for greater flexibility, instead of passing a
> +DESCRIPTION, the user can use the optional argument HELP-STRING.
> +This argument is a string that contains the text with the
> +complete description of all choices.  `read-multiple-choice' will
> +display that description in a help buffer if the user requests
> +it.

You say "user" here several times, but I think you mean the Lisp
program that calls this function, not the Emacs user.

> +                               (substitute-command-keys
> +                                "Recursive edit.  Resume with \\[exit-recursive-edit]"))

I think "resume" is too vague here to be useful.  Suggest to reword:

  Recursive edit; type \\[exit-recursive-edit] to return to help screen.

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

[Eli Zaretskii]

> Date: Thu, 06 May 2021 11:30:57 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 45412@debbugs.gnu.org, larsi@gnus.org, jidanni@jidanni.org, juri@linkov.net
> 
> > From: Daniel Martín <mardani29@yahoo.es>
> > Cc: Eli Zaretskii <eliz@gnu.org>,  45412@debbugs.gnu.org,
> >   jidanni@jidanni.org,  juri@linkov.net
> > Date: Thu, 06 May 2021 01:10:08 +0200
> > 
> > I think I've applied all code review suggestions (patches attached).
> 
> Thanks, there are still a couple of nits left:

Looks like these comments were ignored, so I went ahead and made the
changes myself.  (Except that there's no way of fixing the quoting in
the commit log message...)

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

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> Looks like these comments were ignored, so I went ahead and made the
> changes myself.

Thanks.

> (Except that there's no way of fixing the quoting in the commit log
> message...)

I don't think we have any policy on quoting symbols in the commit
messages.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: mardani29@yahoo.es,  45412@debbugs.gnu.org,  jidanni@jidanni.org,
>   juri@linkov.net
> Date: Sat, 08 May 2021 13:37:32 +0200
> 
> I don't think we have any policy on quoting symbols in the commit
> messages.

We decided to quote 'like this' in NEWS and ChangeLog files, AFAIR.
Since ChangeLog files are nowadays produced from the Git log, I
thought this was in effect there as well.

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

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> We decided to quote 'like this' in NEWS and ChangeLog files, AFAIR.
> Since ChangeLog files are nowadays produced from the Git log, I
> thought this was in effect there as well.

It makes sense to insist on a convention in the NEWS file, but in the
commit messages (and therefore ChangeLog files), that requires more
effort than it's worth.  (And very few people seem to be following the
same conventions currently.)

There's too many hurdles to contribute to Emacs already -- let's not add
more.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: mardani29@yahoo.es,  45412@debbugs.gnu.org,  jidanni@jidanni.org,
>   juri@linkov.net
> Date: Sun, 09 May 2021 11:57:53 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > We decided to quote 'like this' in NEWS and ChangeLog files, AFAIR.
> > Since ChangeLog files are nowadays produced from the Git log, I
> > thought this was in effect there as well.
> 
> It makes sense to insist on a convention in the NEWS file, but in the
> commit messages (and therefore ChangeLog files), that requires more
> effort than it's worth.  (And very few people seem to be following the
> same conventions currently.)
> 
> There's too many hurdles to contribute to Emacs already -- let's not add
> more.

I think having 2 different conventions is worse than having just one,
but I won't argue further.

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

[Juri Linkov]

> The idea to show the help text also for graphical prompts is a good
> idea, but as there's no command already in Emacs that supports it,
> AFAIK, it could be a separate feature request.  What do you think?

Thanks.  While testing your patches, I noticed a compilation warning:

  emacs-lisp/rmc.el:29:1: Warning: Variable `buf' left uninitialized

Also these let-bindings are duplicated:

  (prompt (format "File %s is large (%s), really %s?"
                  (file-name-nondirectory filename)
                  (funcall byte-count-to-string-function size) op-type))

Otherwise, it looks nice, except of Eli's comment about the need
to keep graphical dialogs.