GNU bug report logs - #46374
28.0.50; Ask me to save buffers only if they are under callers dir

Previous Next

Package: emacs;

Reported by: Tino Calancha <tino.calancha <at> gmail.com>

Date: Sun, 7 Feb 2021 22:33:01 UTC

Severity: wishlist

Tags: fixed

Merged with 50380

Fixed in version 28.0.60

Done: Juri Linkov <juri <at> linkov.net>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 46374 in the body.
You can then email your comments to 46374 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to monnier <at> iro.umontreal.ca, uyennhi.qm <at> gmail.com, bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Sun, 07 Feb 2021 22:33:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Tino Calancha <tino.calancha <at> gmail.com>:
New bug report received and forwarded. Copy sent to monnier <at> iro.umontreal.ca, uyennhi.qm <at> gmail.com, bug-gnu-emacs <at> gnu.org.

Your message specified a Severity: in the pseudo-header, but the severity value patch was not recognised. The default severity normal is being used instead. The recognised values are: critical, grave, serious, important, normal, minor, wishlist.

(Sun, 07 Feb 2021 22:33:02 GMT) Full text and rfc822 format available.


Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Tino Calancha <tino.calancha <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 28.0.50; Ask me to save buffers only if they are under callers dir
Date: Sun, 07 Feb 2021 23:32:07 +0100
X-Debbugs-Cc: Stefan Monnier <monnier <at> iro.umontreal.ca>, <uyennhi.qm <at> gmail.com>
Severity: wishlist
Severity: patch

I wish, by default, to only been prompted for buffers whose default-directory
is under the caller dir (except when closing Emacs).

## Description
Everyday I connect via Tramp with machines at each of my properties (Hawaii,
Maldives, Palawan, and a very large etc.)
Then, in the same Emacs session, I connect to a host in Wall Street to see
how my stock grows, making me richer.
Often, a compilation buffer at New York city prompts me to save a buffer
at Waikiki beach.  Quite distracting! Cannot focus on my money!

I found some people with related issues:
https://emacs.stackexchange.com/questions/7268/package-el-asks-whether-i-want-to-save-modified-files-before-package-installatio
https://emacs.stackexchange.com/questions/40593/automatically-dont-save-buffers-before-compiling

## How to reproduce
Sure, I understand not all of you can possibly reproduce the above conditions.
Maybe you can try the following poor man's recipe:

emacs -Q ~/foo.txt
;; write something and do not save
foo
;; now visit another, for instance, the Emacs source dir
C-x d EMACS-SRC-DIR RET
;; call rgrep with whatever string
M-x rgrep money RET *.el RET RET
;; You will be prompted to save ~/foo.txt

I am aware of `grep-save-buffers', `compilation-save-buffers-predicate' and the solutions
proposed in the links above.

My proposal adds a new option `save-some-buffers-restrict-to-caller-subdirs'.
I am already using it for a while with joy (10 bagger at GameStop using it!).


--8<-----------------------------cut here---------------start------------->8---
commit 85e5399f035fb698fcfbb50ca01980fbbc68707c
Author: Tino Calancha <ccalancha <at> suse.com>
Date:   Thu Feb 4 21:39:37 2021 +0100

    save-some-buffers: Add option restricting to files in a caller's subdir
    
    Restrict the action to buffers with `default-directory' lying in a
    subdir of the `default-directory' from where the command is invoked.
    
    * lisp/files.el (save-some-buffers-restrict-to-caller-subdirs): New option.
    (save-some-buffers)
    (save-buffers-kill-emacs): Use it.
    
    * doc/emacs/files.texi (Save Commands)
    * doc/lispref/files.texi (Saving Buffers): Document it.
    
    * etc/NEWS (Editing Changes in Emacs 28.1): Announce this change.
    
    * lisp/progmodes/grep.el (grep-save-buffers)
    * lisp/progmodes/compile.el (compilation-save-buffers-predicate):
    Mention it in the docstring.
    
    * test/lisp/files-tests.el (files-tests--save-some-buffers): Helper function.
    (files-tests-with-all-permutations)
    (files-tests--with-buffer-offer-save): Helper macros.
    (files-tests-save-some-buffers)
    (files-tests-buffer-offer-save)
    (files-tests-save-buffers-kill-emacs--asks-to-save-buffers): New tests.

diff --git a/doc/emacs/files.texi b/doc/emacs/files.texi
index 6b3bc430d9..36d38218e9 100644
--- a/doc/emacs/files.texi
+++ b/doc/emacs/files.texi
@@ -433,9 +433,11 @@ Save Commands
 
 @noindent
 @vindex save-some-buffers-default-predicate
+@vindex save-some-buffers-restrict-to-caller-subdirs
 You can customize the value of
-@code{save-some-buffers-default-predicate} to control which buffers
-Emacs will ask about.
+@code{save-some-buffers-default-predicate} and
+@code{save-some-buffers-restrict-to-caller-subdirs} to control which
+buffers Emacs will ask about.
 
   @kbd{C-x C-c}, the key sequence to exit Emacs, invokes
 @code{save-some-buffers} and therefore asks the same questions.
diff --git a/doc/lispref/files.texi b/doc/lispref/files.texi
index 4110c51099..a9855fef2b 100644
--- a/doc/lispref/files.texi
+++ b/doc/lispref/files.texi
@@ -371,6 +371,7 @@ Saving Buffers
 querying the user.
 
 @vindex save-some-buffers-default-predicate
+@vindex save-some-buffers-restrict-to-caller-subdirs
 The optional @var{pred} argument provides a predicate that controls
 which buffers to ask about (or to save silently if
 @var{save-silently-p} is non-@code{nil}).  If @var{pred} is
@@ -381,8 +382,12 @@ Saving Buffers
 other non-file buffers---those that have a non-@code{nil} buffer-local
 value of @code{buffer-offer-save} (@pxref{Killing Buffers}).  A user
 who says @samp{yes} to saving a non-file buffer is asked to specify
-the file name to use.  The @code{save-buffers-kill-emacs} function
-passes the value @code{t} for @var{pred}.
+the file name to use.  The option
+@code{save-some-buffers-restrict-to-caller-subdirs} restricts the
+action of this command to buffers with @code{default-directory} in a
+subdirectory of the caller's @code{default-directory}.  The
+@code{save-buffers-kill-emacs} function ignores this option and passes
+the value @code{t} for @var{pred}.
 
 If the predicate is neither @code{t} nor @code{nil}, then it should be
 a function of no arguments.  It will be called in each buffer to decide
diff --git a/etc/NEWS b/etc/NEWS
index b3d53bf73c..f1bd21f26a 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -247,6 +247,11 @@ commands.  The new keystrokes are 'C-x x g' ('revert-buffer'),
 
 * Editing Changes in Emacs 28.1
 
++++
+** The new option 'save-some-buffers-restrict-to-caller-subdirs'
+restricts the action of 'same-some-buffers' to buffers with
+'default-directory' in a subdir of the callers 'default-directory'.
+
 ---
 ** 'eval-expression' now no longer signals an error on incomplete expressions.
 Previously, typing 'M-: ( RET' would result in Emacs saying "End of
diff --git a/lisp/files.el b/lisp/files.el
index dada69c145..d51cd58217 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -5520,6 +5520,19 @@ save-some-buffers-default-predicate
   :type '(choice (const :tag "Default" nil) function)
   :version "26.1")
 
+(defcustom save-some-buffers-restrict-to-caller-subdirs nil
+  "Only save buffers under caller's default directory.
+I.e., only prompt for modified buffers whose `default-directory' is in
+in a subdir of the directory from where `save-some-buffers' is
+invoked.
+Note that `save-buffers-kill-emacs' ignores this value and prompts for
+any unsaved buffer."
+  :group 'auto-save
+  :type '(choice
+          (const :tag "All buffers" nil)
+          (const :tag "Buffers under caller's default directory" t))
+  :version "28.1")
+
 (defun save-some-buffers (&optional arg pred)
   "Save some modified file-visiting buffers.  Asks user about each one.
 You can answer `y' or SPC to save, `n' or DEL not to save, `C-r'
@@ -5543,12 +5556,35 @@ save-some-buffers
 to consider it or not when called with that buffer current.
 PRED defaults to the value of `save-some-buffers-default-predicate'.
 
+You can restrict to modified buffers with `default-directory' under
+the caller's `default-directory' with
+`save-some-buffers-restrict-to-caller-subdirs'.
+
 See `save-some-buffers-action-alist' if you want to
 change the additional actions you can take on files."
   (interactive "P")
-  (unless pred
-    (setq pred save-some-buffers-default-predicate))
-  (let* ((switched-buffer nil)
+  (let* ((caller-dir default-directory)
+         (maybe-save-buffer-p
+          (lambda (buffer)
+            (or (not save-some-buffers-restrict-to-caller-subdirs)
+                (file-in-directory-p (buffer-local-value 'default-directory buffer)
+                                     caller-dir))))
+         (effective-pred
+          (unless (eq t pred)
+            (let ((def-pred save-some-buffers-default-predicate))
+              (lambda () (and (funcall maybe-save-buffer-p (current-buffer))
+                              (if (functionp pred) (funcall pred)
+                                (or (not (functionp def-pred))
+                                    (funcall def-pred))))))))
+         (switched-buffer nil)
+         (non-visiting-buffers-ok (not (null pred)))
+         (buffer-name-matches-filename-p
+          (lambda (buffer)
+            "Return non-nil if BUFFER name is similar to its file name."
+            (let ((file-basename (file-name-nondirectory (buffer-file-name buffer))))
+              (or (equal (buffer-name buffer) file-basename)
+                  (string-match-p (format "\\<%s<[^>]*>\\'" (regexp-quote file-basename))
+                                  (buffer-name buffer))))))
          (save-some-buffers--switch-window-callback
           (lambda (buffer)
             (setq switched-buffer buffer)))
@@ -5578,36 +5614,20 @@ save-some-buffers
                          (buffer-file-name buffer)
                          (with-current-buffer buffer
                            (or (eq buffer-offer-save 'always)
-                               (and pred buffer-offer-save
-                                    (> (buffer-size) 0)))))
-                        (or (not (functionp pred))
-                            (with-current-buffer buffer (funcall pred)))
+                               (and non-visiting-buffers-ok buffer-offer-save (> (buffer-size) 0)))))
+                        (or (not (functionp effective-pred))
+                            (with-current-buffer buffer (funcall effective-pred)))
                         (if arg
-                            t
+                            (funcall maybe-save-buffer-p buffer)
                           (setq queried t)
-                          (if (buffer-file-name buffer)
-                              (if (or
-                                   (equal (buffer-name buffer)
-                                          (file-name-nondirectory
-                                           (buffer-file-name buffer)))
-                                   (string-match
-                                    (concat "\\<"
-                                            (regexp-quote
-                                             (file-name-nondirectory
-                                              (buffer-file-name buffer)))
-                                            "<[^>]*>\\'")
-                                    (buffer-name buffer)))
-                                  ;; The buffer name is similar to the
-                                  ;; file name.
-                                  (format "Save file %s? "
-                                          (buffer-file-name buffer))
-                                ;; The buffer and file names are
-                                ;; dissimilar; display both.
-                                (format "Save file %s (buffer %s)? "
-                                        (buffer-file-name buffer)
-                                        (buffer-name buffer)))
-                            ;; No file name
-                            (format "Save buffer %s? " (buffer-name buffer))))))
+                          (when (funcall maybe-save-buffer-p buffer)
+                            (cond ((null (buffer-file-name buffer))
+                                   (format "Save buffer %s? " (buffer-name buffer)))
+                                  ((funcall buffer-name-matches-filename-p buffer)
+                                   (format "Save file %s? " (buffer-file-name buffer)))
+                                  (t (format "Save file %s (buffer %s)? "
+                                             (buffer-file-name buffer)
+                                             (buffer-name buffer))))))))
                  (lambda (buffer)
                    (with-current-buffer buffer
                      (save-buffer)))
@@ -7362,7 +7382,8 @@ save-buffers-kill-emacs
   (interactive "P")
   ;; Don't use save-some-buffers-default-predicate, because we want
   ;; to ask about all the buffers before killing Emacs.
-  (save-some-buffers arg t)
+  (let ((save-some-buffers-restrict-to-caller-subdirs nil))
+    (save-some-buffers arg t))
   (let ((confirm confirm-kill-emacs))
     (and
      (or (not (memq t (mapcar (lambda (buf)
diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
index 48b5ee9973..d3d3849c83 100644
--- a/lisp/progmodes/compile.el
+++ b/lisp/progmodes/compile.el
@@ -790,7 +790,10 @@ compilation-save-buffers-predicate
     (string-prefix-p my-compilation-root (file-truename (buffer-file-name))))
 to limit saving to files located under `my-compilation-root'.
 Note, that, in general, `compilation-directory' cannot be used instead
-of `my-compilation-root' here."
+of `my-compilation-root' here.
+
+See `save-some-buffers-restrict-to-caller-subdirs' for a consistent
+way to achieve this."
   :type '(choice
           (const :tag "Default (save all file-visiting buffers)" nil)
           (const :tag "Save all buffers" t)
diff --git a/lisp/progmodes/grep.el b/lisp/progmodes/grep.el
index d6ee8bb423..5454d47211 100644
--- a/lisp/progmodes/grep.el
+++ b/lisp/progmodes/grep.el
@@ -246,7 +246,11 @@ grep-save-buffers
 buffer should be saved or not.  E.g., one can set this to
   (lambda ()
     (string-prefix-p my-grep-root (file-truename (buffer-file-name))))
-to limit saving to files located under `my-grep-root'."
+to limit saving to files located under `my-grep-root'.
+
+Note that `my-grep-root' is only known at runtime.  See
+`save-some-buffers-restrict-to-caller-subdirs' for a consistent way to
+achieve the same goal."
   :version "26.1"
   :type '(choice
           (const :tag "Ask before saving" ask)
diff --git a/test/lisp/files-tests.el b/test/lisp/files-tests.el
index 149cc689ae..14d6bc099d 100644
--- a/test/lisp/files-tests.el
+++ b/test/lisp/files-tests.el
@@ -1432,5 +1432,206 @@ files-tests-revert-buffer-with-fine-grain
                                (buffer-substring (point-min) (point-max))
                                nil nil)))))
 
-(provide 'files-tests)
+(defun files-tests--save-some-buffers (pred caller-subdirs-only exp-1 exp-2)
+  "Helper function to test `save-some-buffers'.
+
+This function creates two visiting-file buffers, BUF-1, BUF-2 in
+ different directories at the same level, i.e., none of them is a
+ subdir of the other; then, it modifies both buffers; finally it calls
+ `save-some-buffers' from BUF-1 with first arg t and second arg PRED
+ and `save-some-buffers-restrict-to-caller-subdirs' let-bound to
+ CALLER-SUBDIRS-ONLY.
+
+EXP-1 and EXP-1 are the expected values of the modified flags of BUF-1
+and BUF-2 after the `save-some-buffers' call.
+
+The test is repeated with `save-some-buffers-default-predicate'
+let-bound to PRED and passing nil as second arg of
+`save-some-buffers'."
+  (let* ((dir (make-temp-file "testdir" 'dir))
+         (file-1 (expand-file-name "subdir-1/file.foo" dir))
+         (file-2 (expand-file-name "subdir-2/file.bar" dir))
+         (inhibit-message t)
+         buf-1 buf-2)
+    (unwind-protect
+        (progn
+          (make-empty-file file-1 'parens)
+          (make-empty-file file-2 'parens)
+          (setq buf-1 (find-file file-1)
+                buf-2 (find-file file-2))
+          (dolist (buf (list buf-1 buf-2))
+            (with-current-buffer buf (insert "foobar\n")))
+          ;; buf-2 is ignored if `save-some-buffers-restrict-to-caller-subdirs' is non-nil.
+          (with-current-buffer buf-1
+            (let ((save-some-buffers-restrict-to-caller-subdirs caller-subdirs-only))
+              (save-some-buffers t pred))
+            (should (eq exp-1 (buffer-modified-p buf-1)))
+            (should (eq exp-2 (buffer-modified-p buf-2))))
+
+          ;; Set both buffers as modified to repeat the test.
+          (dolist (buf (list buf-1 buf-2))
+            (with-current-buffer buf (set-buffer-modified-p t)))
+          ;; Same result with: `save-some-buffers-default-predicate' -> pred, pred -> nil.
+          (with-current-buffer buf-1
+            (let ((save-some-buffers-restrict-to-caller-subdirs caller-subdirs-only)
+                  (save-some-buffers-default-predicate (and (functionp pred) pred)))
+              (save-some-buffers t nil))
+            (should (eq exp-1 (buffer-modified-p buf-1)))
+            (should (eq exp-2 (buffer-modified-p buf-2)))))
+      ;; Clean up.
+      (dolist (buf (list buf-1 buf-2))
+        (with-current-buffer buf
+          (set-buffer-modified-p nil)
+          (kill-buffer buf)))
+      (delete-directory dir 'recursive))))
+
+(ert-deftest files-tests-save-some-buffers ()
+  "Test `save-some-buffers'.
+Test the 3 cases for the second argument PRED, i.e., `nil', `t' or
+predicate."
+  (let* ((foo-file-p (lambda () (string-suffix-p ".foo" buffer-file-name)))
+         (bar-file-p (lambda () (string-suffix-p ".bar" buffer-file-name)))
+         (args-results `((nil         nil nil nil)
+                         (nil         t   nil t)
+                         (,foo-file-p nil nil t)
+                         (,bar-file-p nil t   nil)
+                         (,foo-file-p t   nil t)
+                         (,bar-file-p t   t   t)
+                         (t           nil nil nil)
+                         (t           t   nil t))))
+    (pcase-dolist (`(,pred ,caller-subdirs-only ,exp-1 ,exp-2) args-results)
+      (files-tests--save-some-buffers pred caller-subdirs-only exp-1 exp-2))))
+
+(defmacro files-tests--with-buffer-offer-save (buffers-offer fn-test fn-binders args-results)
+  "Helper macro to test `save-some-buffers' and `save-buffers-kill-emacs'.
+
+This macro creates several non-visiting-file buffers in different
+ directories at the same level, i.e., none of them is a subdir of the
+ other; then, it modifies the buffers and sets `buffer-offer-save' per
+ each buffer as specified by BUFFERS-OFFER, a list of elements
+ (BUFFER OFFER-SAVE).  Finally it calls FN-TEST from the first
+ buffer.
+
+FN-TEST is the function to test: either `save-some-buffers' or
+`save-buffers-kill-emacs'.  This function is called with
+`save-some-buffers-restrict-to-caller-subdirs' let-bound to a value
+specified inside ARGS-RESULTS.
+
+FN-BINDERS is a list of elements (FUNCTION . BINDING), where FUNCTION
+is a function symbol that this macro temporary binds to BINDING during
+the FN-TEST call.
+ARGS-RESULTS is a list of elements (FN-ARGS CALLERS-DIR RESULTS), where
+ FN-ARGS are the arguments for FN-TEST;
+ CALLERS-DIR specify the value to let-bind
+`save-some-buffers-restrict-to-caller-subdirs';
+ RESULTS are the expected results of the test."
+  (declare (debug (form symbol form form)))
+  (let ((dir (gensym "dir"))
+        (buffers (gensym "buffers")))
+    `(let* ((,dir (make-temp-file "testdir" 'dir))
+            (inhibit-message t)
+            (use-dialog-box nil)
+            ,buffers)
+       (pcase-dolist (`(,bufsym ,offer-save) ,buffers-offer)
+         (let* ((buf (generate-new-buffer (symbol-name bufsym)))
+                (subdir (expand-file-name
+                         (format "subdir-%s" (buffer-name buf))
+                         ,dir)))
+           (make-directory subdir 'parens)
+           (push buf ,buffers)
+           (with-current-buffer buf
+             (cd subdir)
+             (setq buffer-offer-save offer-save)
+             (insert "foobar\n"))))
+       (setq ,buffers (nreverse ,buffers))
+
+       (let ((nb-saved-buffers 0))
+         (unwind-protect
+             (pcase-dolist (`(,fn-test-args ,callers-dir ,expected)
+                            ,args-results)
+               (setq nb-saved-buffers 0)
+               (with-current-buffer (car ,buffers)
+                 (cl-letf
+                     (,@(mapcar (lambda (pair) `((symbol-function ,(car pair)) ,(cdr pair)))
+                                fn-binders)
+                      (save-some-buffers-restrict-to-caller-subdirs callers-dir))
+                   (apply #',fn-test fn-test-args)
+                   (should (equal nb-saved-buffers expected)))))
+           ;; Clean up.
+           (dolist (buf ,buffers)
+             (with-current-buffer buf
+               (set-buffer-modified-p nil)
+               (kill-buffer buf)))
+           (delete-directory ,dir 'recursive))))))
+
+(defmacro files-tests-with-all-permutations (permutation list &rest body)
+  "Execute BODY forms for all permutation of LIST.
+Execute the forms with the symbol PERMUTATION bound to the current
+permutation."
+  (declare (indent 2) (debug (symbol form body)))
+  (let ((vec (gensym "vec")))
+    `(let ((,vec (vconcat ,list)))
+       (cl-labels ((swap (,vec i j)
+                         (let ((tmp (aref ,vec j)))
+                           (aset ,vec j (aref ,vec i))
+                           (aset ,vec i tmp)))
+                   (permute (,vec l r)
+                            (if (= l r)
+                                (let ((,permutation (append ,vec nil)))
+                                  ,@body)
+                              (cl-loop for idx from l below (1+ r) do
+                                       (swap ,vec idx l)
+                                       (permute ,vec (1+ l) r)
+                                       (swap ,vec idx l)))))
+         (permute ,vec 0 (1- (length ,vec)))))))
+
+(ert-deftest files-tests-buffer-offer-save ()
+  "Test `save-some-buffers'.
+Check the expected behavior for non-visiting-file buffers with
+a non-nil value of `buffer-offer-save'."
+  (let* ((buffers-offer-init '((buf-1 t) (buf-2 always) (buf-3 nil)))
+         (nb-might-save
+          (length
+           (cl-remove-if (lambda (l) (null (cadr l))) buffers-offer-init)))
+         (nb-always-save
+          (length
+           (cl-remove-if-not (lambda (l) (eq 'always (cadr l))) buffers-offer-init))))
+    (files-tests-with-all-permutations
+     buffers-offer
+     buffers-offer-init
+     (dolist (must-save `(nil t))
+       (dolist (callers-dir `(nil t))
+         (let* ((head-offer (cadar buffers-offer))
+                (res (if must-save
+                         (if callers-dir (or (and head-offer 1) 0)
+                           nb-might-save)
+                       (if callers-dir (or (and (eq 'always head-offer) 1) 0)
+                         nb-always-save)))
+                (args-res `(((nil ,must-save) ,callers-dir ,res))))
+           (files-tests--with-buffer-offer-save
+            buffers-offer
+            save-some-buffers
+            ;; Increase counter and answer 'n' when prompted to save a buffer.
+            (('read-event . (lambda () (cl-incf nb-saved-buffers) ?n)))
+            args-res)))))))
+
+(ert-deftest files-tests-save-buffers-kill-emacs--asks-to-save-buffers ()
+  "Test that `save-buffers-kill-emacs' asks to save buffers as expected."
+  (let* ((buffers-offer-init '((buf-1 t) (buf-2 always) (buf-3 nil)))
+         (nb-might-save
+          (length
+           (cl-remove-if (lambda (l) (null (cadr l))) buffers-offer-init))))
+    (files-tests-with-all-permutations
+     buffers-offer
+     buffers-offer-init
+     ;; Order doesn't matter: ask to save any buffer with non-nil `buffer-offer-save'.
+     (files-tests--with-buffer-offer-save
+      buffers-offer
+      save-buffers-kill-emacs
+      ;; Increase counter and answer 'n' when prompted to save a buffer.
+      (('read-event . (lambda () (cl-incf nb-saved-buffers) ?n))
+       ('kill-emacs . #'ignore)) ; Do not kill Emacs.
+      `((nil nil ,nb-might-save) (nil t ,nb-might-save))))))
+
+
 ;;; files-tests.el ends here

--8<-----------------------------cut here---------------end--------------->8---


In GNU Emacs 28.0.50 (build 2, x86_64-pc-linux-gnu, X toolkit, cairo version 1.16.0, Xaw scroll bars)
 of 2021-02-07 built on localhost.example.com
Repository revision: 7c5938ad7d8884d03471e2395937e11611faadb9
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12010000
System Description: openSUSE Tumbleweed

Configured using:
 'configure --with-x-toolkit=lucid'

Configured features:
CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG
LIBSELINUX LIBXML2 MODULES NOTIFY INOTIFY PDUMPER PNG SOUND THREADS TIFF
TOOLKIT_SCROLL_BARS X11 XDBE XIM XPM LUCID ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
rfc822 mml easymenu mml-sec epa derived epg epg-config gnus-util rmail
rmail-loaddefs auth-source cl-seq eieio eieio-core cl-macs
eieio-loaddefs password-cache json map text-property-search time-date
subr-x seq byte-opt gv bytecomp byte-compile cconv mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader cl-loaddefs
cl-lib sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils
iso-transl tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel term/x-win x-win term/common-win x-dnd tool-bar
dnd fontset image regexp-opt fringe tabulated-list replace newcomment
text-mode elisp-mode lisp-mode prog-mode register page tab-bar menu-bar
rfn-eshadow isearch timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core term/tty-colors frame minibuffer cl-generic
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice button loaddefs faces cus-face macroexp files
window text-properties overlay sha1 md5 base64 format env code-pages
mule custom widget hashtable-print-readable backquote threads dbusbind
inotify dynamic-setting system-font-setting font-render-setting cairo
x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 52136 5739)
 (symbols 48 6583 1)
 (strings 32 19227 1884)
 (string-bytes 1 625859)
 (vectors 16 12486)
 (vector-slots 8 169553 9399)
 (floats 8 23 41)
 (intervals 56 211 0)
 (buffers 984 10))




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Mon, 08 Feb 2021 15:08:01 GMT) Full text and rfc822 format available.

Message #8 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: 46374 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca, uyennhi.qm <at> gmail.com
Subject: Re: bug#46374: 28.0.50;
 Ask me to save buffers only if they are under callers dir
Date: Mon, 08 Feb 2021 17:07:09 +0200
> From: Tino Calancha <tino.calancha <at> gmail.com>
> Date: Sun, 07 Feb 2021 23:32:07 +0100
> Cc: stefan monnier <monnier <at> iro.umontreal.ca>, uyennhi.qm <at> gmail.com
> 
> 
> I wish, by default, to only been prompted for buffers whose default-directory
> is under the caller dir (except when closing Emacs).
> 
> ## Description
> Everyday I connect via Tramp with machines at each of my properties (Hawaii,
> Maldives, Palawan, and a very large etc.)
> Then, in the same Emacs session, I connect to a host in Wall Street to see
> how my stock grows, making me richer.
> Often, a compilation buffer at New York city prompts me to save a buffer
> at Waikiki beach.  Quite distracting! Cannot focus on my money!
> 
> I found some people with related issues:
> https://emacs.stackexchange.com/questions/7268/package-el-asks-whether-i-want-to-save-modified-files-before-package-installatio
> https://emacs.stackexchange.com/questions/40593/automatically-dont-save-buffers-before-compiling
> 
> ## How to reproduce
> Sure, I understand not all of you can possibly reproduce the above conditions.
> Maybe you can try the following poor man's recipe:
> 
> emacs -Q ~/foo.txt
> ;; write something and do not save
> foo
> ;; now visit another, for instance, the Emacs source dir
> C-x d EMACS-SRC-DIR RET
> ;; call rgrep with whatever string
> M-x rgrep money RET *.el RET RET
> ;; You will be prompted to save ~/foo.txt

You are assuming that when save-some-buffers runs, the default
directory is the same as when you issued whatever command triggered
the call to save-some-buffers?  Is that assumption really accurate?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Mon, 08 Feb 2021 15:48:01 GMT) Full text and rfc822 format available.

Message #11 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: 46374 <at> debbugs.gnu.org, uyennhi.qm <at> gmail.com
Subject: Re: bug#46374: 28.0.50; Ask me to save buffers only if they are
 under callers dir
Date: Mon, 08 Feb 2021 10:47:15 -0500
> I wish, by default, to only been prompted for buffers whose default-directory
> is under the caller dir (except when closing Emacs).

If we want to have such a behavior and make it optional, it's fine by me.

But if we want to make it a default behavior, then I think
`project-root` would be much a much better choice (when available) than
`default-directory`.


        Stefan





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Tue, 09 Feb 2021 18:16:02 GMT) Full text and rfc822 format available.

Message #14 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: 46374 <at> debbugs.gnu.org, stefan monnier <monnier <at> iro.umontreal.ca>,
 uyennhi.qm <at> gmail.com
Subject: Re: bug#46374: 28.0.50; Ask me to save buffers only if they are
 under callers dir
Date: Tue, 09 Feb 2021 19:50:33 +0200
>  @vindex save-some-buffers-default-predicate
> +@vindex save-some-buffers-restrict-to-caller-subdirs
>  You can customize the value of
> -@code{save-some-buffers-default-predicate} to control which buffers
> -Emacs will ask about.
> +@code{save-some-buffers-default-predicate} and
> +@code{save-some-buffers-restrict-to-caller-subdirs} to control which
> +buffers Emacs will ask about.

Why not simply add a new option to the existing variable
save-some-buffers-default-predicate?  For example:

(defcustom save-some-buffers-default-predicate nil
  "Default predicate for `save-some-buffers'."
  :group 'auto-save
  ;; FIXME nil should not be a valid option, let alone the default,
  ;; eg so that add-function can be used.
  :type '(choice (const :tag "Default" nil)
                 (const :tag "Subdirs of default directory" default-directory)
                 (const :tag "Project root" project-root)
                 function)




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Sun, 07 Mar 2021 20:35:01 GMT) Full text and rfc822 format available.

Message #17 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Tino Calancha <tino.calancha <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 46374 <at> debbugs.gnu.org, stefan monnier <monnier <at> iro.umontreal.ca>,
 uyennhi.qm <at> gmail.com
Subject: Re: bug#46374: 28.0.50; Ask me to save buffers only if they are
 under callers dir
Date: Sun, 07 Mar 2021 21:34:13 +0100
Sorry for the late response: many meetings with investors.
Then I went on a vacation to my castle in Seychelles.


Eli Zaretskii <eliz <at> gnu.org> writes:
>> You are assuming that when save-some-buffers runs, the default
>> directory is the same as when you issued whatever command triggered
>> the call to save-some-buffers?  Is that assumption really accurate?
I don't know if there are some corner cases.
In my experience, I am hit by this (very often) when using packages
that call `compilation-start`.  Prominent examples are `rgrep` or
when downloading ELPA updates (they will trigger a compilation as well).
The patch is serving me very well so far.


Juri Linkov <juri <at> linkov.net> writes:
>>  @vindex save-some-buffers-default-predicategn
>> +@vindex save-some-buffers-restrict-to-caller-subdirs
>>  You can customize the value of
>> -@code{save-some-buffers-default-predicate} to control which buffers
>> -Emacs will ask about.
>> +@code{save-some-buffers-default-predicate} and
>> +@code{save-some-buffers-restrict-to-caller-subdirs} to control which
>> +buffers Emacs will ask about.
>
> Why not simply add a new option to the existing variable
> save-some-buffers-default-predicate?  For example:
>
> (defcustom save-some-buffers-default-predicate nil
>   "Default predicate for `save-some-buffers'."
>   :group 'auto-save
>   ;; FIXME nil should not be a valid option, let alone the default,
>   ;; eg so that add-function can be used.
>   :type '(choice (const :tag "Default" nil)
>                  (const :tag "Subdirs of default directory" default-directory)
>                  (const :tag "Project root" project-root)
>                  function)

Indeed, this was my initial implemention.
Then I moved to the one I shared here; I found my second version
superior for the following reason:

1. Users can restrict the buffer default directory AND still pass a
   predicate to filter by any other thing they wish;
   i.e. you don't need to chose either one of the other.

2. I find it cleaner having it in a separated option.


Stefan Monnier <monnier <at> iro.umontreal.ca>
>> If we want to have such a behavior and make it optional, it's fine by me.
It's optional in my patch; some people might like the current situation:
been prompted about any modified buffer.

>> But if we want to make it a default behavior, then I think
>> `project-root` would be much a much better choice (when available) than
>> `default-directory`.

Would this `project-root` feature work for `rgrep`?
I mean, I can call `rgrep` in my local computer, out of any project, for
instance to search for information in some plain text notes.  In this
case, `default-directory` works quite well.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Sun, 07 Mar 2021 21:12:02 GMT) Full text and rfc822 format available.

Message #20 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: 46374 <at> debbugs.gnu.org, stefan monnier <monnier <at> iro.umontreal.ca>,
 uyennhi.qm <at> gmail.com
Subject: Re: bug#46374: 28.0.50; Ask me to save buffers only if they are
 under callers dir
Date: Sun, 07 Mar 2021 23:08:02 +0200
> Sorry for the late response: many meetings with investors.
> Then I went on a vacation to my castle in Seychelles.

Oh, I thought you were busy running for President.

>> Why not simply add a new option to the existing variable
>> save-some-buffers-default-predicate?  For example:
>>
>> (defcustom save-some-buffers-default-predicate nil
>>   "Default predicate for `save-some-buffers'."
>>   :group 'auto-save
>>   ;; FIXME nil should not be a valid option, let alone the default,
>>   ;; eg so that add-function can be used.
>>   :type '(choice (const :tag "Default" nil)
>>                  (const :tag "Subdirs of default directory" default-directory)
>>                  (const :tag "Project root" project-root)
>>                  function)
>
> Indeed, this was my initial implemention.
> Then I moved to the one I shared here; I found my second version
> superior for the following reason:
>
> 1. Users can restrict the buffer default directory AND still pass a
>    predicate to filter by any other thing they wish;
>    i.e. you don't need to chose either one of the other.
>
> 2. I find it cleaner having it in a separated option.

I'm not sure if there is a need for adding
another dimension with another option.

But if yes, then still shouldn't a new option
provide a wider choice of restricting predicates
like above?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Mon, 08 Mar 2021 17:37:02 GMT) Full text and rfc822 format available.

Message #23 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: 46374 <at> debbugs.gnu.org, stefan monnier <monnier <at> iro.umontreal.ca>,
 Quách Mỹ Uyên Nhi <uyennhi.qm <at> gmail.com>
Subject: Re: bug#46374: 28.0.50; Ask me to save buffers only if they are
 under callers dir
Date: Mon, 08 Mar 2021 19:28:00 +0200
>>>> Why not simply add a new option to the existing variable
>>>> save-some-buffers-default-predicate?  For example:
>>>>
>>>> (defcustom save-some-buffers-default-predicate nil
>>>>   "Default predicate for `save-some-buffers'."
>>>>   :group 'auto-save
>>>>   ;; FIXME nil should not be a valid option, let alone the default,
>>>>   ;; eg so that add-function can be used.
>>>>   :type '(choice (const :tag "Default" nil)
>>>>                  (const :tag "Subdirs of default directory" default-directory)
>>>>                  (const :tag "Project root" project-root)
>>>>                  function)
>>>
>>> Indeed, this was my initial implemention.
>>> Then I moved to the one I shared here; I found my second version
>>> superior for the following reason:
>>>
>>> 1. Users can restrict the buffer default directory AND still pass a
>>>    predicate to filter by any other thing they wish;
>>>    i.e. you don't need to chose either one of the other.
>>>
>>> 2. I find it cleaner having it in a separated option.
>>
>> I'm not sure if there is a need for adding
>> another dimension with another option.
>
> This is  explained in my point 1. above.
> I might want to restrict to the caller subdir and still filter with
> a function.  You cannot get both things just with one option.
>
> For my normal worflow, this is a very important addition and I am looking
> forward to see it added in master soon.

Please consider the consequences of backward-incompatibility
of adding such a separate option.  For example, in the org package:

org-mobile-push:
      (save-some-buffers nil
			 (lambda () (memq (current-buffer) agenda-buffers)))

When a user customizes a new option to restrict saving of buffers to
the subdirectories only, this means it will skip saving agenda-buffers
since most of them usually are located outside of the current directory.
This change would have a drastic effect for external packages where
a new option can't be forced to be bound to nil.

More examples:

org-save-all-org-buffers:
  (save-some-buffers t (lambda () (derived-mode-p 'org-mode)))

etc.

This means reusing the existing save-some-buffers-default-predicate
would be still preferable that guarantees backward-compatibility.
When it's customized to a predicate to filter out non-current subdirs,
then such call '(save-some-buffers t (lambda () (derived-mode-p 'org-mode)))'
still overrides the customized value.  This is the right thing to do.

>> But if yes, then still shouldn't a new option
>> provide a wider choice of restricting predicates
>> like above?
>
> Yeah, we could made wider the new option as you've suggested:
>>>>                  (const :tag "Subdirs of default directory" default-directory)
>>>>                  (const :tag "Project root" project-root)
>
> Since Stefan mentioned `project-root` is not ready and I am not familiar
> with it, I suggest we add the option as it is, possibly with a modified
> name that fit well our future intention and a TODO comment.
> Then, me or someone else, can update the option when `project-root`
> become mature.
>
> Agreed?

Then the option could be named 'project-root-or-subdirs',
so when a command is called outside a project, it will
fall back to subdirs.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Sun, 14 Mar 2021 12:18:02 GMT) Full text and rfc822 format available.

Message #26 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Tino Calancha <tino.calancha <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 46374 <at> debbugs.gnu.org, stefan monnier <monnier <at> iro.umontreal.ca>,
 Quách Mỹ Uyên Nhi <uyennhi.qm <at> gmail.com>
Subject: Re: bug#46374: 28.0.50; Ask me to save buffers only if they are
 under callers dir
Date: Sun, 14 Mar 2021 13:17:05 +0100
Juri Linkov <juri <at> linkov.net> writes:

> This means reusing the existing save-some-buffers-default-predicate
> would be still preferable that guarantees backward-compatibility.
> When it's customized to a predicate to filter out non-current subdirs,
> then such call '(save-some-buffers t (lambda () (derived-mode-p 'org-mode)))'
> still overrides the customized value.  This is the right thing to do.

OK, back to my original implementation (i.e., adding a new option
to `save-some-buffers-default-predicate`).

I have been playing with the followig patch this morning.
- it only adds a new option 'project-root
- in case there is not a root there, then `default-directory` is taken
  (this is a requirement from the OP, that ie me :-)
- this patch doesn't interfere with the 2nd argument of `save-some-buffers'.

Please, try it:

--8<-----------------------------cut here---------------start------------->8---
diff --git a/lisp/files.el b/lisp/files.el
index dada69c145..d890e5b7b7 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -5517,7 +5517,9 @@ save-some-buffers-default-predicate
   :group 'auto-save
   ;; FIXME nil should not be a valid option, let alone the default,
   ;; eg so that add-function can be used.
-  :type '(choice (const :tag "Default" nil) function)
+  :type '(choice (const :tag "Default" nil)
+                 (const :tag "Project root" project-root)
+                 function)
   :version "26.1")
 
 (defun save-some-buffers (&optional arg pred)
@@ -5546,9 +5548,22 @@ save-some-buffers
 See `save-some-buffers-action-alist' if you want to
 change the additional actions you can take on files."
   (interactive "P")
-  (unless pred
-    (setq pred save-some-buffers-default-predicate))
-  (let* ((switched-buffer nil)
+  (let* ((project-dir (or (and (project-current) (project-root (project-current)))
+                          default-directory))
+         (effective-pred
+          (or pred
+              (if (eq 'project-root save-some-buffers-default-predicate)
+                  (lambda () (file-in-directory-p default-directory project-dir))
+                save-some-buffers-default-predicate)))
+         (switched-buffer nil)
+         (non-visiting-buffers-ok (not (null pred)))
+         (buffer-name-matches-filename-p
+          (lambda (buffer)
+            "Return non-nil if BUFFER name is similar to its file name."
+            (let ((file-basename (file-name-nondirectory (buffer-file-name buffer))))
+              (or (equal (buffer-name buffer) file-basename)
+                  (string-match-p (format "\\<%s<[^>]*>\\'" (regexp-quote file-basename))
+                                  (buffer-name buffer))))))
          (save-some-buffers--switch-window-callback
           (lambda (buffer)
             (setq switched-buffer buffer)))
@@ -5578,36 +5593,19 @@ save-some-buffers
                          (buffer-file-name buffer)
                          (with-current-buffer buffer
                            (or (eq buffer-offer-save 'always)
-                               (and pred buffer-offer-save
-                                    (> (buffer-size) 0)))))
-                        (or (not (functionp pred))
-                            (with-current-buffer buffer (funcall pred)))
+                               (and non-visiting-buffers-ok buffer-offer-save (> (buffer-size) 0)))))
+                        (or (not (functionp effective-pred))
+                            (with-current-buffer buffer (funcall effective-pred)))
                         (if arg
                             t
                           (setq queried t)
-                          (if (buffer-file-name buffer)
-                              (if (or
-                                   (equal (buffer-name buffer)
-                                          (file-name-nondirectory
-                                           (buffer-file-name buffer)))
-                                   (string-match
-                                    (concat "\\<"
-                                            (regexp-quote
-                                             (file-name-nondirectory
-                                              (buffer-file-name buffer)))
-                                            "<[^>]*>\\'")
-                                    (buffer-name buffer)))
-                                  ;; The buffer name is similar to the
-                                  ;; file name.
-                                  (format "Save file %s? "
-                                          (buffer-file-name buffer))
-                                ;; The buffer and file names are
-                                ;; dissimilar; display both.
-                                (format "Save file %s (buffer %s)? "
-                                        (buffer-file-name buffer)
-                                        (buffer-name buffer)))
-                            ;; No file name
-                            (format "Save buffer %s? " (buffer-name buffer))))))
+                          (cond ((null (buffer-file-name buffer))
+                                 (format "Save buffer %s? " (buffer-name buffer)))
+                                ((funcall buffer-name-matches-filename-p buffer)
+                                 (format "Save file %s? " (buffer-file-name buffer)))
+                                (t (format "Save file %s (buffer %s)? "
+                                           (buffer-file-name buffer)
+                                           (buffer-name buffer)))))))
                  (lambda (buffer)
                    (with-current-buffer buffer
                      (save-buffer)))

--8<-----------------------------cut here---------------end--------------->8---




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Mon, 15 Mar 2021 17:11:03 GMT) Full text and rfc822 format available.

Message #29 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: 46374 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Quách Mỹ Uyên Nhi <uyennhi.qm <at> gmail.com>
Subject: Re: bug#46374: 28.0.50; Ask me to save buffers only if they are
 under callers dir
Date: Mon, 15 Mar 2021 19:10:02 +0200
>> This means reusing the existing save-some-buffers-default-predicate
>> would be still preferable that guarantees backward-compatibility.
>> When it's customized to a predicate to filter out non-current subdirs,
>> then such call '(save-some-buffers t (lambda () (derived-mode-p 'org-mode)))'
>> still overrides the customized value.  This is the right thing to do.
>
> OK, back to my original implementation (i.e., adding a new option
> to `save-some-buffers-default-predicate`).
>
> I have been playing with the followig patch this morning.
> - it only adds a new option 'project-root
> - in case there is not a root there, then `default-directory` is taken
>   (this is a requirement from the OP, that ie me :-)
> - this patch doesn't interfere with the 2nd argument of `save-some-buffers'.
>
> Please, try it:

Thanks, I tried it.  The only problem is that the value 'project-root'
is hard-coded.  Would it be possible to create a real function from it?
This will require giving a proper prefix to it, indeed, such as e.g.
'save-some-buffers-project-root'.  But this will allow greater customization.
For example, everyone could copy its default implementation to the init file
and modify its logic to check separately project-root from subdirs, etc.
Also it will remove unnecessary details of getting the project root
from the implementation of 'save-some-buffers', and to avoid the need
to preload project.el.

Is it a problem that such separate function can't get
information about the original default-directory when
called as: (with-current-buffer buffer (funcall pred))?

Actually, this is the shortcoming of the old design of
save-some-buffers-default-predicate.  Is it possible to add
a new arg (with an original dir) to the predicate function?
Then for backward-compatibility before calling it, the function arity
could be checked, then called with one arg when it's accepted
by the function, otherwise call without args.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Tue, 16 Mar 2021 17:59:01 GMT) Full text and rfc822 format available.

Message #32 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: 46374 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Quách Mỹ Uyên Nhi <uyennhi.qm <at> gmail.com>
Subject: Re: bug#46374: 28.0.50; Ask me to save buffers only if they are
 under callers dir
Date: Tue, 16 Mar 2021 19:49:27 +0200
>> OK, back to my original implementation (i.e., adding a new option
>> to `save-some-buffers-default-predicate`).
>
> Is it a problem that such separate function can't get
> information about the original default-directory when
> called as: (with-current-buffer buffer (funcall pred))?
>
> Actually, this is the shortcoming of the old design of
> save-some-buffers-default-predicate.  Is it possible to add
> a new arg (with an original dir) to the predicate function?
> Then for backward-compatibility before calling it, the function arity
> could be checked, then called with one arg when it's accepted
> by the function, otherwise call without args.

I meant to replace in 'save-some-buffers'

  (with-current-buffer buffer (funcall pred))

with

  (let ((one-arg (equal (func-arity pred) '(1 . 1))))
    (dolist (buffer (buffer-list))
      (if one-arg
          (funcall pred buffer)
        (with-current-buffer buffer (funcall pred))))

Then 'default-directory' of the original buffer will be preserved
inside the pred function, so it will be possible to call the pred function
with one arg 'buffer' and check whether the buffer is under the original
'default-directory' with

  (file-in-directory-p (buffer-local-value 'default-directory buffer)
                       default-directory)




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Tue, 16 Mar 2021 22:55:01 GMT) Full text and rfc822 format available.

Message #35 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Juri Linkov <juri <at> linkov.net>
Cc: 46374 <at> debbugs.gnu.org,
 Quách Mỹ Uyên Nhi <uyennhi.qm <at> gmail.com>,
 Tino Calancha <tino.calancha <at> gmail.com>
Subject: Re: bug#46374: 28.0.50; Ask me to save buffers only if they are
 under callers dir
Date: Tue, 16 Mar 2021 18:54:12 -0400
>   (let ((one-arg (equal (func-arity pred) '(1 . 1))))

Please don't.  This will break down as soon as you set the function
with `add-function`.
[ I wish people could forget that `func-arity` exists.  Its use is
  a bug-in-the-waiting in most cases.  ]

>     (dolist (buffer (buffer-list))
>       (if one-arg
>           (funcall pred buffer)
>         (with-current-buffer buffer (funcall pred))))
>
> Then 'default-directory' of the original buffer will be preserved
> inside the pred function, so it will be possible to call the pred function
> with one arg 'buffer' and check whether the buffer is under the original
> 'default-directory' with
>
>   (file-in-directory-p (buffer-local-value 'default-directory buffer)
>                        default-directory)

How 'bout using something like `isearch-search-fun-function`:
i.e. *compute* the predicate by calling
save-some-buffers-predicate-function, so this function can return
a predicate that remembers the default-dir of the original buffer (or
any other aspect relevant to the state from which we started the
save-some-buffers).


        Stefan





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Tue, 16 Mar 2021 23:38:01 GMT) Full text and rfc822 format available.

Message #38 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>, Juri Linkov <juri <at> linkov.net>
Cc: "46374 <at> debbugs.gnu.org" <46374 <at> debbugs.gnu.org>,
 Quách Mỹ Uyên Nhi <uyennhi.qm <at> gmail.com>,
 Tino Calancha <tino.calancha <at> gmail.com>
Subject: RE: [External] : bug#46374: 28.0.50; Ask me to save buffers only if
 they are under callers dir
Date: Tue, 16 Mar 2021 23:37:30 +0000
> Please don't.  This will break down as soon as you
> set the function with `add-function`.
>
> [ I wish people could forget that `func-arity` exists.  
>   Its use is a bug-in-the-waiting in most cases. ]

Yeah.  But maybe Emacs Lisp can have a slightly better
function that does something like what it tries to do?

This was introduced in Emacs 26, IIUC.  It's not like
it's old cruft.  (Maybe new. ;-))

Here's what some have come up with for Common Lisp:

https://stackoverflow.com/q/15465138/729907

As long as `func-arity' exists, how about documenting
particular cases where it can likely cause trouble, or
cases where it's less likely to cause trouble, to give
users some guidance (beyond "Please don't use it")?

If it's problematic when used with things like
`add-function', is there something that can be said
about particular uses that are more or less likely to
cause trouble?  IOW, again, can we say more than just
"Don't use it"?

I'm guessing that many use cases of `func-arity' try
to sanity-test things like whether an unknown function
can accept zero args (none required), or can accept at
least one arg, etc.  If such tests are conservative
they can sometimes be more help than just testing
whether an object is `functionp'.

IOW, depend on the `func-arity' return value only in
a conservative direction; don't count on it as any
kind of guarantee that you _can_ do something, but use
it as a check that you might not want to bother to try
to do it.

An alternative for at least some use cases is to try
funcalling a function with some (possibly empty) arg
list, and see whether that raises an error.  But
sometimes that might not be what you really want to do.

Clearly, functions are essentially opaque, as data,
at least in our environment.  Still, some things can
be done, even if limited and tentative.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Wed, 17 Mar 2021 17:25:02 GMT) Full text and rfc822 format available.

Message #41 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 46374 <at> debbugs.gnu.org,
 Quách Mỹ Uyên Nhi <uyennhi.qm <at> gmail.com>,
 Tino Calancha <tino.calancha <at> gmail.com>
Subject: Re: bug#46374: 28.0.50; Ask me to save buffers only if they are
 under callers dir
Date: Wed, 17 Mar 2021 19:10:15 +0200
> How 'bout using something like `isearch-search-fun-function`:
> i.e. *compute* the predicate by calling
> save-some-buffers-predicate-function, so this function can return
> a predicate that remembers the default-dir of the original buffer (or
> any other aspect relevant to the state from which we started the
> save-some-buffers).

This should work.  Then in save-some-buffers it's possible
to add after the existing 2 lines:

  (unless pred
    (setq pred save-some-buffers-default-predicate))

only 3 additional lines:

  (let ((pred-fun (and (functionp pred) (funcall pred))))
    (when (functionp pred-fun)
      (setq pred pred-fun)))

Then a pred function could contain something like:

  (lambda ()
    (let ((project-dir (or (project-root (project-current)) default-directory)))
      (lambda () (file-in-directory-p default-directory project-dir))))




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Wed, 17 Mar 2021 17:26:02 GMT) Full text and rfc822 format available.

Message #44 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: "46374 <at> debbugs.gnu.org" <46374 <at> debbugs.gnu.org>,
 Tino Calancha <tino.calancha <at> gmail.com>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Quách Mỹ Uyên Nhi <uyennhi.qm <at> gmail.com>
Subject: Re: [External] : bug#46374: 28.0.50; Ask me to save buffers only if
 they are under callers dir
Date: Wed, 17 Mar 2021 19:12:20 +0200
> As long as `func-arity' exists, how about documenting
> particular cases where it can likely cause trouble

Everything is already documented in (info "(elisp) What Is a Function"):

     Note that this function might return inaccurate results in some
     situations, such as the following:

        − Functions defined using ‘apply-partially’ (*note
          apply-partially: Calling Functions.).

        − Functions that are advised using ‘advice-add’ (*note Advising
          Named Functions::).

        − Functions that determine the argument list dynamically, as
          part of their code.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Sun, 21 Mar 2021 18:00:02 GMT) Full text and rfc822 format available.

Message #47 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Tino Calancha <tino.calancha <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 46374 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Quách Mỹ Uyên Nhi <uyennhi.qm <at> gmail.com>
Subject: Re: bug#46374: 28.0.50; Ask me to save buffers only if they are
 under callers dir
Date: Sun, 21 Mar 2021 18:59:04 +0100
Juri Linkov <juri <at> linkov.net> writes:

>> How 'bout using something like `isearch-search-fun-function`:
>> i.e. *compute* the predicate by calling
>> save-some-buffers-predicate-function, so this function can return
>> a predicate that remembers the default-dir of the original buffer (or
>> any other aspect relevant to the state from which we started the
>> save-some-buffers).
>
> This should work.  Then in save-some-buffers it's possible
> to add after the existing 2 lines:
>
>   (unless pred
>     (setq pred save-some-buffers-default-predicate))
>
> only 3 additional lines:
>
>   (let ((pred-fun (and (functionp pred) (funcall pred))))
>     (when (functionp pred-fun)
>       (setq pred pred-fun)))
>
> Then a pred function could contain something like:
>
>   (lambda ()
>     (let ((project-dir (or (project-root (project-current)) default-directory)))
>       (lambda () (file-in-directory-p default-directory project-dir))))

Thanks Juri and Stefan; it looks more general now.

I have noticed that I haven't enabled lexical-binding in my .emacs file,
which it is crucial for my use case.
To avoid surprises, I have mentioned this issue in the docstring.


--8<-----------------------------cut here---------------start------------->8---
diff --git a/lisp/files.el b/lisp/files.el
index dada69c145..e87fbe9599 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -5520,6 +5520,28 @@ save-some-buffers-default-predicate
   :type '(choice (const :tag "Default" nil) function)
   :version "26.1")
 
+(defvar save-some-buffers-fun-function (lambda () save-some-buffers-default-predicate)
+  "Overrides the behavior of `save-some-buffers-default-predicate'.
+This variable's value should be a function, which will be called
+by `save-some-buffers' with no arguments to override the default predicate.
+This allow you to capture variables in the environment of `save-some-buffers',
+and use them to decide which buffers must be saved.
+For instance, the following expression restricts to save only buffers inside
+the project from where `save-some-buffers' is invoked, or under the
+caller's `default-directory' if no project is found:
+
+\(lambda ()
+  (let ((project-dir
+         (or (and (project-current) (project-root (project-current)))
+             default-directory)))
+    (lambda () (file-in-directory-p default-directory project-dir))))
+
+Note that, the example above requires that you evaluate this expression
+in a file or buffer with lexical binding enabled.")
+
+(defun save-some-buffers-fun ()
+  (funcall save-some-buffers-fun-function))
+
 (defun save-some-buffers (&optional arg pred)
   "Save some modified file-visiting buffers.  Asks user about each one.
 You can answer `y' or SPC to save, `n' or DEL not to save, `C-r'
@@ -5547,8 +5569,17 @@ save-some-buffers
 change the additional actions you can take on files."
   (interactive "P")
   (unless pred
-    (setq pred save-some-buffers-default-predicate))
+    (setq pred (save-some-buffers-fun)))
+

--8<-----------------------------cut here---------------end--------------->8---




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Sun, 21 Mar 2021 20:45:02 GMT) Full text and rfc822 format available.

Message #50 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: 46374 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Quách Mỹ Uyên Nhi <uyennhi.qm <at> gmail.com>
Subject: Re: bug#46374: 28.0.50; Ask me to save buffers only if they are
 under callers dir
Date: Sun, 21 Mar 2021 22:10:04 +0200
> I have noticed that I haven't enabled lexical-binding in my .emacs file,
> which it is crucial for my use case.
> To avoid surprises, I have mentioned this issue in the docstring.

Unfortunately, your patch is truncated.  Could you send a complete patch?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Sun, 18 Apr 2021 14:28:01 GMT) Full text and rfc822 format available.

Message #53 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Tino Calancha <tino.calancha <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 46374 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Quách Mỹ Uyên Nhi <uyennhi.qm <at> gmail.com>
Subject: Re: bug#46374: 28.0.50; Ask me to save buffers only if they are
 under callers dir
Date: Sun, 18 Apr 2021 16:27:33 +0200
Juri Linkov <juri <at> linkov.net> writes:

>> I have noticed that I haven't enabled lexical-binding in my .emacs file,
>> which it is crucial for my use case.
>> To avoid surprises, I have mentioned this issue in the docstring.
>
> Unfortunately, your patch is truncated.  Could you send a complete patch?

Here is:

--8<-----------------------------cut here---------------start------------->8---
commit 73b2ff479ab1f9af399e6d09ceff85bb8eaa66b0
Author: Tino Calancha <ccalancha <at> suse.com>
Date:   Sun Apr 18 16:14:13 2021 +0200

    patch for Bug#46374
    
    On top of commit 2822246b5d8154d0166e17ffd28a1d85b57d68aa

diff --git a/lisp/files.el b/lisp/files.el
index 7440c11a21..56ef949eeb 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -5520,6 +5520,28 @@ save-some-buffers-default-predicate
   :type '(choice (const :tag "Default" nil) function)
   :version "26.1")
 
+(defvar save-some-buffers-fun-function (lambda () save-some-buffers-default-predicate)
+  "Overrides the behavior of `save-some-buffers-default-predicate'.
+This variable's value should be a function, which will be called
+by `save-some-buffers' with no arguments to override the default predicate.
+This allow you to capture variables in the environment of `save-some-buffers',
+and use them to decide which buffers must be saved.
+For instance, the following expression restricts to save only buffers inside
+the project from where `save-some-buffers' is invoked, or under the
+caller's `default-directory' if no project is found:
+
+\(lambda ()
+  (let ((project-dir
+         (or (and (project-current) (project-root (project-current)))
+             default-directory)))
+    (lambda () (file-in-directory-p default-directory project-dir))))
+
+Note that, the example above requires that you evaluate this expression
+in a file or buffer with lexical binding enabled.")
+
+(defun save-some-buffers-fun ()
+  (funcall save-some-buffers-fun-function))
+
 (defun save-some-buffers (&optional arg pred)
   "Save some modified file-visiting buffers.  Asks user about each one.
 You can answer `y' or SPC to save, `n' or DEL not to save, `C-r'
@@ -5547,7 +5569,7 @@ save-some-buffers
 change the additional actions you can take on files."
   (interactive "P")
   (unless pred
-    (setq pred save-some-buffers-default-predicate))
+    (setq pred (save-some-buffers-fun)))
   (let* ((switched-buffer nil)
          (save-some-buffers--switch-window-callback
           (lambda (buffer)

--8<-----------------------------cut here---------------end--------------->8---




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Sat, 24 Apr 2021 22:22:02 GMT) Full text and rfc822 format available.

Message #56 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: 46374 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Quách Mỹ Uyên Nhi <uyennhi.qm <at> gmail.com>
Subject: Re: bug#46374: 28.0.50; Ask me to save buffers only if they are
 under callers dir
Date: Sun, 25 Apr 2021 01:13:13 +0300
>>> I have noticed that I haven't enabled lexical-binding in my .emacs file,
>>> which it is crucial for my use case.
>>> To avoid surprises, I have mentioned this issue in the docstring.
>>
>> Unfortunately, your patch is truncated.  Could you send a complete patch?
>
> Here is:

Thanks.  Your first patch was better in terms of customizability.
It allowed the users to easily customize the used predicate,
but its disadvantage was that it introduced a new defcustom.

Would it be possible to merge the advantage of your first patch
with the cleaner solution of your latest patch?

> +(defvar save-some-buffers-fun-function (lambda () save-some-buffers-default-predicate)

Maybe it's possible to hard-code this as the default option of the
existing defcustom save-some-buffers-default-predicate?

Currently there is such FIXME comment in save-some-buffers-default-predicate:

  ;; FIXME nil should not be a valid option, let alone the default,
  ;; eg so that add-function can be used.

Is it possible to add the default value as a function that returns nil?
This will implement FIXME, and will allow adding more options
with more predicates:

> +\(lambda ()
> +  (let ((project-dir
> +         (or (and (project-current) (project-root (project-current)))
> +             default-directory)))
> +    (lambda () (file-in-directory-p default-directory project-dir))))

Then this code could be implemented as an additional option for
save-some-buffers-default-predicate.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Wed, 28 Apr 2021 19:32:01 GMT) Full text and rfc822 format available.

Message #59 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Tino Calancha <tino.calancha <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 46374 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Quách Mỹ Uyên Nhi <uyennhi.qm <at> gmail.com>
Subject: Re: bug#46374: 28.0.50; Ask me to save buffers only if they are
 under callers dir
Date: Wed, 28 Apr 2021 21:31:25 +0200
Juri Linkov <juri <at> linkov.net> writes:


> Your first patch was better in terms of customizability.
> but its disadvantage was that it introduced a new defcustom.
>
> Would it be possible to merge the advantage of your first patch
> with the cleaner solution of your latest patch?
Good idea.

> Is it possible to add the default value as a function that returns nil?
> This will implement FIXME, and will allow adding more options
> with more predicates:

I have found a way to merge the two approaches.  Not sure if
idiomatic, but it works.

[The patch omits NEWS entry and documentation; I will add those once we
found the implementation]

--8<-----------------------------cut here---------------start------------->8---
commit 3abcf022a1fb375f15df8438ebf0cf5b67082bbc
Author: Tino Calancha <ccalancha <at> suse.com>
Date:   Wed Apr 28 21:18:28 2021 +0200

    Merge the two approaches
    
    * lisp/files (some-buffers-default-predicate):
    Redefined; store a function that builds the actual predicate
    used in save-some-buffers.
    
    On top of commit 0c7f1e2e42d6bf9f95e88c02d4e1ed9cb40693d8

diff --git a/lisp/files.el b/lisp/files.el
index 8e8fbac8dc..707c6a77bc 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -5512,19 +5512,37 @@ save-some-buffers-action-alist
 (defvar-local buffer-save-without-query nil
   "Non-nil means `save-some-buffers' should save this buffer without asking.")
 
-(defcustom save-some-buffers-default-predicate nil
-  "Default predicate for `save-some-buffers'.
+(defvar save-some-buffers-default-fun (lambda () nil))
 
-This allows you to stop `save-some-buffers' from asking
-about certain files that you'd usually rather not save.
+(defvar save-some-buffers-in-current-project-fun
+  (lambda ()
+    (let ((project-dir (or (and (project-current) (project-root (project-current)))
+                           default-directory)))
+      (lambda () (file-in-directory-p default-directory project-dir)))))
 
-This function is called (with no parameters) from the buffer to
-be saved."
+(defcustom save-some-buffers-default-predicate save-some-buffers-default-fun
+  "Generator function of the default predicate for `save-some-buffers'.
+
+It must be a function with no arguments that returns a predicate.
+This predicate is called (with no parameters) from the buffer to be
+saved.
+
+This allows you to stop `save-some-buffers' from asking about certain
+files that you'd usually rather not save.
+
+The default value builds a predicate that prompts to save any modified
+file-visiting buffer.  The second value restricts to buffers inside
+the project from where `save-some-buffers' is invoked, or under the
+caller's `default-directory' if no project is found."
   :group 'auto-save
-  ;; FIXME nil should not be a valid option, let alone the default,
-  ;; eg so that add-function can be used.
-  :type '(choice (const :tag "Default" nil) function)
-  :version "26.1")
+  :type `(choice
+          (function :tag "All buffers" :value ,save-some-buffers-default-fun)
+          (function :tag "Buffers inside current project"
+                    :value ,save-some-buffers-in-current-project-fun))
+  :version "28.1")
+
+(defun save-some-buffers-fun ()
+  (funcall save-some-buffers-default-predicate))
 
 (defun save-some-buffers (&optional arg pred)
   "Save some modified file-visiting buffers.  Asks user about each one.
@@ -5553,7 +5571,8 @@ save-some-buffers
 change the additional actions you can take on files."
   (interactive "P")
   (unless pred
-    (setq pred save-some-buffers-default-predicate))
+    (setq pred (save-some-buffers-fun)))
+
   (let* ((switched-buffer nil)
          (save-some-buffers--switch-window-callback
           (lambda (buffer)

--8<-----------------------------cut here---------------end--------------->8---




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Wed, 28 Apr 2021 20:01:01 GMT) Full text and rfc822 format available.

Message #62 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: 46374 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Quách Mỹ Uyên Nhi <uyennhi.qm <at> gmail.com>
Subject: Re: bug#46374: 28.0.50; Ask me to save buffers only if they are
 under callers dir
Date: Wed, 28 Apr 2021 22:51:51 +0300
> I have found a way to merge the two approaches.  Not sure if
> idiomatic, but it works.
>
> [The patch omits NEWS entry and documentation; I will add those once we
> found the implementation]

Thanks, now your patch is taking a good shape.  I'm testing it now.
Here is a condensed version of your patch to think about it.

#+begin_src emacs-lisp
(defvar save-some-buffers-default-fun (lambda () nil))

(defvar save-some-buffers-in-current-project-fun
  (lambda ()
    (let ((project-dir (or (and (project-current) (project-root (project-current)))
                           default-directory)))
      (lambda () (file-in-directory-p default-directory project-dir)))))

(defcustom save-some-buffers-default-predicate save-some-buffers-default-fun
  :type `(choice
          (function :tag "All buffers" :value ,save-some-buffers-default-fun)
          (function :tag "Buffers inside current project"
                    :value ,save-some-buffers-in-current-project-fun)))

(defun save-some-buffers-fun ()
  (funcall save-some-buffers-default-predicate))

(defun save-some-buffers (&optional arg pred)
  (unless pred
    (setq pred (save-some-buffers-fun)))
#+end_src

I see a problem in save-some-buffers-fun: what if
save-some-buffers-default-predicate is still customized to nil?
This value is currently allowed.  It seems it should check
if it's a function, then call it.  Also needs to check if the function
doesn't return another lambda, then return its original value.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Wed, 28 Apr 2021 20:36:01 GMT) Full text and rfc822 format available.

Message #65 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Tino Calancha <tino.calancha <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 46374 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Quách Mỹ Uyên Nhi <uyennhi.qm <at> gmail.com>
Subject: Re: bug#46374: 28.0.50; Ask me to save buffers only if they are
 under callers dir
Date: Wed, 28 Apr 2021 22:35:02 +0200
Juri Linkov <juri <at> linkov.net> writes:

> (defun save-some-buffers-fun ()
>   (funcall save-some-buffers-default-predicate))
>
> I see a problem in save-some-buffers-fun: what if
> save-some-buffers-default-predicate is still customized to nil?
I assumed we were OK with the backward incompatible change; I mean,
with requiring from now on `save-some-buffers-default-predicate'
to be a function generating the predicate.

> This value is currently allowed.  It seems it should check
> if it's a function, then call it.  Also needs to check if the function
> doesn't return another lambda, then return its original value.
We can do that; but it seems to me that the spirit of the FIXME comment
is in the line to not accept `nil' as a valid value, just functions.

BYW, thanks for testing the patch.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Thu, 29 Apr 2021 09:17:02 GMT) Full text and rfc822 format available.

Message #68 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: uyennhi.qm <at> gmail.com, 46374 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 juri <at> linkov.net
Subject: Re: bug#46374: 28.0.50;
 Ask me to save buffers only if they are under callers dir
Date: Thu, 29 Apr 2021 12:16:27 +0300
> From: Tino Calancha <tino.calancha <at> gmail.com>
> Date: Wed, 28 Apr 2021 21:31:25 +0200
> Cc: 46374 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>,
>  Quách Mỹ Uyên Nhi
>  <uyennhi.qm <at> gmail.com>
> 
> Juri Linkov <juri <at> linkov.net> writes:
> 
> +(defcustom save-some-buffers-default-predicate save-some-buffers-default-fun
> +  "Generator function of the default predicate for `save-some-buffers'.
> +
> +It must be a function with no arguments that returns a predicate.
> +This predicate is called (with no parameters) from the buffer to be
> +saved.

Reading this, I'm confused: a function that returns a predicate?  If
this is literally so, why do we need this two-step approach? why not
have the value _be_ a predicate function?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Thu, 29 Apr 2021 09:18:01 GMT) Full text and rfc822 format available.

Message #71 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: uyennhi.qm <at> gmail.com, 46374 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 juri <at> linkov.net
Subject: Re: bug#46374: 28.0.50;
 Ask me to save buffers only if they are under callers dir
Date: Thu, 29 Apr 2021 12:17:31 +0300
> From: Tino Calancha <tino.calancha <at> gmail.com>
> Date: Wed, 28 Apr 2021 22:35:02 +0200
> Cc: 46374 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>,
>  Quách Mỹ Uyên Nhi
>  <uyennhi.qm <at> gmail.com>
> 
> Juri Linkov <juri <at> linkov.net> writes:
> 
> > (defun save-some-buffers-fun ()
> >   (funcall save-some-buffers-default-predicate))
> >
> > I see a problem in save-some-buffers-fun: what if
> > save-some-buffers-default-predicate is still customized to nil?
> I assumed we were OK with the backward incompatible change; I mean,
> with requiring from now on `save-some-buffers-default-predicate'
> to be a function generating the predicate.

No, we always prefer backward-compatible changes where it is
practical.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Thu, 29 Apr 2021 16:32:02 GMT) Full text and rfc822 format available.

Message #74 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: 46374 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Quách Mỹ Uyên Nhi <uyennhi.qm <at> gmail.com>
Subject: Re: bug#46374: 28.0.50; Ask me to save buffers only if they are
 under callers dir
Date: Thu, 29 Apr 2021 19:04:49 +0300
>> I see a problem in save-some-buffers-fun: what if
>> save-some-buffers-default-predicate is still customized to nil?
> I assumed we were OK with the backward incompatible change; I mean,
> with requiring from now on `save-some-buffers-default-predicate'
> to be a function generating the predicate.
>
>> This value is currently allowed.  It seems it should check
>> if it's a function, then call it.  Also needs to check if the function
>> doesn't return another lambda, then return its original value.
> We can do that; but it seems to me that the spirit of the FIXME comment
> is in the line to not accept `nil' as a valid value, just functions.

Indeed, to remove the FIXME comment we could set the default value to
'ignore' that returns nil:

  (funcall (lambda () nil)) => nil
  (funcall 'ignore)         => nil

But also it's easy to support the old value 'nil', and also
support old functions that don't return a predicate.
You can just return the same value if it's not a function,
and also return the function if it doesn't return a predicate.
Something like this (untested):

  (defun save-some-buffers-fun ()
    (if (functionp save-some-buffers-default-predicate)
        (let ((ret (funcall save-some-buffers-default-predicate)))
          (if (functionp ret) ret save-some-buffers-default-predicate))
      save-some-buffers-default-predicate))




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Tue, 18 May 2021 17:47:02 GMT) Full text and rfc822 format available.

Message #77 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Tino Calancha <tino.calancha <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: uyennhi.qm <at> gmail.com, juri <at> linkov.net, 46374 <at> debbugs.gnu.org,
 monnier <at> iro.umontreal.ca, Tino Calancha <tino.calancha <at> gmail.com>
Subject: Re: bug#46374: 28.0.50; Ask me to save buffers only if they are
 under callers dir
Date: Tue, 18 May 2021 19:46:15 +0200 (CEST)

On Thu, 29 Apr 2021, Eli Zaretskii wrote:

>>> (defun save-some-buffers-fun ()
>>>   (funcall save-some-buffers-default-predicate))
>>>
>>> I see a problem in save-some-buffers-fun: what if
>>> save-some-buffers-default-predicate is still customized to nil?
>> I assumed we were OK with the backward incompatible change; I mean,
>> with requiring from now on `save-some-buffers-default-predicate'
>> to be a function generating the predicate.
>
> No, we always prefer backward-compatible changes where it is
> practical.
OK.  It makes a lot of sense.





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Tue, 18 May 2021 18:05:02 GMT) Full text and rfc822 format available.

Message #80 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Tino Calancha <tino.calancha <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: uyennhi.qm <at> gmail.com, juri <at> linkov.net, 46374 <at> debbugs.gnu.org,
 monnier <at> iro.umontreal.ca, Tino Calancha <tino.calancha <at> gmail.com>
Subject: Re: bug#46374: 28.0.50; Ask me to save buffers only if they are
 under callers dir
Date: Tue, 18 May 2021 20:04:24 +0200 (CEST)
[Message part 1 (text/plain, inline)]

On Thu, 29 Apr 2021, Eli Zaretskii wrote:

>> From: Tino Calancha <tino.calancha <at> gmail.com>
>> Date: Wed, 28 Apr 2021 21:31:25 +0200
>> Cc: 46374 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>,
>>  Quách Mỹ Uyên Nhi
>>  <uyennhi.qm <at> gmail.com>
>>
>> Juri Linkov <juri <at> linkov.net> writes:
>>
>> +(defcustom save-some-buffers-default-predicate save-some-buffers-default-fun
>> +  "Generator function of the default predicate for `save-some-buffers'.
>> +
>> +It must be a function with no arguments that returns a predicate.
>> +This predicate is called (with no parameters) from the buffer to be
>> +saved.
>
> Reading this, I'm confused: a function that returns a predicate?  If
> this is literally so, why do we need this two-step approach? why not
> have the value _be_ a predicate function?

The point of this two-step approach is to get access to the caller 
environment:

- We have a function, generate-foo, that builds a predicate foo.
- By calling generate-foo inside same-some-buffers, we build a foo with
  access to the enviroment, i.e. foo is a closure.

That makes the trick to restrict the prompt to saving buffers only inside 
the current project.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Tue, 18 May 2021 18:24:01 GMT) Full text and rfc822 format available.

Message #83 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: uyennhi.qm <at> gmail.com, 46374 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 juri <at> linkov.net
Subject: Re: bug#46374: 28.0.50; Ask me to save buffers only if they are
 under callers dir
Date: Tue, 18 May 2021 21:23:37 +0300
> From: Tino Calancha <tino.calancha <at> gmail.com>
> Date: Tue, 18 May 2021 20:04:24 +0200 (CEST)
> cc: Tino Calancha <tino.calancha <at> gmail.com>, juri <at> linkov.net, 
>     46374 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca, uyennhi.qm <at> gmail.com
> 
> On Thu, 29 Apr 2021, Eli Zaretskii wrote:
> 
> >> From: Tino Calancha <tino.calancha <at> gmail.com>
> >> Date: Wed, 28 Apr 2021 21:31:25 +0200
> >> Cc: 46374 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>,
> >>  Quách Mỹ Uyên Nhi
> >>  <uyennhi.qm <at> gmail.com>
> >>
> >> Juri Linkov <juri <at> linkov.net> writes:
> >>
> >> +(defcustom save-some-buffers-default-predicate save-some-buffers-default-fun
> >> +  "Generator function of the default predicate for `save-some-buffers'.
> >> +
> >> +It must be a function with no arguments that returns a predicate.
> >> +This predicate is called (with no parameters) from the buffer to be
> >> +saved.
> >
> > Reading this, I'm confused: a function that returns a predicate?  If
> > this is literally so, why do we need this two-step approach? why not
> > have the value _be_ a predicate function?
> 
> The point of this two-step approach is to get access to the caller 
> environment:
> 
> - We have a function, generate-foo, that builds a predicate foo.
> - By calling generate-foo inside same-some-buffers, we build a foo with
>    access to the enviroment, i.e. foo is a closure.
> 
> That makes the trick to restrict the prompt to saving buffers only inside 
> the current project.

IMNSHO, this is not a very good idea for a defcustom.  What user will
be able to provide such a function willy-nilly?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Tue, 18 May 2021 18:48:02 GMT) Full text and rfc822 format available.

Message #86 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Tino Calancha <tino.calancha <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: uyennhi.qm <at> gmail.com, juri <at> linkov.net, 46374 <at> debbugs.gnu.org,
 monnier <at> iro.umontreal.ca, Tino Calancha <tino.calancha <at> gmail.com>
Subject: Re: bug#46374: 28.0.50; Ask me to save buffers only if they are
 under callers dir
Date: Tue, 18 May 2021 20:47:42 +0200 (CEST)

On Tue, 18 May 2021, Eli Zaretskii wrote:

>> - We have a function, generate-foo, that builds a predicate foo.
>> - By calling generate-foo inside same-some-buffers, we build a foo with
>>    access to the enviroment, i.e. foo is a closure.
>>
>> That makes the trick to restrict the prompt to saving buffers only inside
>> the current project.
>
> IMNSHO, this is not a very good idea for a defcustom.  What user will
> be able to provide such a function willy-nilly?

Yeah, I agree that's not easy to wrote; that's why Juri suggested me to 
provide sensible options for this defcustom.

If you check the code that Juri embeded few emails before this one, you 
see that users can control the behaviour with the customization UI.

In the future, we could extend those defaults if we find other useful 
behaviors.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Fri, 13 Aug 2021 07:15:02 GMT) Full text and rfc822 format available.

Message #89 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: 46374 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Quách Mỹ Uyên Nhi <uyennhi.qm <at> gmail.com>
Subject: Re: bug#46374: 28.0.50; Ask me to save buffers only if they are
 under callers dir
Date: Fri, 13 Aug 2021 10:11:34 +0300
tags 46374 fixed
close 46374 28.0.50
thanks

Thanks for this useful feature.  It was requested yesterday in bug#49980.
So now it's pushed to master and closed.

>> Indeed, to remove the FIXME comment we could set the default value to
>> 'ignore' that returns nil:
>>
>>  (funcall (lambda () nil)) => nil
>>  (funcall 'ignore)         => nil
>
> Opps, This is a pitfall: the second one will do exactly the opposite that
> the first one does :-)
> No problem! We can apply Bon Jovi!
>
> (funcall (lambda () nil)) => nil
> (funcall 'always)         => t
>
> [the above are equivalent; yeah, it sounds impossible, but if you read the
> code you will understand why]
>
>> But also it's easy to support the old value 'nil', and also
>> support old functions that don't return a predicate.
>> You can just return the same value if it's not a function,
>> and also return the function if it doesn't return a predicate.
>> Something like this (untested):
>>
>>  (defun save-some-buffers-fun ()
>>    (if (functionp save-some-buffers-default-predicate)
>>        (let ((ret (funcall save-some-buffers-default-predicate)))
>>          (if (functionp ret) ret save-some-buffers-default-predicate))
>>      save-some-buffers-default-predicate))
>
> Neat!
> I am going to use this version for a while in my machine, to give it more
> testing.
> Thank you.




Added tag(s) fixed. Request was from Juri Linkov <juri <at> linkov.net> to control <at> debbugs.gnu.org. (Fri, 13 Aug 2021 07:15:03 GMT) Full text and rfc822 format available.

bug marked as fixed in version 28.0.50, send any further explanations to 46374 <at> debbugs.gnu.org and Tino Calancha <tino.calancha <at> gmail.com> Request was from Juri Linkov <juri <at> linkov.net> to control <at> debbugs.gnu.org. (Fri, 13 Aug 2021 07:15:03 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Fri, 13 Aug 2021 16:12:01 GMT) Full text and rfc822 format available.

Message #96 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: 46374 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Quách Mỹ Uyên Nhi <uyennhi.qm <at> gmail.com>
Subject: Re: bug#46374: 28.0.50; Ask me to save buffers only if they are
 under callers dir
Date: Fri, 13 Aug 2021 19:08:15 +0300
> I might apply some addition changes.
> I've been months testing the feature every day (and evry happy with it);
> but I solved a small issue about 1 month ago (I didn't mentioned it here).
>
> Since I will start vacation tomorrow, I will have time to add my
> minor tweaks.
>
> I also want to add some unit tests.

You are welcome to improve this feature when you want.
Enjoy your time.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Sun, 29 Aug 2021 12:52:01 GMT) Full text and rfc822 format available.

Message #99 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Hauke Rehfeld <hauke <at> haukerehfeld.de>
To: 46374 <at> debbugs.gnu.org
Subject: Regression: erronous calls to PRED switch major-mode of unrelated
 modified buffers
Date: Sat, 28 Aug 2021 18:25:23 +0200
Hmmm, I'm pretty sure the changes to `save-some-buffers` here
regarding the predicate indirection caused a regression and/or
very unexpected behavior that causes bugs in previously working
code.

Consider

`(save-some-buffers t (lambda () (derived-mode-p 'org-mode)))'

called from an org-mode buffer. It runs through the part of
`save-some-buffers' that is trying to resolve the PRED
indirection:

https://github.com/emacs-mirror/emacs/blob/692da8c6a82f8de376a2eec9304773b3e85205f3/lisp/files.el#L5792

``` emacs-lisp
 ;; Allow `pred' to be a function that returns a predicate
 ;; with lexical bindings in its original environment
 (bug#46374).
 (let ((pred-fun (and (functionp pred) (funcall pred))))
   (when (functionp pred-fun)
     (setq pred pred-fun)))
```
which evaluates the predicate to check if it returns a function --
which `(derived-mode-p)' does, as it simply returns the major-mode
symbol on success! In this case, it would be `#'org-mode'. So then
`org-mode' is called on any unsaved buffers as a PREDICATE,
switching those buffers to org-mode.

Compare this usage in org-mode, org.el:15357:

``` emacs-lisp
(defun org-save-all-org-buffers ()
 "Save all Org buffers without user confirmation."
 (interactive)
 (message "Saving all Org buffers...")
 (save-some-buffers t (lambda () (derived-mode-p 'org-mode)))
 (when (featurep 'org-id) (org-id-locations-save))
 (message "Saving all Org buffers... done"))
```




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Sun, 29 Aug 2021 16:56:03 GMT) Full text and rfc822 format available.

Message #102 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Hauke Rehfeld <hauke <at> haukerehfeld.de>
Cc: 46374 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Tino Calancha <tino.calancha <at> gmail.com>
Subject: Re: bug#46374: Regression: erronous calls to PRED switch major-mode
 of unrelated modified buffers
Date: Sun, 29 Aug 2021 19:38:20 +0300
> Hmmm, I'm pretty sure the changes to `save-some-buffers` here
> regarding the predicate indirection caused a regression and/or
> very unexpected behavior that causes bugs in previously working
> code.

Thanks, this is a very interesting case, so Cc'ing Stefan and Tino.

> Consider
>
> `(save-some-buffers t (lambda () (derived-mode-p 'org-mode)))'
>
> called from an org-mode buffer. It runs through the part of
> `save-some-buffers' that is trying to resolve the PRED
> indirection:
>
> ``` emacs-lisp
>  ;; Allow `pred' to be a function that returns a predicate
>  ;; with lexical bindings in its original environment
>  (bug#46374).
>  (let ((pred-fun (and (functionp pred) (funcall pred))))
>    (when (functionp pred-fun)
>      (setq pred pred-fun)))
> ```
> which evaluates the predicate to check if it returns a function --
> which `(derived-mode-p)' does, as it simply returns the major-mode
> symbol on success! In this case, it would be `#'org-mode'. So then
> `org-mode' is called on any unsaved buffers as a PREDICATE,
> switching those buffers to org-mode.

Indeed, the predicate returns a function, just to add more fun:

  (funcall (lambda () (derived-mode-p 'lisp-interaction-mode)))
  => lisp-interaction-mode

This means we need to invent some ad-hoc format to distinguish between
these cases.  For example, to create a lexically-bound predicate
at the beginning, it could be called with e.g.

  (save-some-buffers t '(eval . save-some-buffers-root))

and defcustom will look like:

(defcustom save-some-buffers-default-predicate nil
  :type '(choice (const :tag "Default" nil)
                 (function :tag "Only in subdirs of root"
                           (eval . save-some-buffers-root))
                 (function :tag "Custom function"))

Quite ugly, but I see no more natural way.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Mon, 30 Aug 2021 07:42:01 GMT) Full text and rfc822 format available.

Message #105 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Hauke Rehfeld <hauke <at> haukerehfeld.de>
Cc: 46374 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Tino Calancha <tino.calancha <at> gmail.com>
Subject: Re: bug#46374: Regression: erronous calls to PRED switch major-mode
 of unrelated modified buffers
Date: Mon, 30 Aug 2021 10:28:36 +0300
> This means we need to invent some ad-hoc format to distinguish between
> these cases.  For example, to create a lexically-bound predicate
> at the beginning, it could be called with e.g.
>
>   (save-some-buffers t '(eval . save-some-buffers-root))
>
> and defcustom will look like:
>
> (defcustom save-some-buffers-default-predicate nil
>   :type '(choice (const :tag "Default" nil)
>                  (function :tag "Only in subdirs of root"
>                            (eval . save-some-buffers-root))
>                  (function :tag "Custom function"))

Or maybe simply '(save-some-buffers-root):

  (defcustom save-some-buffers-default-predicate nil
    :type '(choice (const :tag "Default" nil)
                   (function :tag "Only in subdirs of root"
                             (save-some-buffers-root))

Then the following two calls both will have exactly the same effect:

    (save-some-buffers t '(save-some-buffers-root))
    (save-some-buffers t (save-some-buffers-root))




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Mon, 30 Aug 2021 14:57:02 GMT) Full text and rfc822 format available.

Message #108 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Hauke Rehfeld <hauke <at> haukerehfeld.de>
To: Juri Linkov <juri <at> linkov.net>
Cc: 46374 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Tino Calancha <tino.calancha <at> gmail.com>
Subject: Re: bug#46374: Regression: erronous calls to PRED switch major-mode
 of unrelated modified buffers
Date: Mon, 30 Aug 2021 13:43:23 +0200
I'm way not deep enough into this issue, but how about using a
temp buffer and checking for major mode equivalent (can pred ever
reasonably be fundamental-mode?):

   ;; Allow `pred' to be a function that returns a predicate
   ;; with lexical bindings in its original environment
   (bug#46374).
   (let ((pred-fun (and (functionp pred))))
     ;; don't use the result of `pred' if it returns the current
     buffer major-mode
     (with-temp-buffer
       (let ((pred-fun (funcall pred)))
         (when (and (functionp pred-fun) (not (eq major-mode
         pred-fun)))
         (setq pred pred-fun)))))


Juri Linkov <juri <at> linkov.net> writes:

>> This means we need to invent some ad-hoc format to distinguish
>> between
>> these cases.  For example, to create a lexically-bound
>> predicate
>> at the beginning, it could be called with e.g.
>>
>>   (save-some-buffers t '(eval . save-some-buffers-root))
>>
>> and defcustom will look like:
>>
>> (defcustom save-some-buffers-default-predicate nil
>>   :type '(choice (const :tag "Default" nil)
>>                  (function :tag "Only in subdirs of root"
>>                            (eval . save-some-buffers-root))
>>                  (function :tag "Custom function"))
>
> Or maybe simply '(save-some-buffers-root):
>
>   (defcustom save-some-buffers-default-predicate nil
>     :type '(choice (const :tag "Default" nil)
>                    (function :tag "Only in subdirs of root"
>                              (save-some-buffers-root))
>
> Then the following two calls both will have exactly the same
> effect:
>
>     (save-some-buffers t '(save-some-buffers-root))
>     (save-some-buffers t (save-some-buffers-root))




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Mon, 30 Aug 2021 16:07:01 GMT) Full text and rfc822 format available.

Message #111 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Hauke Rehfeld <hauke <at> haukerehfeld.de>
To: Juri Linkov <juri <at> linkov.net>
Cc: 46374 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Tino Calancha <tino.calancha <at> gmail.com>
Subject: Re: bug#46374: Regression: erronous calls to PRED switch major-mode
 of unrelated modified buffers
Date: Mon, 30 Aug 2021 18:04:31 +0200
Sorry, that was broken when called with an empty `pred' argument.
This should work (but testing all code paths is indeed not a
simple task):

   ;; Allow `pred' to be a function that returns a predicate
   ;; with lexical bindings in its original environment
   (bug#46374).
   (when (functionp pred)
     ;; don't use the result of `pred' if it returns the current
     buffer major-mode
     (with-temp-buffer
       (let ((pred-fun (funcall pred)))
         (when (and (functionp pred-fun) (not (eq major-mode
         pred-fun)))
           (setq pred pred-fun)))))

Hauke Rehfeld <hauke <at> haukerehfeld.de> writes:

> I'm way not deep enough into this issue, but how about using a
> temp buffer and checking for major mode equivalent (can pred
> ever
> reasonably be fundamental-mode?):
>
>    ;; Allow `pred' to be a function that returns a predicate
>    ;; with lexical bindings in its original environment
>    (bug#46374).
>    (let ((pred-fun (and (functionp pred))))
>      ;; don't use the result of `pred' if it returns the current
>      buffer major-mode
>      (with-temp-buffer
>        (let ((pred-fun (funcall pred)))
>          (when (and (functionp pred-fun) (not (eq major-mode
>          pred-fun)))
>          (setq pred pred-fun)))))
>
>
> Juri Linkov <juri <at> linkov.net> writes:
>
>>> This means we need to invent some ad-hoc format to distinguish
>>> between
>>> these cases.  For example, to create a lexically-bound
>>> predicate
>>> at the beginning, it could be called with e.g.
>>>
>>>   (save-some-buffers t '(eval . save-some-buffers-root))
>>>
>>> and defcustom will look like:
>>>
>>> (defcustom save-some-buffers-default-predicate nil
>>>   :type '(choice (const :tag "Default" nil)
>>>                  (function :tag "Only in subdirs of root"
>>>                            (eval . save-some-buffers-root))
>>>                  (function :tag "Custom function"))
>>
>> Or maybe simply '(save-some-buffers-root):
>>
>>   (defcustom save-some-buffers-default-predicate nil
>>     :type '(choice (const :tag "Default" nil)
>>                    (function :tag "Only in subdirs of root"
>>                              (save-some-buffers-root))
>>
>> Then the following two calls both will have exactly the same
>> effect:
>>
>>     (save-some-buffers t '(save-some-buffers-root))
>>     (save-some-buffers t (save-some-buffers-root))




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Tue, 31 Aug 2021 07:12:02 GMT) Full text and rfc822 format available.

Message #114 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Hauke Rehfeld <hauke <at> haukerehfeld.de>
Cc: 46374 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Tino Calancha <tino.calancha <at> gmail.com>
Subject: Re: bug#46374: Regression: erronous calls to PRED switch major-mode
 of unrelated modified buffers
Date: Tue, 31 Aug 2021 10:02:42 +0300
> I'm way not deep enough into this issue, but how about using a
> temp buffer and checking for major mode equivalent (can pred ever
> reasonably be fundamental-mode?):

This assumes that only derived-mode-p can be used as the predicate,
so it will fix only this particular case until some other code
will find a more clever way to circumvent this assumption
by returning some other arbitrary function that does
more nasty things than changing the major mode.




bug No longer marked as fixed in versions 28.0.50 and reopened. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Sun, 05 Sep 2021 08:12:02 GMT) Full text and rfc822 format available.

Forcibly Merged 46374 50380. Request was from Juri Linkov <juri <at> linkov.net> to control <at> debbugs.gnu.org. (Sun, 05 Sep 2021 08:12:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Sun, 05 Sep 2021 10:10:01 GMT) Full text and rfc822 format available.

Message #121 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Tino Calancha <tino.calancha <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 46374 <at> debbugs.gnu.org, Hauke Rehfeld <hauke <at> haukerehfeld.de>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#46374: 28.0.50; Ask me to save buffers only if they are
 under callers dir
Date: Sun, 05 Sep 2021 12:09:26 +0200
Juri Linkov <juri <at> linkov.net> writes:

>> This means we need to invent some ad-hoc format to distinguish between
>> these cases.  For example, to create a lexically-bound predicate
>> at the beginning, it could be called with e.g.
>>
>>   (save-some-buffers t '(eval . save-some-buffers-root))
>>
>> and defcustom will look like:
>>
>> (defcustom save-some-buffers-default-predicate nil
>>   :type '(choice (const :tag "Default" nil)
>>                  (function :tag "Only in subdirs of root"
>>                            (eval . save-some-buffers-root))
>>                  (function :tag "Custom function"))
>
> Or maybe simply '(save-some-buffers-root):

Hi Juri!

Being able to set `save-some-buffers-root' as the value of
`save-some-buffers-default-predicate' is easy for users.

The problem I see is that it hides the real nature of `save-some-buffers-root':
- it's not a predicate (as the docstring of `save-some-buffers-default-predicate' suggests).
- it's a function generating the default predicate.

We can make the distinction (default predicate <-> func generating a default predicate)
more clear if we put the generating functions in a list.

Then, we can restrict the allowed pred-fun to the elements inside such a list.

I have played today with this quick-and-dirty patch:
How do you think?

--8<-----------------------------cut here---------------start------------->8---
diff --git a/lisp/files.el b/lisp/files.el
index 7e4bdab507..91582ec9b0 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -5731,6 +5731,13 @@ save-some-buffers-action-alist
 (defvar-local buffer-save-without-query nil
   "Non-nil means `save-some-buffers' should save this buffer without asking.")
 
+(defvar save-some-buffers-fn-generating-pred '(save-some-buffers-root)
+  "List of supported functions to generate a default predicate for `save-some-buffers'.
+Each element is a function with no arguments that returns a predicate
+suitable for `save-some-buffers'.
+You can use any of these functions as the value of
+`save-some-buffers-default-predicate'.")
+
 (defcustom save-some-buffers-default-predicate nil
   "Default predicate for `save-some-buffers'.
 
@@ -5789,7 +5796,8 @@ save-some-buffers
     (setq pred save-some-buffers-default-predicate))
   ;; Allow `pred' to be a function that returns a predicate
   ;; with lexical bindings in its original environment (bug#46374).
-  (let ((pred-fun (and (functionp pred) (funcall pred))))
+  (let ((pred-fun (and (memq pred save-some-buffers-fn-generating-pred)
+                       (funcall pred))))
     (when (functionp pred-fun)
       (setq pred pred-fun)))
   (let* ((switched-buffer nil)



--8<-----------------------------cut here---------------end--------------->8---




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Sun, 05 Sep 2021 16:42:03 GMT) Full text and rfc822 format available.

Message #124 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: 46374 <at> debbugs.gnu.org, Hauke Rehfeld <hauke <at> haukerehfeld.de>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#46374: 28.0.50; Ask me to save buffers only if they are
 under callers dir
Date: Sun, 05 Sep 2021 19:21:34 +0300
> Being able to set `save-some-buffers-root' as the value of
> `save-some-buffers-default-predicate' is easy for users.
>
> The problem I see is that it hides the real nature of `save-some-buffers-root':
> - it's not a predicate (as the docstring of `save-some-buffers-default-predicate' suggests).
> - it's a function generating the default predicate.
>
> We can make the distinction (default predicate <-> func generating a default predicate)
> more clear if we put the generating functions in a list.
>
> Then, we can restrict the allowed pred-fun to the elements inside such a list.
>
> I have played today with this quick-and-dirty patch:
> How do you think?

Nice idea, and it's better than what I proposed.
The only change that I suggest is instead of a list,
to put a special symbol on the symbol property,
for example:

  (put 'save-some-buffers-root 'higher-order-function t)
or simply
  (put 'save-some-buffers-root 'hof t)

A better name is appreciated.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Sun, 10 Oct 2021 17:41:02 GMT) Full text and rfc822 format available.

Message #127 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: 46374 <at> debbugs.gnu.org, Hauke Rehfeld <hauke <at> haukerehfeld.de>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#46374: 28.0.50; Ask me to save buffers only if they are
 under callers dir
Date: Sun, 10 Oct 2021 20:38:56 +0300
close 46374 28.0.60
thanks

>> Being able to set `save-some-buffers-root' as the value of
>> `save-some-buffers-default-predicate' is easy for users.
>>
>> The problem I see is that it hides the real nature of `save-some-buffers-root':
>> - it's not a predicate (as the docstring of `save-some-buffers-default-predicate' suggests).
>> - it's a function generating the default predicate.
>>
>> We can make the distinction (default predicate <-> func generating a default predicate)
>> more clear if we put the generating functions in a list.
>>
>> Then, we can restrict the allowed pred-fun to the elements inside such a list.
>>
>> I have played today with this quick-and-dirty patch:
>> How do you think?
>
> Nice idea, and it's better than what I proposed.
> The only change that I suggest is instead of a list,
> to put a special symbol on the symbol property,
> for example:
>
>   (put 'save-some-buffers-root 'higher-order-function t)
> or simply
>   (put 'save-some-buffers-root 'hof t)
>
> A better name is appreciated.

I can't invent a better name than 'save-some-buffers-function'.
So now pushed with this property name.




bug marked as fixed in version 28.0.60, send any further explanations to 46374 <at> debbugs.gnu.org and Tino Calancha <tino.calancha <at> gmail.com> Request was from Juri Linkov <juri <at> linkov.net> to control <at> debbugs.gnu.org. (Sun, 10 Oct 2021 17:41:03 GMT) Full text and rfc822 format available.

bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Mon, 08 Nov 2021 12:24:07 GMT) Full text and rfc822 format available.

bug unarchived. Request was from Stefan Monnier <monnier <at> iro.umontreal.ca> to control <at> debbugs.gnu.org. (Thu, 06 Jan 2022 17:11:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Thu, 06 Jan 2022 19:16:01 GMT) Full text and rfc822 format available.

Message #136 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 46374 <at> debbugs.gnu.org, Hauke Rehfeld <hauke <at> haukerehfeld.de>,
 Tino Calancha <tino.calancha <at> gmail.com>
Subject: Re: bug#46374: Regression: erronous calls to PRED switch major-mode
 of unrelated modified buffers
Date: Thu, 06 Jan 2022 21:09:47 +0200
> It's actually easy to distinguish this specific case: when the pred is
> passed directly to `save-some-buffers` we don't need to do the "call
> PRED to get the actual predicate" dance, since the caller could just as
> well do that dance before calling us if that's what they want.
>
> IOW, the test for `save-some-buffers-function` is only needed for those
> functions that come from `save-some-buffers-default-predicate`.
>
> Any objection to the patch below (which also aligns the code closer
> with the doc since the doc of `save-some-buffers` doesn't mention the
> use of the `save-some-buffers-function` property on its `pred`
> argument).

Then this change needs to be in the release branch, so users won't start
relying on the unintended case of using of the `save-some-buffers-function`
property on the `pred` argument.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Thu, 06 Jan 2022 20:17:02 GMT) Full text and rfc822 format available.

Message #139 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: 46374 <at> debbugs.gnu.org, hauke <at> haukerehfeld.de, monnier <at> iro.umontreal.ca,
 tino.calancha <at> gmail.com
Subject: Re: bug#46374: Regression: erronous calls to PRED switch major-mode of
 unrelated modified buffers
Date: Thu, 06 Jan 2022 22:17:06 +0200
> From: Juri Linkov <juri <at> linkov.net>
> Date: Thu, 06 Jan 2022 21:09:47 +0200
> Cc: 46374 <at> debbugs.gnu.org, Hauke Rehfeld <hauke <at> haukerehfeld.de>,
>  Tino Calancha <tino.calancha <at> gmail.com>
> 
> > It's actually easy to distinguish this specific case: when the pred is
> > passed directly to `save-some-buffers` we don't need to do the "call
> > PRED to get the actual predicate" dance, since the caller could just as
> > well do that dance before calling us if that's what they want.
> >
> > IOW, the test for `save-some-buffers-function` is only needed for those
> > functions that come from `save-some-buffers-default-predicate`.
> >
> > Any objection to the patch below (which also aligns the code closer
> > with the doc since the doc of `save-some-buffers` doesn't mention the
> > use of the `save-some-buffers-function` property on its `pred`
> > argument).
> 
> Then this change needs to be in the release branch, so users won't start
> relying on the unintended case of using of the `save-some-buffers-function`
> property on the `pred` argument.

I don't see the message to which you are responding, so I cannot tell
whether the patch you have in mind is appropriate for the release
branch.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Thu, 06 Jan 2022 20:30:02 GMT) Full text and rfc822 format available.

Message #142 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 46374 <at> debbugs.gnu.org, hauke <at> haukerehfeld.de, monnier <at> iro.umontreal.ca,
 tino.calancha <at> gmail.com
Subject: Re: bug#46374: Regression: erronous calls to PRED switch major-mode
 of unrelated modified buffers
Date: Thu, 06 Jan 2022 22:28:02 +0200
>> Then this change needs to be in the release branch, so users won't start
>> relying on the unintended case of using of the `save-some-buffers-function`
>> property on the `pred` argument.
>
> I don't see the message to which you are responding, so I cannot tell
> whether the patch you have in mind is appropriate for the release
> branch.

Oh, it was archived, so here is a copy:

> It's actually easy to distinguish this specific case: when the pred is
> passed directly to `save-some-buffers` we don't need to do the "call
> PRED to get the actual predicate" dance, since the caller could just as
> well do that dance before calling us if that's what they want.
>
> IOW, the test for `save-some-buffers-function` is only needed for those
> functions that come from `save-some-buffers-default-predicate`.
>
> Any objection to the patch below (which also aligns the code closer
> with the doc since the doc of `save-some-buffers` doesn't mention the
> use of the `save-some-buffers-function` property on its `pred`
> argument).
>
>
>         Stefan
>
>
> diff --git a/lisp/files.el b/lisp/files.el
> index a11786fca2c..cd43b94622e 100644
> --- a/lisp/files.el
> +++ b/lisp/files.el
> @@ -5799,7 +5799,7 @@ save-some-buffers-default-predicate
>    ;; FIXME nil should not be a valid option, let alone the default,
>    ;; eg so that add-function can be used.
>    :type '(choice (const :tag "Default" nil)
> -                 (function :tag "Only in subdirs of root"
> +                 (function :tag "Only in subdirs of current project"
>                             save-some-buffers-root)
>                   (function :tag "Custom function"))
>    :version "26.1")
> @@ -5835,21 +5835,22 @@ save-some-buffers
>  Optional second argument PRED determines which buffers are considered:
>  If PRED is nil, all the file-visiting buffers are considered.
>  If PRED is t, then certain non-file buffers will also be considered.
> -If PRED is a zero-argument function, it indicates for each buffer whether
> -to consider it or not when called with that buffer current.
> +If PRED is a function, it is called with no argument in each buffer and
> +should return non-nil if that buffer should be considered.
>  PRED defaults to the value of `save-some-buffers-default-predicate'.
>  
>  See `save-some-buffers-action-alist' if you want to
>  change the additional actions you can take on files."
>    (interactive "P")
>    (unless pred
> -    (setq pred save-some-buffers-default-predicate))
> -  ;; Allow `pred' to be a function that returns a predicate
> -  ;; with lexical bindings in its original environment (bug#46374).
> -  (when (and (symbolp pred) (get pred 'save-some-buffers-function))
> -    (let ((pred-fun (and (functionp pred) (funcall pred))))
> -      (when (functionp pred-fun)
> -        (setq pred pred-fun))))
> +    (setq pred
> +          ;; Allow `pred' to be a function that returns a predicate
> +          ;; with lexical bindings in its original environment (bug#46374).
> +          (if (and (symbolp save-some-buffers-default-predicate)
> +                   (get save-some-buffers-default-predicate
> +                        'save-some-buffers-function))
> +              (funcall save-some-buffers-default-predicate)
> +            save-some-buffers-default-predicate)))
>    (let* ((switched-buffer nil)
>           (save-some-buffers--switch-window-callback
>            (lambda (buffer)




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Fri, 07 Jan 2022 14:46:01 GMT) Full text and rfc822 format available.

Message #145 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: 46374 <at> debbugs.gnu.org, hauke <at> haukerehfeld.de, monnier <at> iro.umontreal.ca,
 tino.calancha <at> gmail.com
Subject: Re: bug#46374: Regression: erronous calls to PRED switch major-mode
 of unrelated modified buffers
Date: Fri, 07 Jan 2022 16:45:47 +0200
> From: Juri Linkov <juri <at> linkov.net>
> Cc: monnier <at> iro.umontreal.ca,  46374 <at> debbugs.gnu.org,
>   hauke <at> haukerehfeld.de,  tino.calancha <at> gmail.com
> Date: Thu, 06 Jan 2022 22:28:02 +0200
> 
> >> Then this change needs to be in the release branch, so users won't start
> >> relying on the unintended case of using of the `save-some-buffers-function`
> >> property on the `pred` argument.
> >
> > I don't see the message to which you are responding, so I cannot tell
> > whether the patch you have in mind is appropriate for the release
> > branch.
> 
> Oh, it was archived, so here is a copy:

Thanks.  This is okay for the release branch.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46374; Package emacs. (Mon, 10 Jan 2022 03:19:02 GMT) Full text and rfc822 format available.

Message #148 received at 46374 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 46374 <at> debbugs.gnu.org, hauke <at> haukerehfeld.de, tino.calancha <at> gmail.com,
 Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#46374: Regression: erronous calls to PRED switch major-mode
 of unrelated modified buffers
Date: Sun, 09 Jan 2022 22:18:36 -0500
> Thanks.  This is okay for the release branch.

Thanks, installed,


        Stefan





bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Mon, 07 Feb 2022 12:24:05 GMT) Full text and rfc822 format available.

This bug report was last modified 2 years and 72 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.