bug#67310: [PATCH] Include the project--list as history when prompting for a project

[Spencer Baugh]

Dmitry Gutov <dmitry@gutov.dev> writes:
> On 21/11/2023 13:06, Dmitry Gutov wrote:
>> On 20/11/2023 21:58, Spencer Baugh wrote:
>>> +              (completing-read "Select project: " choices nil t
>>> nil 'project--list))))
>> I wonder if this will make 'project--list' to be automatically
>> managed my savehist-mode (because of what savehist-minibuffer-hook
>> does).
>> And then the contents of this var might be restored by savehist-mode
>> (when enabled) at a time or in a way that project.el is not
>> expecting.
>
> Sorry, I sent this by accident, it was in drafts.
>
> You explained this in the patch's message. But could there be a way
> that the list of overwritten anyway? Like when the user enables
> savehist-mode mid-session (or simply after project--list is used for
> the first time), and savehist-mode reads the histories from a saved
> file, overwriting the current session's values?

Oh, good point.

> Perhaps it would be more reliable to have separate history variables
> (one for directory names, and one for project names), and construct
> their values dynamically before reading the project.

Agreed, done in this patch.

I also stopped changing the format of project--list, so the patch is
overall simpler and more compatible.

>From c42a43008657fa6a203c533dd15499765a0bcfbf Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh@janestreet.com>
Date: Tue, 21 Nov 2023 10:11:52 -0500
Subject: [PATCH] Use the project--list as history when prompting for a project

The project--list is already ordered such that the most recently used
projects are at the front.  Now we use it as the minibuffer history
when prompting for a project.

To avoid savehist from picking up project--list as a minibuffer
history variable and overriding our own persistence mechanism, we
don't pass project--list directly as a history variable, but instead
pass project--dir-history or project--name-history, dynamically-bound
to an appropriate value.  project--dir-history and
project--name-history won't be persisted since they're always unbound
at the top level; but if they are persisted anyway somehow, it won't
affect us.

If we later find a way to rely on savehist for persistence instead of
having our own mechanism, we can change the in-memory format of
project--list to be just a list of directories, and our explicit calls
to project--add-dir can be replaced by let-binding
history-delete-duplicates=t, history-length=t.

* lisp/progmodes/project.el (project--add-dir): Add.
(project-remember-project): Use project--add-dir.
(project--name-history, project-prompt-project-name)
(project--dir-history, project-prompt-project-dir): Pass a
preprocessed project--list as HIST to completing-read and call
project-add-dir afterwards.
---
 lisp/progmodes/project.el | 45 +++++++++++++++++++++++++++++----------
 1 file changed, 34 insertions(+), 11 deletions(-)

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index 95db9d0ef4c..a2fbfe2aab3 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -1721,13 +1721,12 @@ project--write-project-list
             (current-buffer)))
       (write-region nil nil filename nil 'silent))))
 
-;;;###autoload
-(defun project-remember-project (pr &optional no-write)
-  "Add project PR to the front of the project list.
+(defun project--add-dir (root &optional no-write)
+  "Add project root ROOT 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."
   (project--ensure-read-project-list)
-  (let ((dir (abbreviate-file-name (project-root pr))))
+  (let ((dir (abbreviate-file-name root)))
     (unless (equal (caar project--list) dir)
       (dolist (ent project--list)
         (when (equal dir (car ent))
@@ -1736,6 +1735,13 @@ project-remember-project
       (unless no-write
         (project--write-project-list)))))
 
+;;;###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."
+  (project--add-dir (project-root pr) no-write))
+
 (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
@@ -1757,6 +1763,8 @@ project-forget-project
   (project--remove-from-project-list
    project-root "Project `%s' removed from known projects"))
 
+(defvar project--dir-history)
+
 (defun project-prompt-project-dir ()
   "Prompt the user for a directory that is one of the known project roots.
 The project is chosen among projects known from the project list,
@@ -1769,27 +1777,38 @@ project-prompt-project-dir
           ;; completion style).
           (project--file-completion-table
            (append project--list `(,dir-choice))))
+         (project--dir-history (project-known-project-roots))
          (pr-dir ""))
     (while (equal pr-dir "")
       ;; If the user simply pressed RET, do this again until they don't.
-      (setq pr-dir (completing-read "Select project: " choices nil t)))
+      (setq pr-dir
+            (let ((history-add-new-input nil))
+              (completing-read "Select project: " choices nil t nil 
'project--dir-history))))
     (if (equal pr-dir dir-choice)
         (read-directory-name "Select directory: " default-directory nil t)
+      (project--add-dir pr-dir)
       pr-dir)))
 
+(defvar project--name-history)
+
 (defun project-prompt-project-name ()
   "Prompt the user for a project, by name, that is one of the known project 
roots.
 The project is chosen among projects known from the project list,
 see `project-list-file'.
 It's also possible to enter an arbitrary directory not in the list."
   (let* ((dir-choice "... (choose a dir)")
+         project--name-history
          (choices
           (let (ret)
-            (dolist (dir (project-known-project-roots))
+            ;; Iterate in reverse order so project--name-history is in
+            ;; the correct order.
+            (dolist (dir (reverse (project-known-project-roots)))
               ;; we filter out directories that no longer map to a project,
               ;; since they don't have a clean project-name.
-              (if-let (proj (project--find-in-directory dir))
-                  (push (cons (project-name proj) proj) ret)))
+              (when-let (proj (project--find-in-directory dir))
+                (let ((name (project-name proj)))
+                  (push name project--name-history)
+                  (push (cons name proj) ret))))
             ret))
          ;; XXX: Just using this for the category (for the substring
          ;; completion style).
@@ -1798,11 +1817,15 @@ project-prompt-project-name
          (pr-name ""))
     (while (equal pr-name "")
       ;; If the user simply pressed RET, do this again until they don't.
-      (setq pr-name (completing-read "Select project: " table nil t)))
+      (setq pr-name
+            (let ((history-add-new-input nil))
+              (completing-read "Select project: " table nil t nil 
'project--name-history))))
     (if (equal pr-name dir-choice)
         (read-directory-name "Select directory: " default-directory nil t)
-      (let ((proj (assoc pr-name choices)))
-        (if (stringp proj) proj (project-root (cdr proj)))))))
+      (let* ((proj (assoc pr-name choices))
+             (root (project-root (cdr proj))))
+        (project--add-dir root)
+        root))))
 
 ;;;###autoload
 (defun project-known-project-roots ()
-- 
2.39.3