Subject: Re: bug#66993: [PATCH] project.el: avoid asking user about project-list-file lock

From: [email-masked]
Date: Sat, 18 Nov 2023 15:48:33 +0000 (UTC)

Dmitry Gutov <[email-masked]> writes:
> On 15/11/2023 22:49, Spencer Baugh wrote:
>> project--list is the new history variable.  Using it is mostly trivial;
>> a small bit of work is necessary in project-prompt-project-name to get
>> project-names for the history, but that worked out great.  Everything
>> else just seems to work.
>> And with savehist-mode enabled, project--list just gets stored
>> automatically without any code in project.el.
>> I can implement a backwards-compatible version if this seems like a
>> reasonable direction to go in.
>
> I haven't tried using it, but there's indeed a lot to like about this patch.
>
> What happens if savehist-mode is nil, though? Which it is by
> default. For users with this setup the project history will just
> disappear.

Indeed.

> What kind of backward compatibility did you have in mind?

I was thinking about either keeping our code for saving to
project-list-file around in some obsoleted form, or using a subset of
the savehist code to save only project--list when it's not otherwise
enabled.

But actually, maybe it's time that we just enable savehist by default.
I'll try that first - I mailed emacs-devel about it.  Then we could
delete our project-list-file code once project.el eventually starts
requiring Emacs 30.

Regardless, how about the below patch which changes the default behavior
to only write project-list-file when Emacs exits?  I think this is a
useful first step no matter what else we do, since it matches the
behavior of savehist better.

>From 1e2ca2abf98f2589db384dc56e68ed4b8e73f1a3 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <[email-masked]>
Date: Sat, 18 Nov 2023 15:43:44 +0000
Subject: [PATCH] Only save project-list-file on exit by default.

Saving on most project commands can cause conflicts between Emacs
instances.  Saving on exit only is more reliable, and it matches the
behavior of savehist better, which we might use eventually instead of
our own code.

To allow opting in to the old behavior, add a new defcustom
project-list-file-save defaulting to on-exit.

* lisp/progmodes/project.el (project-list-file-save)
(project--project-list-changed, project--kill-emacs-hook):
Add. (bug#66993)
(kill-emacs-hook): Call project--kill-emacs-hook.
(project-remember-project, project--remove-from-project-list)
(project-remember-projects-under, project-forget-projects-under): Call
project--project-list-changed.
---
 etc/NEWS                  |  5 +++++
 lisp/progmodes/project.el | 45 +++++++++++++++++++++++++++++++--------
 2 files changed, 41 insertions(+), 9 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 8324eb7da1e..bbb27183e96 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1062,6 +1062,11 @@ the current project.
 The look of the key prompt in the project switcher has been changed
 slightly.  To get the previous one, set this option to 'brackets'.
 
+*** New user option 'project-list-file-save', defaulting to on-exit.
+By default, 'project-list-file' is now only saved on exit.  To get the
+old behavior of saving whenever the project list changed, set this
+option to 'on-change' or 'on-change-and-exit'.
+
 *** 'project-try-vc' tries harder to find the responsible VCS.
 When 'project-vc-extra-root-markers' is non-nil, and causes
 subdirectory project to be detected which is not a VCS root, we now
diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index 0855f76aa13..da2cec6abb2 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -1713,6 +1713,21 @@ project--ensure-read-project-list
   (when (eq project--list 'unset)
     (project--read-project-list)))
 
+(defcustom project-list-file-save 'on-exit
+  "When to save `project-list-file'.
+
+If non-nil, the list of known projects is saved in
+`project-list-file'.
+
+If set to `on-change', the list is saved every time it changes.
+If set to `on-exit', the list is saved when Emacs exits.  If set
+to `on-change-and-exit', the list is saved in both cases."
+  :type '(choice (const :tag "Save on change" on-change)
+                 (const :tag "Save when exiting" on-exit)
+                 (const :tag "Save on change and when exiting" on-change-and-exit))
+  :group 'project
+  :version "30.1")
+
 (defun project--write-project-list ()
   "Save `project--list' in `project-list-file'."
   (let ((filename project-list-file))
@@ -1728,11 +1743,22 @@ project--write-project-list
             (current-buffer)))
       (write-region nil nil filename nil 'silent))))
 
+(defun project--project-list-changed ()
+  (when (memq project-list-file-save '(on-change on-change-and-exit))
+    (project--write-project-list)))
+
+(defun project--kill-emacs-hook ()
+  (when (memq project-list-file-save '(on-exit on-change-and-exit))
+    (project--write-project-list)))
+
+(add-hook 'kill-emacs-hook #'project--kill-emacs-hook)
+
 ;;;###autoload
 (defun project-remember-project (pr &optional no-write)
   "Add project PR to the front of the project list.
 Save the result in `project-list-file' if the list of projects
-has changed, and NO-WRITE is nil."
+has changed, `project-list-file-save' is configured to save on
+change, and NO-WRITE is nil."
   (project--ensure-read-project-list)
   (let ((dir (abbreviate-file-name (project-root pr))))
     (unless (equal (caar project--list) dir)
@@ -1741,19 +1767,20 @@ project-remember-project
           (setq project--list (delq ent project--list))))
       (push (list dir) project--list)
       (unless no-write
-        (project--write-project-list)))))
+        (project--project-list-changed)))))
 
 (defun project--remove-from-project-list (project-root report-message)
   "Remove directory PROJECT-ROOT of a missing project from the project list.
-If the directory was in the list before the removal, save the
-result in `project-list-file'.  Announce the project's removal
-from the list using REPORT-MESSAGE, which is a format string
-passed to `message' as its first argument."
+If the directory was in the list before the removal and
+`project-list-file-save' is configured to save on change, save
+the result in `project-list-file'.  Announce the project's
+removal from the list using REPORT-MESSAGE, which is a format
+string passed to `message' as its first argument."
   (project--ensure-read-project-list)
   (when-let ((ent (assoc (abbreviate-file-name project-root) project--list)))
     (setq project--list (delq ent project--list))
     (message report-message project-root)
-    (project--write-project-list)))
+    (project--project-list-changed)))
 
 ;;;###autoload
 (defun project-forget-project (project-root)
@@ -1892,7 +1919,7 @@ project-remember-projects-under
         (setq count (1+ count))))
     (if (zerop count)
         (message "No projects were found")
-      (project--write-project-list)
+      (project--project-list-changed)
       (message "%d project%s were found"
                count (if (= count 1) "" "s")))
     count))
@@ -1923,7 +1950,7 @@ project-forget-projects-under
           (setq count (1+ count)))))
     (if (zerop count)
         (message "No projects were forgotten")
-      (project--write-project-list)
+      (project--project-list-changed)
       (message "%d project%s were forgotten"
                count (if (= count 1) "" "s")))
     count))
-- 
2.42.1