bug#30073: 27.0.50; dired-do-delete ignores customization for short answers

classic Classic list List threaded Threaded
29 messages Options
12
Reply | Threaded
Open this post in threaded view
|

bug#30073: 27.0.50; dired-do-delete ignores customization for short answers

Juri Linkov-2
I customized ‘yes-or-no-p’ in ~/.emacs to short answers:

  (fset 'yes-or-no-p 'y-or-n-p)

but now ‘dired-do-delete’ ignores this customization, and always requires
typing a full answer “yes” with RET when deleting recursively all subdirs,
instead of accepting just a single letter ‘y’ as before the recent change.

I think in this case better is to accept ‘y’ for “yes”, and ‘!’ for “all”
like in ‘query-replace-map’.



Reply | Threaded
Open this post in threaded view
|

bug#30073: 27.0.50; dired-do-delete ignores customization for short answers

Basil L. Contovounesios
Juri Linkov <[hidden email]> writes:

> I customized ‘yes-or-no-p’ in ~/.emacs to short answers:
>
>   (fset 'yes-or-no-p 'y-or-n-p)
>
> but now ‘dired-do-delete’ ignores this customization, and always requires
> typing a full answer “yes” with RET when deleting recursively all subdirs,
> instead of accepting just a single letter ‘y’ as before the recent change.
>
> I think in this case better is to accept ‘y’ for “yes”, and ‘!’ for “all”
> like in ‘query-replace-map’.

Just linking bug#27940, where the move away from using yes-or-no-p in
function dired-delete-file was discussed:

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=27940

--
Basil



Reply | Threaded
Open this post in threaded view
|

bug#30073: 27.0.50; dired-do-delete ignores customization for short answers

Eli Zaretskii
> From: "Basil L. Contovounesios" <[hidden email]>
> Date: Thu, 11 Jan 2018 03:00:38 +0000
> Cc: [hidden email]
>
> Juri Linkov <[hidden email]> writes:
>
> > I customized ‘yes-or-no-p’ in ~/.emacs to short answers:
> >
> >   (fset 'yes-or-no-p 'y-or-n-p)
> >
> > but now ‘dired-do-delete’ ignores this customization, and always requires
> > typing a full answer “yes” with RET when deleting recursively all subdirs,
> > instead of accepting just a single letter ‘y’ as before the recent change.
> >
> > I think in this case better is to accept ‘y’ for “yes”, and ‘!’ for “all”
> > like in ‘query-replace-map’.
>
> Just linking bug#27940, where the move away from using yes-or-no-p in
> function dired-delete-file was discussed:
>
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=27940

Indeed, this horse has been beaten to death already.  The current
solution is for you to set dired-deletion-confirmer to y-or-n-p.
Admittedly, that's not a very user-friendly customization method, but
then neither is fset.



Reply | Threaded
Open this post in threaded view
|

bug#30073: 27.0.50; dired-do-delete ignores customization for short answers

Basil L. Contovounesios

Eli Zaretskii <[hidden email]> writes:

>> Juri Linkov <[hidden email]> writes:
>>
>> > I customized ‘yes-or-no-p’ in ~/.emacs to short answers:
>> >
>> >   (fset 'yes-or-no-p 'y-or-n-p)
>> >
>> > [...]
>
> [...]
>
> Indeed, this horse has been beaten to death already.  The current
> solution is for you to set dired-deletion-confirmer to y-or-n-p.
> Admittedly, that's not a very user-friendly customization method, but
> then neither is fset.

FWIW, I find

  (advice-add 'yes-or-no-p :override #'y-or-n-p)

to be a slightly user-friendlier alternative to

  (fset 'yes-or-no-p 'y-or-n-p)

as the former is at least reported by the help system.

--
Basil



Reply | Threaded
Open this post in threaded view
|

bug#30073: 27.0.50; dired-do-delete ignores customization for short answers

Eli Zaretskii
> From: "Basil L. Contovounesios" <[hidden email]>
> Cc: Eli Zaretskii <[hidden email]>, <[hidden email]>
> Date: Thu, 11 Jan 2018 17:34:21 +0000
>
> > Indeed, this horse has been beaten to death already.  The current
> > solution is for you to set dired-deletion-confirmer to y-or-n-p.
> > Admittedly, that's not a very user-friendly customization method, but
> > then neither is fset.
>
> FWIW, I find
>
>   (advice-add 'yes-or-no-p :override #'y-or-n-p)
>
> to be a slightly user-friendlier alternative to
>
>   (fset 'yes-or-no-p 'y-or-n-p)
>
> as the former is at least reported by the help system.

IMO, none of that is user-friendly as customization goes.  These are
semi-kludgey hacks, which are okay for personal use, but not as
advertised customization means.  YMMV, of course.



Reply | Threaded
Open this post in threaded view
|

bug#30073: 27.0.50; dired-do-delete ignores customization for short answers

Basil L. Contovounesios
Eli Zaretskii <[hidden email]> writes:

>> FWIW, I find
>>
>>   (advice-add 'yes-or-no-p :override #'y-or-n-p)
>>
>> to be a slightly user-friendlier alternative to
>>
>>   (fset 'yes-or-no-p 'y-or-n-p)
>>
>> as the former is at least reported by the help system.
>
> IMO, none of that is user-friendly as customization goes.  These are
> semi-kludgey hacks, which are okay for personal use, but not as
> advertised customization means.

Oh, absolutely; I meant that only as a side note.  Besides, one could
argue that the means to achieving the equivalent of this hack shouldn't
be any user-friendlier.

--
Basil



Reply | Threaded
Open this post in threaded view
|

bug#30073: 27.0.50; dired-do-delete ignores customization for short answers

Juri Linkov-2
In reply to this post by Eli Zaretskii
>> Just linking bug#27940, where the move away from using yes-or-no-p in
>> function dired-delete-file was discussed:
>>
>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=27940
>
> Indeed, this horse has been beaten to death already.  The current
> solution is for you to set dired-deletion-confirmer to y-or-n-p.
> Admittedly, that's not a very user-friendly customization method, but
> then neither is fset.

What is worse, it doesn't work at all - setting dired-deletion-confirmer
to y-or-n-p has no effect on the question “Recursively delete ...? ”.
It still expects “[yes, no, all, quit, help]” answers, not short ones
like “y/n/!/q/?”.



Reply | Threaded
Open this post in threaded view
|

bug#30073: 27.0.50; dired-do-delete ignores customization for short answers

Juri Linkov-2
In reply to this post by Eli Zaretskii
>> FWIW, I find
>>
>>   (advice-add 'yes-or-no-p :override #'y-or-n-p)
>>
>> to be a slightly user-friendlier alternative to
>>
>>   (fset 'yes-or-no-p 'y-or-n-p)
>>
>> as the former is at least reported by the help system.
>
> IMO, none of that is user-friendly as customization goes.  These are
> semi-kludgey hacks, which are okay for personal use, but not as
> advertised customization means.  YMMV, of course.

Maybe we should introduce a boolean customizable variable with a name
‘minibuffer-short-answers’ to be used by different packages that
currently rely on ‘yes-or-no-p’.



Reply | Threaded
Open this post in threaded view
|

bug#30073: 27.0.50; dired-do-delete ignores customization for short answers

Eli Zaretskii
In reply to this post by Juri Linkov-2
> From: Juri Linkov <[hidden email]>
> Cc: "Basil L. Contovounesios" <[hidden email]>,  [hidden email]
> Date: Thu, 11 Jan 2018 23:54:41 +0200
>
> What is worse, it doesn't work at all - setting dired-deletion-confirmer
> to y-or-n-p has no effect on the question “Recursively delete ...? ”.
> It still expects “[yes, no, all, quit, help]” answers, not short ones
> like “y/n/!/q/?”.

If you want to use y-or-n-p there, you will have to replace
dired--yes-no-all-quit-help with your own function.



Reply | Threaded
Open this post in threaded view
|

bug#30073: 27.0.50; dired-do-delete ignores customization for short answers

Juri Linkov-2
>> What is worse, it doesn't work at all - setting dired-deletion-confirmer
>> to y-or-n-p has no effect on the question “Recursively delete ...? ”.
>> It still expects “[yes, no, all, quit, help]” answers, not short ones
>> like “y/n/!/q/?”.
>
> If you want to use y-or-n-p there, you will have to replace
> dired--yes-no-all-quit-help with your own function.

Thanks for the idea.  Here is the first version of its implementation:

diff --git a/lisp/dired.el b/lisp/dired.el
index b853d64..e6a7eeb 100644
--- a/lisp/dired.el
+++ b/lisp/dired.el
@@ -2997,6 +2998,8 @@ dired-recursive-deletes
 ;; Match anything but `.' and `..'.
 (defvar dired-re-no-dot "^\\([^.]\\|\\.\\([^.]\\|\\..\\)\\).*")
 
+(defvar dired-recursive-deletion-confirmer 'dired--yes-no-all-quit-help) ;; or 'dired--y-n-!-q-?
+
 (defconst dired-delete-help
   "Type:
 `yes' to delete recursively the current directory,
@@ -3005,6 +3008,14 @@ dired-delete-help
 `quit' to exit,
 `help' to show this help message.")
 
+(defconst dired-delete-help-short
+  "Type:
+`y' to delete recursively the current directory,
+`n' to skip to next,
+`!' to delete all remaining directories with no more questions,
+`q' to exit,
+`?' to show this help message.")
+
 (defun dired--yes-no-all-quit-help (prompt &optional help-msg)
   "Ask a question with valid answers: yes, no, all, quit, help.
 PROMPT must end with '? ', for instance, 'Delete it? '.
@@ -3028,6 +3039,56 @@ dired--yes-no-all-quit-help
       (setq answer (funcall input-fn)))
     answer))
 
+(defvar read-short-answers
+  '(("y" "yes")
+    ("n" "no")
+    ("!" "all")
+    ("q" "quit")
+    ("?" "help")))
+
+(defvar read-short-answer-map
+  (let ((map (make-sparse-keymap)))
+    (set-keymap-parent map minibuffer-local-map)
+    (dolist (answer read-short-answers)
+      (define-key map (car answer)
+        (lambda ()
+          (interactive)
+          (delete-minibuffer-contents)
+          (insert (cadr answer))
+          (exit-minibuffer))))
+    (define-key map [remap self-insert-command]
+      (lambda ()
+        (interactive)
+        (delete-minibuffer-contents)
+        (beep)
+        (message "Please answer `y' or `n' or `!' or `q'")
+        (sleep-for 2)))
+    map)
+  "Keymap used for non-blocking reading of short one-character answers.")
+
+(defun dired--y-n-!-q-? (prompt &optional help-msg)
+  "Ask a question with valid answers: y, n, !, q, ?.
+PROMPT must end with '? ', for instance, 'Delete it? '.
+If optional arg HELP-MSG is non-nil, then is a message to show when
+the user answers '?'.  Otherwise, default to `dired-delete-help-short'."
+  (let ((valid-answers (list "yes" "no" "all" "quit"))
+        (answer "")
+        (input-fn (lambda ()
+                    (read-from-minibuffer
+                     (format "%s [y, n, !, q, ?] " prompt) nil read-short-answer-map))))
+    (setq answer (funcall input-fn))
+    (when (string= answer "help")
+      (with-help-window "*Help*"
+        (with-current-buffer "*Help*"
+          (insert (or help-msg dired-delete-help-short)))))
+    (while (not (member answer valid-answers))
+      (unless (string= answer "help")
+        (beep)
+        (message "Please answer `y' or `n' or `!' or `q'")
+        (sleep-for 2))
+      (setq answer (funcall input-fn)))
+    answer))
+
 ;; Delete file, possibly delete a directory and all its files.
 ;; This function is useful outside of dired.  One could change its name
 ;; to e.g. recursive-delete-file and put it somewhere else.
@@ -3057,7 +3118,7 @@ dired-delete-file
     "trash"
   "delete")
  (dired-make-relative file))))
-                   (pcase (dired--yes-no-all-quit-help prompt) ; Prompt user.
+                   (pcase (apply dired-recursive-deletion-confirmer (list prompt)) ; Prompt user.
                      ('"all" (setq recursive 'always dired-recursive-deletes recursive))
                      ('"yes" (if (eq recursive 'top) (setq recursive 'always)))
                      ('"no" (setq recursive nil))
                      ('"no" (setq recursive nil))



PS: and here is not a patch, but a diff that shows the difference
between two functions to help to combine them into one later:

@@ -1,22 +1,22 @@
-(defun dired--yes-no-all-quit-help (prompt &optional help-msg)
-  "Ask a question with valid answers: yes, no, all, quit, help.
+(defun dired--y-n-!-q-? (prompt &optional help-msg)
+  "Ask a question with valid answers: y, n, !, q, ?.
 PROMPT must end with '? ', for instance, 'Delete it? '.
 If optional arg HELP-MSG is non-nil, then is a message to show when
-the user answers 'help'.  Otherwise, default to `dired-delete-help'."
+the user answers '?'.  Otherwise, default to `dired-delete-help-short'."
   (let ((valid-answers (list "yes" "no" "all" "quit"))
         (answer "")
         (input-fn (lambda ()
-                    (read-string
-                     (format "%s [yes, no, all, quit, help] " prompt)))))
+                    (read-from-minibuffer
+                     (format "%s [y, n, !, q, ?] " prompt) nil read-short-answer-map))))
     (setq answer (funcall input-fn))
     (when (string= answer "help")
       (with-help-window "*Help*"
         (with-current-buffer "*Help*"
-          (insert (or help-msg dired-delete-help)))))
+          (insert (or help-msg dired-delete-help-short)))))
     (while (not (member answer valid-answers))
       (unless (string= answer "help")
         (beep)
-        (message "Please answer `yes' or `no' or `all' or `quit'")
+        (message "Please answer `y' or `n' or `!' or `q'")
         (sleep-for 2))
       (setq answer (funcall input-fn)))
     answer))



Reply | Threaded
Open this post in threaded view
|

bug#30073: 27.0.50; dired-do-delete ignores customization for short answers

Tino Calancha-2
Juri Linkov <[hidden email]> writes:

>>> What is worse, it doesn't work at all - setting dired-deletion-confirmer
>>> to y-or-n-p has no effect on the question “Recursively delete ...? ”.
>>> It still expects “[yes, no, all, quit, help]” answers, not short ones
>>> like “y/n/!/q/?”.
>>
>> If you want to use y-or-n-p there, you will have to replace
>> dired--yes-no-all-quit-help with your own function.
>
> Thanks for the idea.  Here is the first version of its implementation:
Thank you for the patch.  I like it.
Is OK if we add docstring to those vars?

-(defvar dired-recursive-deletion-confirmer 'dired--yes-no-all-quit-help) ;; or 'dired--y-n-!-q-?
+(defvar dired-recursive-deletion-confirmer 'dired--yes-no-all-quit-help ; or 'dired--y-n-!-q-?
+  "Function for asking user confirmation to delete directories recursively.
+Set it to `dired--y-n-!-q-?' if you prefer reply shortly.")
 
-(defvar dired-deletion-confirmer 'yes-or-no-p) ; or y-or-n-p?
+(defvar dired-deletion-confirmer 'yes-or-no-p ; or y-or-n-p?
+  "Function for asking user confirmation to delete files.
+Set it to `y-or-n-p' if you prefer reply shorty.")



Reply | Threaded
Open this post in threaded view
|

bug#30073: 27.0.50; dired-do-delete ignores customization for short answers

Juri Linkov-2
>> Thanks for the idea.  Here is the first version of its implementation:
> Thank you for the patch.  I like it.

But I don't like it :-)

Neither (fset 'yes-or-no-p 'y-or-n-p) nor
(advice-add 'yes-or-no-p :override #'y-or-n-p)
are good methods of customization, so dired-deletion-confirmer
and dired-recursive-deletion-confirmer are equally bad.

What I'm thinking about is introducing a boolean customizable variable
that would define whether abbreviated answers are preferred by the user.
Then a new minibuffer-reading function could accept a list of abbreviations
and map them to long full answers.

Something like ‘read-multiple-choice’ or ‘map-y-or-n-p’, but that
would allow either long or short answers depending on customization
like ‘rmail-confirm-expunge’, ‘url-confirmation-func’,
‘org-confirm-shell-link-function’, ‘org-confirm-elisp-link-function’,
or on its argument like ‘strong-query’ in ‘custom-command-apply’.

WDYT?

diff --git a/lisp/dired.el b/lisp/dired.el
index b853d64..0ce24d0 100644
--- a/lisp/dired.el
+++ b/lisp/dired.el
@@ -3005,27 +3006,60 @@ dired-delete-help
 `quit' to exit,
 `help' to show this help message.")
 
-(defun dired--yes-no-all-quit-help (prompt &optional help-msg)
-  "Ask a question with valid answers: yes, no, all, quit, help.
-PROMPT must end with '? ', for instance, 'Delete it? '.
-If optional arg HELP-MSG is non-nil, then is a message to show when
-the user answers 'help'.  Otherwise, default to `dired-delete-help'."
-  (let ((valid-answers (list "yes" "no" "all" "quit"))
-        (answer "")
-        (input-fn (lambda ()
-                    (read-string
-             (format "%s [yes, no, all, quit, help] " prompt)))))
-    (setq answer (funcall input-fn))
-    (when (string= answer "help")
-      (with-help-window "*Help*"
-        (with-current-buffer "*Help*"
-          (insert (or help-msg dired-delete-help)))))
-    (while (not (member answer valid-answers))
-      (unless (string= answer "help")
+(defcustom read-answers-short nil
+  "If non-nil, accept short answers to the question."
+  :version "27.1"
+  :type 'boolean)
+
+(defun read-answers (prompt answers &optional help-msg short)
+  (let* ((short (or short read-answers-short))
+         (prompt (format "%s [%s] " prompt
+                         (mapconcat (lambda (a)
+                                      (if short (cadr a) (car a)))
+                                    answers ", ")))
+         (message (format "Please answer %s"
+                          (mapconcat (lambda (a)
+                                       (format "`%s'" (if short (cadr a) (car a))))
+                                     answers " or ")))
+         (short-answer-map (when short
+                             (let ((map (make-sparse-keymap)))
+                               (set-keymap-parent map minibuffer-local-map)
+                               (dolist (answer read-short-answers)
+                                 (define-key map (car answer)
+                                   (lambda ()
+                                     (interactive)
+                                     (delete-minibuffer-contents)
+                                     (insert (cadr answer))
+                                     (exit-minibuffer))))
+                               (define-key map [remap self-insert-command]
+                                 (lambda ()
+                                   (interactive)
+                                   (delete-minibuffer-contents)
+                                   (beep)
+                                   (message message)
+                                   (sleep-for 2)))
+                               map)))
+         answer)
+    (while (not (assoc (setq answer
+                             (if short
+                                 (read-from-minibuffer
+                                  prompt nil short-answer-map)
+                               (read-string prompt)))
+                       answers))
+      (if (and (string= answer "help") (stringp help-msg))
+          (with-help-window "*Help*"
+            (with-current-buffer "*Help*"
+              (insert (if short
+                          (seq-reduce (lambda (msg a)
+                                        (replace-regexp-in-string
+                                         (format "`%s'" (car a))
+                                         (format "`%s'" (cadr a))
+                                         msg nil t))
+                                      answers help-msg)
+                        help-msg))))
         (beep)
-        (message "Please answer `yes' or `no' or `all' or `quit'")
-        (sleep-for 2))
-      (setq answer (funcall input-fn)))
+        (message message)
+        (sleep-for 2)))
     answer))
 
 ;; Delete file, possibly delete a directory and all its files.
@@ -3057,11 +3091,16 @@ dired-delete-file
     "trash"
   "delete")
  (dired-make-relative file))))
-                   (pcase (dired--yes-no-all-quit-help prompt) ; Prompt user.
+                   (pcase (read-answers prompt '(("yes" "y")
+                                                 ("no" "n")
+                                                 ("all" "!")
+                                                 ("quit" "q"))
+                                        dired-delete-help)
                      ('"all" (setq recursive 'always dired-recursive-deletes recursive))
                      ('"yes" (if (eq recursive 'top) (setq recursive 'always)))
                      ('"no" (setq recursive nil))
-                     ('"quit" (keyboard-quit)))))
+                     ('"quit" (keyboard-quit))
+                     (_ (keyboard-quit))))) ; catch all unknown answers
              (setq recursive nil)) ; Empty dir or recursive is nil.
            (delete-directory file recursive trash))))
 



Reply | Threaded
Open this post in threaded view
|

bug#30073: 27.0.50; dired-do-delete ignores customization for short answers

Eli Zaretskii
> From: Juri Linkov <[hidden email]>
> Cc: Eli Zaretskii <[hidden email]>,  [hidden email],  [hidden email]
> Date: Mon, 15 Jan 2018 00:53:45 +0200
>
> What I'm thinking about is introducing a boolean customizable variable
> that would define whether abbreviated answers are preferred by the user.
> Then a new minibuffer-reading function could accept a list of abbreviations
> and map them to long full answers.
>
> Something like ‘read-multiple-choice’ or ‘map-y-or-n-p’, but that
> would allow either long or short answers depending on customization
> like ‘rmail-confirm-expunge’, ‘url-confirmation-func’,
> ‘org-confirm-shell-link-function’, ‘org-confirm-elisp-link-function’,
> or on its argument like ‘strong-query’ in ‘custom-command-apply’.
>
> WDYT?

Why limit this to Dired?  People who use fset to get y-or-n-p want
that everywhere, right?




Reply | Threaded
Open this post in threaded view
|

bug#30073: 27.0.50; dired-do-delete ignores customization for short answers

Drew Adams
In reply to this post by Juri Linkov-2
> Neither (fset 'yes-or-no-p 'y-or-n-p) nor
> (advice-add 'yes-or-no-p :override #'y-or-n-p)
> are good methods of customization, so dired-deletion-confirmer
> and dired-recursive-deletion-confirmer are equally bad.
>
> What I'm thinking about is introducing a boolean customizable variable
> that would define whether abbreviated answers are preferred by the user.
> Then a new minibuffer-reading function could accept a list of
> abbreviations and map them to long full answers.
>
> Something like ‘read-multiple-choice’ or ‘map-y-or-n-p’, but that
> would allow either long or short answers depending on customization
> like ‘rmail-confirm-expunge’, ‘url-confirmation-func’,
> ‘org-confirm-shell-link-function’, ‘org-confirm-elisp-link-function’,
> or on its argument like ‘strong-query’ in ‘custom-command-apply’.
>
> WDYT?

(I haven't been following this thread.  Apologies if I
misunderstand/misspeak.)

First, why is this being discussed (only) in the context of
Dired?  Isn't this something to consider more generally?

Do we all agree on the following points?

1. Users advising the yes/no confirmation-prompt functions
   is not a good solution to users wanting to sometimes (or
   always) use a different prompting approach from the one
   chosen by the author of the code that prompts.

2. Choosing a single prompt approach (e.g. `y-or-n-p' or
   `yes-or-no-p') for all contexts might be appropriate for
   some users, but it is probably not a great idea in general.

3. Even a given user might appreciate that a given prompting
   context asks them using the slow approach (`yes-or-no-p')
   at first, or most of the time, but she might sometimes,
   or even generally after some experience, prefer that that
   prompting context use a faster approach (e.g. `y-or-n-p').

   IOW, it would be good for a user to be able to easily
   specify anytime, for a given prompting context, which
   prompting approach to use.

An code author chooses which confirmation-prompting approach
s?he thinks is most appropriate for most users in a given
context.  Typically, contexts where the wrong answer can have
graver consequences get the `yes-or-no-p' approach.

IIUC, this bug discussion is looking into the possibility of
giving users a simple way to choose one or the other approach
generally, i.e., for use everywhere, or at least everywhere
within some overall context (e.g. Dired).  Is that right?
If so, that responds to #1 above, but not to #2 (or #3).

Along those lines, instead of a Boolean value for a user
option, to choose one or the other approach everywhere,
perhaps an alist value would be good, with the alist entries
(somehow) characterizing the prompting approach to use for
different contexts (defined how?).

Even so, though helpful, that would not really be a good
response to #3 above.

______

I've proposed the following before, but I'll mention it again.
It doesn't respond to #1 or #2, so it is likely complementary
to what I think you're proposing, which is a user option to
choose generally, as an alternative to advising.  That is, it
could be complementary.

The first thing needed here is for confirmation-prompting
functions to be able to (and preferably to do it) specify the
current context in which they are called.  That will give
users a way to specify what they want for a particular such
context.

A start in that direction is to provide to the prompting
function the name of the function that is calling it.

No, that won't distinguish particular calls (occurrences).
But it will be a good start.

For example, if `y-or-n-p' is called from function `foo',
and if the call records that fact, then a user can specify
that all `y-or-n-p' calls from `foo' should be handled as
if they were `yes-or-no-p' calls.

This means adding an optional argument to `y-or-n-p' and
`yes-or-no-p' (and other confirmation-prompting functions).

If function `foo' wants to give a user control then it
would use `(y-or-n-p "Do you agree? " 'foo)' instead of
just `(y-or-n-p "Do you agree? ")'.  Likewise, for
`yes-or-no-p'.  That's the only change needed, to give
users control here.

I've implemented this in library `yes-no.el'.  I think it's
a good start.  Combined with a user option as discussed
above, and overriding the option's more general behavior,
it could offer quite a lot of user control unobtrusively
and straightforwardly.

`yes-no.el' works by allowing, besides the y/n and yes/no
responses, a response that says "Switch to using the other
prompting method from now on".  It thus lets users change
the prompting function interactively for the given calling
function.

In addition or alternatively, users can specify such
preferences using a user option.  Specifying behavior
interactively updates the option value and saves it.  IOW,
interactive specification is an on-the-fly shortcut to
using Customize.

Rather than describe here just what it does and how, I
invite you to have a look at the Commentary or code of
the library, if you're interested.  But the easiest way
to see what it does is just to try it.

https://www.emacswiki.org/emacs/download/yes-no.el

I think it responds to the twin needs for (1) code author
being able to judge which behavior is preferable for most
users most of the time and (2) user being able to change
the behavior anytime.



Reply | Threaded
Open this post in threaded view
|

bug#30073: 27.0.50; dired-do-delete ignores customization for short answers

Juri Linkov-2
In reply to this post by Eli Zaretskii
>> What I'm thinking about is introducing a boolean customizable variable
>> that would define whether abbreviated answers are preferred by the user.
>> Then a new minibuffer-reading function could accept a list of abbreviations
>> and map them to long full answers.
>>
>> Something like ‘read-multiple-choice’ or ‘map-y-or-n-p’, but that
>> would allow either long or short answers depending on customization
>> like ‘rmail-confirm-expunge’, ‘url-confirmation-func’,
>> ‘org-confirm-shell-link-function’, ‘org-confirm-elisp-link-function’,
>> or on its argument like ‘strong-query’ in ‘custom-command-apply’.
>>
>> WDYT?
>
> Why limit this to Dired?  People who use fset to get y-or-n-p want
> that everywhere, right?

Right, this is not specific to Dired.  So we could add a new
customizable variable, and easily change ‘yes-or-no-p’ to call
‘y-or-n-p’ if it's non-nil.  Oh, wait, not so easily,
I see that ‘yes-or-no-p’ is implemented in C.
Is there a particular reason why it's not in Lisp?

In any case this would be better customization to choose between
“yes/no” and “y/n”.  Regarding a new multiple options reading function
‘read-answer’, I'm not sure where to put it.  There is a separate file
map-ynp.el for a single function ‘map-y-or-n-p’, and a separate file
rmc.el for a single function ‘read-multiple-choice’.  But I hesitate
to do the same and create a new file for just one function.



Reply | Threaded
Open this post in threaded view
|

bug#30073: 27.0.50; dired-do-delete ignores customization for short answers

Juri Linkov-2
In reply to this post by Drew Adams
> Do we all agree on the following points?
>
> 1. Users advising the yes/no confirmation-prompt functions
>    is not a good solution to users wanting to sometimes (or
>    always) use a different prompting approach from the one
>    chosen by the author of the code that prompts.

I agree that advising the yes/no confirmation is not good
from a customization standpoint.

> 2. Choosing a single prompt approach (e.g. `y-or-n-p' or
>    `yes-or-no-p') for all contexts might be appropriate for
>    some users, but it is probably not a great idea in general.

Please see an optional argument ‘short’ that I added to my previous
patch.  It will allow using short answers even when customizable
variable is nil where code authors deem appropriate.

> 3. Even a given user might appreciate that a given prompting
>    context asks them using the slow approach (`yes-or-no-p')
>    at first, or most of the time, but she might sometimes,
>    or even generally after some experience, prefer that that
>    prompting context use a faster approach (e.g. `y-or-n-p').

I'm not sure if we need more fine-grained customization.
If the user decides that a short answer is enough, enough is enough.



Reply | Threaded
Open this post in threaded view
|

bug#30073: 27.0.50; dired-do-delete ignores customization for short answers

Drew Adams
> I agree that advising the yes/no confirmation is not good
> from a customization standpoint.
>
>> 2. Choosing a single prompt approach (e.g. `y-or-n-p' or
>>    `yes-or-no-p') for all contexts might be appropriate for
>>    some users, but it is probably not a great idea in general.
>
> Please see an optional argument ‘short’ that I added to my previous
> patch.  It will allow using short answers even when customizable
> variable is nil where code authors deem appropriate.

I saw it.  That's not the problem to solve, IMO.

It's not just about giving users a cleaner way to get
`y-or-n-p' behavior globally, in place of advising
`yes-or-no-p'.  (I think/hope you agree with that much.)

But it's also not about letting code authors decide
which to use, even by overriding a user's preference.

There's no need for that at all, AFAICS.  Authors
can already get "short" behavior anytime they want,
and overriding a user setting that way would not be
helpful.

You're solving the wrong problem, I think.

It's not only about moving from long to short.  A user
can want to go the other way too.  And it's definitely
not just about a systematic move in one direction or
the other.

Just as each _author_ can (and does) decide, for any
given context, which kind of confirmation prompting to
use, so can a _user_ make her own judgment about that.
We're just not giving her a way to do that, today.

We don't provide library authors with just a binary
choice: ALL of your confirmation prompts must be long
or ALL of them must be short.  That would be silly.

For the exact same reasons we should allow _users_ the
same possibilities to judge and decide what behavior
they want here and there.  An author gets to decide
that; so should users - a fortiori.

We just need to provide an easy-to-use way to choose.

That's what I've tried to do with `yes-no.el'.  Other
approaches to solving that problem are possible.  But
that's the problem to solve, IMO.  Not just providing
a substitute for advising - another way to give users
an all-or-nothing choice.  And not giving authors a
way to override a user's choice.

Users are different.  They have different degrees of
familiarity with Emacs and different degrees of
confidence about the use of this or that part of Emacs.

And those things can and do change over time.  The
first time you use an operation that deletes a file
you might well want it to make you confirm slowly.
After using it 1000 times you might want it to let
you just hit `y'.  Or not.

>> 3. Even a given user might appreciate that a given prompting
>>    context asks them using the slow approach (`yes-or-no-p')
>>    at first, or most of the time, but she might sometimes,
>>    or even generally after some experience, prefer that that
>>    prompting context use a faster approach (e.g. `y-or-n-p').
>
> I'm not sure if we need more fine-grained customization.
> If the user decides that a short answer is enough, enough is
> enough.

I disagree.  It's not about just a general movement,
everywhere from `yes-or-no-p' to `y-or-n-p'.  It's
about the particular user and the particular context.

A given library might (from a given user's point of
view) inappropriately use one or the other approach.
It might use `yes-or-no-p' behavior everywhere,
annoying many users.  Or it might inappropriately
use `y-or-n-p' behavior everywhere.

There is nothing that guarantees a match between an
author's idea of what is best overall and a user's
idea of what is best for her in a given context at
a given time.

Did you try `yes-no.el'?  The implementation and
use are pretty simple - not a big deal.



Reply | Threaded
Open this post in threaded view
|

bug#30073: 27.0.50; dired-do-delete ignores customization for short answers

Eli Zaretskii
In reply to this post by Juri Linkov-2
> From: Juri Linkov <[hidden email]>
> Cc: [hidden email],  [hidden email],  [hidden email]
> Date: Tue, 16 Jan 2018 01:02:27 +0200
>
> > Why limit this to Dired?  People who use fset to get y-or-n-p want
> > that everywhere, right?
>
> Right, this is not specific to Dired.  So we could add a new
> customizable variable, and easily change ‘yes-or-no-p’ to call
> ‘y-or-n-p’ if it's non-nil.  Oh, wait, not so easily,
> I see that ‘yes-or-no-p’ is implemented in C.

Why is that a problem?  C code can access Lisp variables and can call
Lips functions.

> Is there a particular reason why it's not in Lisp?

Probably history, coupled with the fact that a few things it does are
easier done in C.

> In any case this would be better customization to choose between
> “yes/no” and “y/n”.

Why better?

> Regarding a new multiple options reading function ‘read-answer’, I'm
> not sure where to put it.

How many callers does it have now, and which callers are those?



Reply | Threaded
Open this post in threaded view
|

bug#30073: 27.0.50; dired-do-delete ignores customization for short answers

Juri Linkov-2
In reply to this post by Drew Adams
> We just need to provide an easy-to-use way to choose.

I agree that better customization for yes/no questions would be nice
to have, we could create a separate feature request for that.

But what I'm addressing here in bug#30073 is a regression.
A question for deletions in Dired used to respect user configuration
for accepting short answers, but now it always requires typing
long answers.  This is very annoying.

I'll post a new patch shortly to finally fix this issue.



Reply | Threaded
Open this post in threaded view
|

bug#30073: 27.0.50; dired-do-delete ignores customization for short answers

Juri Linkov-2
In reply to this post by Eli Zaretskii
>> Regarding a new multiple options reading function ‘read-answer’, I'm
>> not sure where to put it.
>
> How many callers does it have now, and which callers are those?

Currently it will have just one caller in ‘dired-delete-file’,
but it's possible to gradually replace all callers that are using
‘yes-or-no-p’ and ‘y-or-n-p’ with equivalent calls of ‘read-answer’
with the argument

  '(("yes"  ?y)
    ("no"   ?n))

that will accept either long or short answer depending on the new
customizable variable.

In this patch I placed new code to map-ynp.el because in loadup.el
"emacs-lisp/map-ynp" is loaded immediately after loading "custom",
so map-ynp.el is the first file that allows using defcustom.
Most importantly, a new function is logically related to map-y-or-n-p,
and grouping related functions in one file is much better than polluting
file namespace with separate files such as map-ynp.el, rmc.el, crm.el...

diff --git a/lisp/emacs-lisp/map-ynp.el b/lisp/emacs-lisp/map-ynp.el
index 2a7edde..85ed9e0 100644
--- a/lisp/emacs-lisp/map-ynp.el
+++ b/lisp/emacs-lisp/map-ynp.el
@@ -256,4 +256,86 @@ map-y-or-n-p
     ;; Return the number of actions that were taken.
     actions))
 
+
+;; For backward compatibility check if short y/n answers are preferred.
+(defcustom read-answer-short (eq (symbol-function 'yes-or-no-p) 'y-or-n-p)
+  "If non-nil, accept short answers to the question."
+  :type 'boolean
+  :version "27.1"
+  :group 'minibuffer)
+
+(defun read-answer (question answers)
+  "Read an answer either as a complete word or its character abbreviation.
+Ask QUESTION and accept an answer from the list of possible ANSWERS.
+This list contains lists in the following format:
+  (LONG-ANSWER SHORT-ANSWER HELP-MESSAGE)
+where
+  LONG-ANSWER is a complete answer,
+  SHORT-ANSWER is an abbreviated one-letter answer,
+  HELP-MESSAGE is a string describing the meaning of the answer.
+
+Example:
+  '((\"yes\"  ?y \"perform the action\")
+    (\"no\"   ?n \"skip to the next\")
+    (\"all\"  ?! \"accept all remaining without more questions\")
+    (\"quit\" ?q \"exit\"))
+
+When `read-answer-short' is non-nil, accept short answers.
+
+Return a long answer even in case of accepting short ones."
+  (custom-reevaluate-setting 'read-answer-short)
+  (let* ((short read-answer-short)
+         (answers-with-help (append answers '(("help" ?? "show this help message"))))
+         (prompt (format "%s(%s) " question
+                         (mapconcat (lambda (a)
+                                      (if short (format "%c=%s" (nth 1 a) (nth 0 a)) (nth 0 a)))
+                                    answers-with-help ", ")))
+         (message (format "Please answer %s."
+                          (mapconcat (lambda (a)
+                                       (format "`%s'" (if short (string (nth 1 a)) (nth 0 a))))
+                                     answers-with-help " or ")))
+         (short-answer-map (when short
+                             (let ((map (make-sparse-keymap)))
+                               (set-keymap-parent map minibuffer-local-map)
+                               (dolist (answer answers-with-help)
+                                 (define-key map (vector (nth 1 answer))
+                                   (lambda ()
+                                     (interactive)
+                                     (delete-minibuffer-contents)
+                                     (insert (nth 0 answer))
+                                     (exit-minibuffer))))
+                               (define-key map [remap self-insert-command]
+                                 (lambda ()
+                                   (interactive)
+                                   (delete-minibuffer-contents)
+                                   (beep)
+                                   (message message)
+                                   (sleep-for 2)))
+                               map)))
+         answer)
+    (while (not (assoc (setq answer (cond (short
+                                           (read-from-minibuffer
+                                            prompt nil short-answer-map))
+                                          (t
+                                           (read-from-minibuffer
+                                            prompt nil nil nil
+                                            'yes-or-no-p-history))))
+                       answers))
+      (if (string= answer "help")
+          (with-help-window "*Help*"
+            (with-current-buffer "*Help*"
+              (insert "Type:\n"
+                      (mapconcat (lambda (a)
+                                   (format "`%s' to %s"
+                                           (if short
+                                               (format "%c (%s)" (nth 1 a) (nth 0 a))
+                                             (nth 0 a))
+                                           (nth 2 a)))
+                                 answers-with-help ",\n")
+                      ".\n")))
+        (beep)
+        (message message)
+        (sleep-for 2)))
+    answer))
+
 ;;; map-ynp.el ends here
diff --git a/lisp/dired.el b/lisp/dired.el
index b853d64..9a412d0 100644
--- a/lisp/dired.el
+++ b/lisp/dired.el
@@ -2997,37 +2998,6 @@ dired-recursive-deletes
 ;; Match anything but `.' and `..'.
 (defvar dired-re-no-dot "^\\([^.]\\|\\.\\([^.]\\|\\..\\)\\).*")
 
-(defconst dired-delete-help
-  "Type:
-`yes' to delete recursively the current directory,
-`no' to skip to next,
-`all' to delete all remaining directories with no more questions,
-`quit' to exit,
-`help' to show this help message.")
-
-(defun dired--yes-no-all-quit-help (prompt &optional help-msg)
-  "Ask a question with valid answers: yes, no, all, quit, help.
-PROMPT must end with '? ', for instance, 'Delete it? '.
-If optional arg HELP-MSG is non-nil, then is a message to show when
-the user answers 'help'.  Otherwise, default to `dired-delete-help'."
-  (let ((valid-answers (list "yes" "no" "all" "quit"))
-        (answer "")
-        (input-fn (lambda ()
-                    (read-string
-             (format "%s [yes, no, all, quit, help] " prompt)))))
-    (setq answer (funcall input-fn))
-    (when (string= answer "help")
-      (with-help-window "*Help*"
-        (with-current-buffer "*Help*"
-          (insert (or help-msg dired-delete-help)))))
-    (while (not (member answer valid-answers))
-      (unless (string= answer "help")
-        (beep)
-        (message "Please answer `yes' or `no' or `all' or `quit'")
-        (sleep-for 2))
-      (setq answer (funcall input-fn)))
-    answer))
-
 ;; Delete file, possibly delete a directory and all its files.
 ;; This function is useful outside of dired.  One could change its name
 ;; to e.g. recursive-delete-file and put it somewhere else.
@@ -3057,11 +3027,17 @@ dired-delete-file
     "trash"
   "delete")
  (dired-make-relative file))))
-                   (pcase (dired--yes-no-all-quit-help prompt) ; Prompt user.
+                   (pcase (read-answer
+                           prompt
+                           '(("yes"  ?y "delete recursively the current directory")
+                             ("no"   ?n "skip to next")
+                             ("all"  ?! "delete all remaining directories with no more questions")
+                             ("quit" ?q "exit")))
                      ('"all" (setq recursive 'always dired-recursive-deletes recursive))
                      ('"yes" (if (eq recursive 'top) (setq recursive 'always)))
                      ('"no" (setq recursive nil))
-                     ('"quit" (keyboard-quit)))))
+                     ('"quit" (keyboard-quit))
+                     (_ (keyboard-quit))))) ; catch all unknown answers
              (setq recursive nil)) ; Empty dir or recursive is nil.
            (delete-directory file recursive trash))))
 
diff --git a/test/lisp/dired-tests.el b/test/lisp/dired-tests.el
index c024213..bb0e1bc 100644
--- a/test/lisp/dired-tests.el
+++ b/test/lisp/dired-tests.el
@@ -384,9 +384,9 @@ dired-test-with-temp-dirs
   (dired-test-with-temp-dirs
    'just-empty-dirs
    (let (asked)
-     (advice-add 'dired--yes-no-all-quit-help
+     (advice-add 'read-answer
                  :override
-                 (lambda (_) (setq asked t) "")
+                 (lambda (_q _a) (setq asked t) "")
                  '((name . dired-test-bug27940-advice)))
      (dired default-directory)
      (dired-toggle-marks)
@@ -395,44 +395,44 @@ dired-test-with-temp-dirs
          (progn
            (should-not asked)
            (should-not (dired-get-marked-files))) ; All dirs deleted.
-       (advice-remove 'dired--yes-no-all-quit-help 'dired-test-bug27940-advice))))
+       (advice-remove 'read-answer 'dired-test-bug27940-advice))))
   ;; Answer yes
   (dired-test-with-temp-dirs
    nil
-   (advice-add 'dired--yes-no-all-quit-help :override (lambda (_) "yes")
+   (advice-add 'read-answer :override (lambda (_q _a) "yes")
                '((name . dired-test-bug27940-advice)))
    (dired default-directory)
    (dired-toggle-marks)
    (dired-do-delete nil)
    (unwind-protect
        (should-not (dired-get-marked-files)) ; All dirs deleted.
-     (advice-remove 'dired--yes-no-all-quit-help 'dired-test-bug27940-advice)))
+     (advice-remove 'read-answer 'dired-test-bug27940-advice)))
   ;; Answer no
   (dired-test-with-temp-dirs
    nil
-   (advice-add 'dired--yes-no-all-quit-help :override (lambda (_) "no")
+   (advice-add 'read-answer :override (lambda (_q _a) "no")
                '((name . dired-test-bug27940-advice)))
    (dired default-directory)
    (dired-toggle-marks)
    (dired-do-delete nil)
    (unwind-protect
        (should (= 5 (length (dired-get-marked-files)))) ; Just the empty dirs deleted.
-     (advice-remove 'dired--yes-no-all-quit-help 'dired-test-bug27940-advice)))
+     (advice-remove 'read-answer 'dired-test-bug27940-advice)))
   ;; Answer all
   (dired-test-with-temp-dirs
    nil
-   (advice-add 'dired--yes-no-all-quit-help :override (lambda (_) "all")
+   (advice-add 'read-answer :override (lambda (_q _a) "all")
                '((name . dired-test-bug27940-advice)))
    (dired default-directory)
    (dired-toggle-marks)
    (dired-do-delete nil)
    (unwind-protect
        (should-not (dired-get-marked-files)) ; All dirs deleted.
-     (advice-remove 'dired--yes-no-all-quit-help 'dired-test-bug27940-advice)))
+     (advice-remove 'read-answer 'dired-test-bug27940-advice)))
   ;; Answer quit
   (dired-test-with-temp-dirs
    nil
-   (advice-add 'dired--yes-no-all-quit-help :override (lambda (_) "quit")
+   (advice-add 'read-answer :override (lambda (_q _a) "quit")
                '((name . dired-test-bug27940-advice)))
    (dired default-directory)
    (dired-toggle-marks)
@@ -440,7 +440,7 @@ dired-test-with-temp-dirs
      (dired-do-delete nil))
    (unwind-protect
        (should (= 6 (length (dired-get-marked-files)))) ; All empty dirs but zeta-empty-dir deleted.
-     (advice-remove 'dired--yes-no-all-quit-help 'dired-test-bug27940-advice))))
+     (advice-remove 'read-answer 'dired-test-bug27940-advice))))
 
 
 (provide 'dired-tests)



12