bug#46374: 28.0.50; Ask me to save buffers only if they are under callers dir

bug#46374: 28.0.50; Ask me to save buffers only if they are under callers dir

[Tino Calancha]

X-Debbugs-Cc: Stefan Monnier <monnier@iro.umontreal.ca>, <uyennhi.qm@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@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'),
 \f
 * 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))

bug#46374: 28.0.50; Ask me to save buffers only if they are under callers dir

[Eli Zaretskii]

> From: Tino Calancha <tino.calancha@gmail.com>
> Date: Sun, 07 Feb 2021 23:32:07 +0100
> Cc: stefan monnier <monnier@iro.umontreal.ca>, uyennhi.qm@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?

bug#46374: 28.0.50; Ask me to save buffers only if they are under callers dir

[Stefan Monnier]

> 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

bug#46374: 28.0.50; Ask me to save buffers only if they are under callers dir

[Juri Linkov]

>  @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)

bug#46374: 28.0.50; Ask me to save buffers only if they are under callers dir

[Tino Calancha]

Sorry for the late response: many meetings with investors.
Then I went on a vacation to my castle in Seychelles.


Eli Zaretskii <eliz@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@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@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.

bug#46374: 28.0.50; Ask me to save buffers only if they are under callers dir

[Juri Linkov]

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

bug#46374: 28.0.50; Ask me to save buffers only if they are under callers dir

[Juri Linkov]

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

bug#46374: 28.0.50; Ask me to save buffers only if they are under callers dir

[Tino Calancha]

Juri Linkov <juri@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---

bug#46374: 28.0.50; Ask me to save buffers only if they are under callers dir

[Juri Linkov]

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

bug#46374: 28.0.50; Ask me to save buffers only if they are under callers dir

[Juri Linkov]

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

bug#46374: 28.0.50; Ask me to save buffers only if they are under callers dir

[Stefan Monnier]

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

bug#46374: [External] : bug#46374: 28.0.50; Ask me to save buffers only if they are under callers dir

[Drew Adams]

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

bug#46374: 28.0.50; Ask me to save buffers only if they are under callers dir

[Juri Linkov]

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

bug#46374: [External] : bug#46374: 28.0.50; Ask me to save buffers only if they are under callers dir

[Juri Linkov]

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

bug#46374: 28.0.50; Ask me to save buffers only if they are under callers dir

[Tino Calancha]

Juri Linkov <juri@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---

bug#46374: 28.0.50; Ask me to save buffers only if they are under callers dir

[Juri Linkov]

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

bug#46374: 28.0.50; Ask me to save buffers only if they are under callers dir

[Tino Calancha]

Juri Linkov <juri@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@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---

bug#46374: 28.0.50; Ask me to save buffers only if they are under callers dir

[Juri Linkov]

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

bug#46374: 28.0.50; Ask me to save buffers only if they are under callers dir

[Tino Calancha]

Juri Linkov <juri@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@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---

bug#46374: 28.0.50; Ask me to save buffers only if they are under callers dir

[Juri Linkov]

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

bug#46374: 28.0.50; Ask me to save buffers only if they are under callers dir

[Tino Calancha]

Juri Linkov <juri@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.

bug#46374: 28.0.50; Ask me to save buffers only if they are under callers dir

[Eli Zaretskii]

> From: Tino Calancha <tino.calancha@gmail.com>
> Date: Wed, 28 Apr 2021 21:31:25 +0200
> Cc: 46374@debbugs.gnu.org, Stefan Monnier <monnier@iro.umontreal.ca>,
>  Quách Mỹ Uyên Nhi
>  <uyennhi.qm@gmail.com>
> 
> Juri Linkov <juri@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?

bug#46374: 28.0.50; Ask me to save buffers only if they are under callers dir

[Eli Zaretskii]

> From: Tino Calancha <tino.calancha@gmail.com>
> Date: Wed, 28 Apr 2021 22:35:02 +0200
> Cc: 46374@debbugs.gnu.org, Stefan Monnier <monnier@iro.umontreal.ca>,
>  Quách Mỹ Uyên Nhi
>  <uyennhi.qm@gmail.com>
> 
> Juri Linkov <juri@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.

bug#46374: 28.0.50; Ask me to save buffers only if they are under callers dir

[Juri Linkov]

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

bug#46374: 28.0.50; Ask me to save buffers only if they are under callers dir

[Tino Calancha]

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.

bug#46374: 28.0.50; Ask me to save buffers only if they are under callers dir

[Tino Calancha]

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



On Thu, 29 Apr 2021, Eli Zaretskii wrote:

>> From: Tino Calancha <tino.calancha@gmail.com>
>> Date: Wed, 28 Apr 2021 21:31:25 +0200
>> Cc: 46374@debbugs.gnu.org, Stefan Monnier <monnier@iro.umontreal.ca>,
>>  Quách Mỹ Uyên Nhi
>>  <uyennhi.qm@gmail.com>
>>
>> Juri Linkov <juri@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.

bug#46374: 28.0.50; Ask me to save buffers only if they are under callers dir

[Eli Zaretskii]

> From: Tino Calancha <tino.calancha@gmail.com>
> Date: Tue, 18 May 2021 20:04:24 +0200 (CEST)
> cc: Tino Calancha <tino.calancha@gmail.com>, juri@linkov.net, 
>     46374@debbugs.gnu.org, monnier@iro.umontreal.ca, uyennhi.qm@gmail.com
> 
> On Thu, 29 Apr 2021, Eli Zaretskii wrote:
> 
> >> From: Tino Calancha <tino.calancha@gmail.com>
> >> Date: Wed, 28 Apr 2021 21:31:25 +0200
> >> Cc: 46374@debbugs.gnu.org, Stefan Monnier <monnier@iro.umontreal.ca>,
> >>  Quách Mỹ Uyên Nhi
> >>  <uyennhi.qm@gmail.com>
> >>
> >> Juri Linkov <juri@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?

bug#46374: 28.0.50; Ask me to save buffers only if they are under callers dir

[Tino Calancha]

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.

bug#46374: 28.0.50; Ask me to save buffers only if they are under callers dir

[Juri Linkov]

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.

bug#46374: 28.0.50; Ask me to save buffers only if they are under callers dir

[Juri Linkov]

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

bug#46374: Regression: erronous calls to PRED switch major-mode of unrelated modified buffers

[Hauke Rehfeld]

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"))
```

bug#46374: Regression: erronous calls to PRED switch major-mode of unrelated modified buffers

[Juri Linkov]

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

bug#46374: Regression: erronous calls to PRED switch major-mode of unrelated modified buffers

[Juri Linkov]

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

bug#46374: Regression: erronous calls to PRED switch major-mode of unrelated modified buffers

[Hauke Rehfeld]

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@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))

bug#46374: Regression: erronous calls to PRED switch major-mode of unrelated modified buffers

[Hauke Rehfeld]

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@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@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))

bug#46374: Regression: erronous calls to PRED switch major-mode of unrelated modified buffers

[Juri Linkov]

> 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#46374: 28.0.50; Ask me to save buffers only if they are under callers dir

[Tino Calancha]

Juri Linkov <juri@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---

bug#46374: 28.0.50; Ask me to save buffers only if they are under callers dir

[Juri Linkov]

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

bug#46374: 28.0.50; Ask me to save buffers only if they are under callers dir

[Juri Linkov]

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#46374: Regression: erronous calls to PRED switch major-mode of unrelated modified buffers

[Juri Linkov]

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

bug#46374: Regression: erronous calls to PRED switch major-mode of unrelated modified buffers

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Date: Thu, 06 Jan 2022 21:09:47 +0200
> Cc: 46374@debbugs.gnu.org, Hauke Rehfeld <hauke@haukerehfeld.de>,
>  Tino Calancha <tino.calancha@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.

bug#46374: Regression: erronous calls to PRED switch major-mode of unrelated modified buffers

[Juri Linkov]

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

bug#46374: Regression: erronous calls to PRED switch major-mode of unrelated modified buffers

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: monnier@iro.umontreal.ca,  46374@debbugs.gnu.org,
>   hauke@haukerehfeld.de,  tino.calancha@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.

bug#46374: Regression: erronous calls to PRED switch major-mode of unrelated modified buffers

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> Thanks.  This is okay for the release branch.

Thanks, installed,


        Stefan