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

Previous Next

To reply to this bug, email your comments to 67310 AT debbugs.gnu.org.

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

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

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: bug-gnu-emacs <at> gnu.org
Cc: dmitry <at> gutov.dev, eliz <at> gnu.org, juri <at> linkov.net
Subject: [PATCH] Include the project--list as history when prompting for a
 project
Date: Mon, 20 Nov 2023 14:58:32 -0500

[Message part 1 (text/plain, inline)]

Tags: patch


The following patch uses project--list as a minibuffer history variable.
Now one can more easily switch between several related projects.

Independent from ongoing discussion about how to persist project--list
in bug#66993, this should be a useful improvement on its own.

This patch takes special care to avoid relying on savehist's persistent
mechanism, since savehist now knows about project--list as a minibuffer
history variable.

This patch does change the in-memory format of project--list, but not
the on-disk format, and project-known-project-roots still works, so this
patch should be backwards compatible.

In GNU Emacs 29.1.90 (build 2, x86_64-pc-linux-gnu, X toolkit, cairo
 version 1.15.12, Xaw scroll bars) of 2023-11-20 built on
 igm-qws-u22796a
Repository revision: dd8669b14b8a2b9a6d214a9d142dd8ac604f83d2
Repository branch: emacs-29
Windowing system distributor 'The X.Org Foundation', version 11.0.12011000
System Description: Rocky Linux 8.8 (Green Obsidian)

Configured using:
 'configure --config-cache --with-x-toolkit=lucid
 --with-gif=ifavailable'

[0001-Include-the-project-list-as-history-when-prompting-f.patch (text/patch, attachment)]

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Spencer Baugh <sbaugh <at> janestreet.com>, 67310 <at> debbugs.gnu.org
Cc: eliz <at> gnu.org, juri <at> linkov.net
Subject: Re: bug#67310: [PATCH] Include the project--list as history when
 prompting for a project
Date: Tue, 21 Nov 2023 13:06:40 +0200

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.

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Spencer Baugh <sbaugh <at> janestreet.com>, 67310 <at> debbugs.gnu.org
Cc: eliz <at> gnu.org, juri <at> linkov.net
Subject: Re: bug#67310: [PATCH] Include the project--list as history when
 prompting for a project
Date: Tue, 21 Nov 2023 13:14:29 +0200

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?

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.

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

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 67310 <at> debbugs.gnu.org, eliz <at> gnu.org, juri <at> linkov.net
Subject: Re: bug#67310: [PATCH] Include the project--list as history when
 prompting for a project
Date: Tue, 21 Nov 2023 10:17:50 -0500

[Message part 1 (text/plain, inline)]

Dmitry Gutov <dmitry <at> 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.

[0001-Use-the-project-list-as-history-when-prompting-for-a.patch (text/x-patch, inline)]

From c42a43008657fa6a203c533dd15499765a0bcfbf Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh <at> 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

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: 67310 <at> debbugs.gnu.org, eliz <at> gnu.org, juri <at> linkov.net
Subject: Re: bug#67310: [PATCH] Include the project--list as history when
 prompting for a project
Date: Wed, 22 Nov 2023 03:40:28 +0200

On 21/11/2023 17:17, Spencer Baugh wrote:
>       (if (equal pr-dir dir-choice)
>           (read-directory-name "Select directory: " default-directory nil t)
> +      (project--add-dir pr-dir)
>         pr-dir)))
> ...
>       (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))))

I think in the (equal pr-dir dir-choice) case we want to add the 
directory name entered by the user into the "history" anyway, don't we?

Though perhaps there's no need to do it here: 'project-current' calls 
'project-remember-project' anyway when maybe-prompt is non-nil.

So what happens if you drop both of the above 'project--add-dir' calls?

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: 67310 <at> debbugs.gnu.org, eliz <at> gnu.org, juri <at> linkov.net
Subject: Re: bug#67310: [PATCH] Include the project--list as history when
 prompting for a project
Date: Wed, 22 Nov 2023 03:42:57 +0200

On 21/11/2023 17:17, Spencer Baugh wrote:
> I also stopped changing the format of project--list, so the patch is
> overall simpler and more compatible.

This part I didn't mind at all, actually (if not for the associated 
breakage in the project-list-file's contents).

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

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 67310 <at> debbugs.gnu.org, eliz <at> gnu.org, juri <at> linkov.net
Subject: Re: bug#67310: [PATCH] Include the project--list as history when
 prompting for a project
Date: Wed, 22 Nov 2023 11:18:58 -0500

Dmitry Gutov <dmitry <at> gutov.dev> writes:
> On 21/11/2023 17:17, Spencer Baugh wrote:
>>       (if (equal pr-dir dir-choice)
>>           (read-directory-name "Select directory: " default-directory nil t)
>> +      (project--add-dir pr-dir)
>>         pr-dir)))
>> ...
>>       (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))))
>
> I think in the (equal pr-dir dir-choice) case we want to add the
> directory name entered by the user into the "history" anyway, don't
> we?

Mmmmaybe?  That would change behavior - currently transient projects
don't go into the project--list, and with that change they would.  Do
you think they should?

I personally never use transient projects so I don't really know how
they should behave.

> Though perhaps there's no need to do it here: 'project-current' calls
> 'project-remember-project' anyway when maybe-prompt is non-nil.
>
> So what happens if you drop both of the above 'project--add-dir' calls?

project-prompter is also called from project-switch-project, which
doesn't call project-remember-project but should also update the history
IMO.

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

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 67310 <at> debbugs.gnu.org, eliz <at> gnu.org, juri <at> linkov.net
Subject: Re: bug#67310: [PATCH] Include the project--list as history when
 prompting for a project
Date: Wed, 22 Nov 2023 11:21:05 -0500

Dmitry Gutov <dmitry <at> gutov.dev> writes:
> On 21/11/2023 17:17, Spencer Baugh wrote:
>> I also stopped changing the format of project--list, so the patch is
>> overall simpler and more compatible.
>
> This part I didn't mind at all, actually (if not for the associated
> breakage in the project-list-file's contents).

Yeah but it makes the patch a fair bit harder to test in a running
Emacs, since project--list changes, and there's not much point to it
yet.  I think it's better to do it later, when it's actually bringing a
real benefit.  (allowing using savehist)

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: 67310 <at> debbugs.gnu.org, eliz <at> gnu.org, juri <at> linkov.net
Subject: Re: bug#67310: [PATCH] Include the project--list as history when
 prompting for a project
Date: Wed, 22 Nov 2023 20:44:53 +0200

On 22/11/2023 18:18, Spencer Baugh wrote:
> Dmitry Gutov <dmitry <at> gutov.dev> writes:
>> On 21/11/2023 17:17, Spencer Baugh wrote:
>>>        (if (equal pr-dir dir-choice)
>>>            (read-directory-name "Select directory: " default-directory nil t)
>>> +      (project--add-dir pr-dir)
>>>          pr-dir)))
>>> ...
>>>        (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))))
>>
>> I think in the (equal pr-dir dir-choice) case we want to add the
>> directory name entered by the user into the "history" anyway, don't
>> we?
> 
> Mmmmaybe?  That would change behavior - currently transient projects
> don't go into the project--list, and with that change they would.  Do
> you think they should?

Hmm, maybe not. Anyway, that sentence was supposed to lead into the next 
paragraph anyway.

> I personally never use transient projects so I don't really know how
> they should behave.
> 
>> Though perhaps there's no need to do it here: 'project-current' calls
>> 'project-remember-project' anyway when maybe-prompt is non-nil.
>>
>> So what happens if you drop both of the above 'project--add-dir' calls?
> 
> project-prompter is also called from project-switch-project, which
> doesn't call project-remember-project but should also update the history
> IMO.

I suppose project-switch-project could add a project-remember-project 
call as well?

It's just that until recently it only supported project-related 
commands, and those would invoke (project-current t) right away -- 
adding the just-selected root into the list.

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

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 67310 <at> debbugs.gnu.org, eliz <at> gnu.org, juri <at> linkov.net
Subject: Re: bug#67310: [PATCH] Include the project--list as history when
 prompting for a project
Date: Wed, 22 Nov 2023 18:14:56 -0500

[Message part 1 (text/plain, inline)]

Dmitry Gutov <dmitry <at> gutov.dev> writes:
> On 22/11/2023 18:18, Spencer Baugh wrote:
>> Dmitry Gutov <dmitry <at> gutov.dev> writes:
>>> On 21/11/2023 17:17, Spencer Baugh wrote:
>>>>        (if (equal pr-dir dir-choice)
>>>>            (read-directory-name "Select directory: " default-directory nil t)
>>>> +      (project--add-dir pr-dir)
>>>>          pr-dir)))
>>>> ...
>>>>        (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))))
>>>
>>> I think in the (equal pr-dir dir-choice) case we want to add the
>>> directory name entered by the user into the "history" anyway, don't
>>> we?
>> Mmmmaybe?  That would change behavior - currently transient projects
>> don't go into the project--list, and with that change they would.  Do
>> you think they should?
>
> Hmm, maybe not. Anyway, that sentence was supposed to lead into the
> next paragraph anyway.
>
>> I personally never use transient projects so I don't really know how
>> they should behave.
>> 
>>> Though perhaps there's no need to do it here: 'project-current' calls
>>> 'project-remember-project' anyway when maybe-prompt is non-nil.
>>>
>>> So what happens if you drop both of the above 'project--add-dir' calls?
>> project-prompter is also called from project-switch-project, which
>> doesn't call project-remember-project but should also update the history
>> IMO.
>
> I suppose project-switch-project could add a project-remember-project
> call as well?
>
> It's just that until recently it only supported project-related
> commands, and those would invoke (project-current t) right away --
> adding the just-selected root into the list.

Yes, that makes sense, done.  (We only have the project root directory
there, so we still need project--add-dir)

[0001-Include-the-project-list-as-history-when-prompting-f.patch (text/x-patch, inline)]

From 063fe822531d51040be53f47f3dbe35ea77f21be Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh <at> janestreet.com>
Date: Mon, 20 Nov 2023 14:38:22 -0500
Subject: [PATCH] Include 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 support this, we minorly change the in-memory format of
project--list: Instead of a list of lists, each containing a
project-root, project--list is just a list whose elements are
project-roots.  This lets us pass it directly to add-to-history.  The
persistent format (what's saved in project-list-file) remains the
same.

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.

* lisp/progmodes/project.el (project--list): Update docstring for new
format.
(project-known-project-roots, project-remember-projects-under)
(project--read-project-list, project--write-project-list)
(project-remember-project, project--remove-from-project-list): Support
new format for project--list.
(project--dir-history, project-prompt-project-dir): Pass project--list
as HIST to completing-read.
(project--name-history, project-prompt-project-name): Pass a
preprocessed project--list as HIST to completing-read.
---
 lisp/progmodes/project.el | 99 +++++++++++++++++++++++++--------------
 1 file changed, 64 insertions(+), 35 deletions(-)

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index 95db9d0ef4c..4baa76b932a 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -1678,11 +1678,16 @@ project-list-file
   :group 'project)
 
 (defvar project--list 'unset
-  "List structure containing root directories of known projects.
-With some possible metadata (to be decided).")
+  "List of root directories of known projects.
+
+This is also the minibuffer history variable for
+`project-prompt-project-dir' and `project-prompt-project-name'.")
 
 (defun project--read-project-list ()
-  "Initialize `project--list' using contents of `project-list-file'."
+  "Initialize `project--list' using contents of `project-list-file'.
+
+We expect `project-list-file' to contain a list of one-element
+lists, each containing a project root."
   (let ((filename project-list-file))
     (setq project--list
           (when (file-exists-p filename)
@@ -1691,11 +1696,11 @@ project--read-project-list
               (mapcar
                (lambda (elem)
                  (let ((name (car elem)))
-                   (list (if (file-remote-p name) name
-                           (abbreviate-file-name name)))))
+                   (if (file-remote-p name) name
+                     (abbreviate-file-name name))))
                (read (current-buffer))))))
     (unless (seq-every-p
-             (lambda (elt) (stringp (car-safe elt)))
+             (lambda (elt) (stringp elt))
              project--list)
       (warn "Contents of %s are in wrong format, resetting"
             project-list-file)
@@ -1707,16 +1712,18 @@ project--ensure-read-project-list
     (project--read-project-list)))
 
 (defun project--write-project-list ()
-  "Save `project--list' in `project-list-file'."
+  "Save `project--list' in `project-list-file'.
+
+We store `project--list' as a list of one-element lists, each
+containing a project root."
   (let ((filename project-list-file))
     (with-temp-buffer
       (insert ";;; -*- lisp-data -*-\n")
       (let ((print-length nil)
             (print-level nil))
-        (pp (mapcar (lambda (elem)
-                      (let ((name (car elem)))
-                        (list (if (file-remote-p name) name
-                                (expand-file-name name)))))
+        (pp (mapcar (lambda (name)
+                      (list (if (file-remote-p name) name
+                                (expand-file-name name))))
                     project--list)
             (current-buffer)))
       (write-region nil nil filename nil 'silent))))
@@ -1728,11 +1735,10 @@ project-remember-project
 has changed, and NO-WRITE is nil."
   (project--ensure-read-project-list)
   (let ((dir (abbreviate-file-name (project-root pr))))
-    (unless (equal (caar project--list) dir)
-      (dolist (ent project--list)
-        (when (equal dir (car ent))
-          (setq project--list (delq ent project--list))))
-      (push (list dir) project--list)
+    (unless (equal (car project--list) dir)
+      (let ((history-delete-duplicates t)
+            (history-length t))
+        (add-to-history 'project--list dir))
       (unless no-write
         (project--write-project-list)))))
 
@@ -1743,10 +1749,11 @@ project--remove-from-project-list
 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)))
+  (let ((dir (abbreviate-file-name project-root)))
+    (when (member dir project--list)
+      (setq project--list (delete dir project--list))
+      (message report-message project-root)
+      (project--write-project-list))))
 
 ;;;###autoload
 (defun project-forget-project (project-root)
@@ -1757,6 +1764,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 +1778,40 @@ project-prompt-project-dir
           ;; completion style).
           (project--file-completion-table
            (append project--list `(,dir-choice))))
+         (project--dir-history project--list)
          (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)
+      (let q((history-delete-duplicates t)
+            (history-length t))
+        (add-to-history 'project--list 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--list))
               ;; 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,17 +1820,23 @@ 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))))
+        (let ((history-delete-duplicates t)
+              (history-length t))
+          (add-to-history 'project--list root))
+        root))))
 
 ;;;###autoload
 (defun project-known-project-roots ()
   "Return the list of root directories of all known projects."
   (project--ensure-read-project-list)
-  (mapcar #'car project--list))
+  project--list)
 
 ;;;###autoload
 (defun project-execute-extended-command ()
@@ -1866,13 +1894,14 @@ project-remember-projects-under
 projects."
   (interactive "DDirectory: \nP")
   (project--ensure-read-project-list)
-  (let ((dirs (if recursive
-                  (directory-files-recursively dir "" t)
-                (directory-files dir t)))
-        (known (make-hash-table :size (* 2 (length project--list))
-                                :test #'equal))
-        (count 0))
-    (dolist (project (mapcar #'car project--list))
+  (let* ((dirs (if recursive
+                   (directory-files-recursively dir "" t)
+                 (directory-files dir t)))
+         (roots (project-known-project-roots))
+         (known (make-hash-table :size (* 2 (length roots))
+                                 :test #'equal))
+         (count 0))
+    (dolist (project roots)
       (puthash project t known))
     (dolist (subdir dirs)
       (when-let (((file-directory-p subdir))
-- 
2.39.3

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: 67310 <at> debbugs.gnu.org, eliz <at> gnu.org, juri <at> linkov.net
Subject: Re: bug#67310: [PATCH] Include the project--list as history when
 prompting for a project
Date: Thu, 23 Nov 2023 04:55:56 +0200

On 23/11/2023 01:14, Spencer Baugh wrote:
> @@ -1769,27 +1778,40 @@ project-prompt-project-dir
>             ;; completion style).
>             (project--file-completion-table
>              (append project--list `(,dir-choice))))
> +         (project--dir-history project--list)
>            (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)
> +      (let q((history-delete-duplicates t)
              ^
               typo

> +            (history-length t))
> +        (add-to-history 'project--list pr-dir))
>         pr-dir)))

Sorry, I thought we agreed that project-prompt-project-dir and 
project-prompt-project-name shouldn't add-to-history?

Because project-current calls project-remember-project already 
(including the cases when the prompter isn't used: when the project is 
auto-detected). And to cover the remaining cases, we can have 
project-switch-project call project-remember-project as well.

This way also we keep the project-prompter implementations with less 
logic inside, meaning it's a bit easier to write the next one.

More DRY, too. At least while there's no other code using 
project-prompter directly (but then we could add a helper).

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: 67310 <at> debbugs.gnu.org, dmitry <at> gutov.dev, juri <at> linkov.net
Subject: Re: bug#67310: [PATCH] Include the project--list as history when
 prompting for a project
Date: Thu, 23 Nov 2023 08:38:33 +0200

> From: Spencer Baugh <sbaugh <at> janestreet.com>
> Cc: 67310 <at> debbugs.gnu.org,  eliz <at> gnu.org,  juri <at> linkov.net
> Date: Wed, 22 Nov 2023 18:14:56 -0500
> 
>  (defvar project--list 'unset
> -  "List structure containing root directories of known projects.
> -With some possible metadata (to be decided).")
> +  "List of root directories of known projects.
> +
> +This is also the minibuffer history variable for
> +`project-prompt-project-dir' and `project-prompt-project-name'.")

Please add here cross-reference(s) to command(s) or option(s) used for
minibuffer-history handling.  IOW, don't assume that when you say
"minibuffer history variable", the reader immediately understands what
you mean and how this aspect is significant.

I also question the decision of making this variable an internal one.
I don't think any other minibuffer-history variables we have are
internal ones, and for good reasons.

> +      (let q((history-delete-duplicates t)
              ^^^
Typo?

> +            ;; Iterate in reverse order so project--name-history is in
> +            ;; the correct order.

What is the "correct" order?

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

Not sure I understand why you replaced if-let with when-let here.

> +            (let ((history-add-new-input nil))

Why this non-standard way of let-binding a variable to nil?

> +        (let ((history-delete-duplicates t)
> +              (history-length t))
> +          (add-to-history 'project--list root))

Why are you overriding the values of these two user options?

> -  (let ((dirs (if recursive
> -                  (directory-files-recursively dir "" t)
> -                (directory-files dir t)))
> -        (known (make-hash-table :size (* 2 (length project--list))
> -                                :test #'equal))
> -        (count 0))
> -    (dolist (project (mapcar #'car project--list))
> +  (let* ((dirs (if recursive
> +                   (directory-files-recursively dir "" t)
> +                 (directory-files dir t)))
> +         (roots (project-known-project-roots))
> +         (known (make-hash-table :size (* 2 (length roots))
> +                                 :test #'equal))
> +         (count 0))

Is it really necessary to use let* here?

Thanks.

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

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 67310 <at> debbugs.gnu.org, eliz <at> gnu.org, juri <at> linkov.net
Subject: Re: bug#67310: [PATCH] Include the project--list as history when
 prompting for a project
Date: Fri, 24 Nov 2023 10:50:43 -0500

[Message part 1 (text/plain, inline)]

Dmitry Gutov <dmitry <at> gutov.dev> writes:
> On 23/11/2023 01:14, Spencer Baugh wrote:
>> @@ -1769,27 +1778,40 @@ project-prompt-project-dir
>>             ;; completion style).
>>             (project--file-completion-table
>>              (append project--list `(,dir-choice))))
>> +         (project--dir-history project--list)
>>            (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)
>> +      (let q((history-delete-duplicates t)
>               ^
>                typo
>
>> +            (history-length t))
>> +        (add-to-history 'project--list pr-dir))
>>         pr-dir)))
>
> Sorry, I thought we agreed that project-prompt-project-dir and
> project-prompt-project-name shouldn't add-to-history?
>
> Because project-current calls project-remember-project already
> (including the cases when the prompter isn't used: when the project is
> auto-detected). And to cover the remaining cases, we can have
> project-switch-project call project-remember-project as well.
>
> This way also we keep the project-prompter implementations with less
> logic inside, meaning it's a bit easier to write the next one.
>
> More DRY, too. At least while there's no other code using
> project-prompter directly (but then we could add a helper).

Oops, sorry, that was just the old version of the patch.  Correct
version attached.

(Perhaps I should find a better workflow for submitting patches than
manually running format-patch and copy-pasting the resulting patch to
attach it to a subsequent email)

[0001-Use-the-project-list-as-history-when-prompting-for-a.patch (text/x-patch, inline)]

From 6b7b82be8a9a2d218a124e8205f3627d77dbb0a1 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh <at> 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.  (bug#67310)
(project-switch-project): Call project--add-dir.
---
 lisp/progmodes/project.el | 39 ++++++++++++++++++++++++++++++---------
 1 file changed, 30 insertions(+), 9 deletions(-)

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index 95db9d0ef4c..fb4f42a844c 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,37 @@ 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)
       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,7 +1816,9 @@ 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)))
@@ -2064,6 +2084,7 @@ project-switch-project
 When called in a program, it will use the project corresponding
 to directory DIR."
   (interactive (list (funcall project-prompter)))
+  (project--add-dir dir)
   (let ((command (if (symbolp project-switch-commands)
                      project-switch-commands
                    (project--switch-project-command)))
-- 
2.39.3

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Eli Zaretskii <eliz <at> gnu.org>, Spencer Baugh <sbaugh <at> janestreet.com>
Cc: 67310 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#67310: [PATCH] Include the project--list as history when
 prompting for a project
Date: Sat, 25 Nov 2023 03:54:13 +0200

I'll try to answer some of the questions that are still relevant to the 
latest patch, myself.

On 23/11/2023 08:38, Eli Zaretskii wrote:

>> +            ;; Iterate in reverse order so project--name-history is in
>> +            ;; the correct order.
> 
> What is the "correct" order?

Their order in project--list.

Iteration and construction of a new list with 'push' leads to the 
reverse order, hence the use of reversion at the beginning to counteract 
that.

>> -              (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))))
> 
> Not sure I understand why you replaced if-let with when-let here.

To reduce the amount of indentation, perhaps.

>> +            (let ((history-add-new-input nil))
> 
> Why this non-standard way of let-binding a variable to nil?

I use this myself sometimes to make the change more explicit.

Anyway, amended.

>> +        (let ((history-delete-duplicates t)
>> +              (history-length t))
>> +          (add-to-history 'project--list root))
> 
> Why are you overriding the values of these two user options?

To implement the current behavior (how additions to project--list) 
happen. I've described that behavior in one of the earlier messages here.

>> -  (let ((dirs (if recursive
>> -                  (directory-files-recursively dir "" t)
>> -                (directory-files dir t)))
>> -        (known (make-hash-table :size (* 2 (length project--list))
>> -                                :test #'equal))
>> -        (count 0))
>> -    (dolist (project (mapcar #'car project--list))
>> +  (let* ((dirs (if recursive
>> +                   (directory-files-recursively dir "" t)
>> +                 (directory-files dir t)))
>> +         (roots (project-known-project-roots))
>> +         (known (make-hash-table :size (* 2 (length roots))
>> +                                 :test #'equal))
>> +         (count 0))
> 
> Is it really necessary to use let* here?

'known' depend on 'roots'.

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: 67310 <at> debbugs.gnu.org, eliz <at> gnu.org, juri <at> linkov.net
Subject: Re: bug#67310: [PATCH] Include the project--list as history when
 prompting for a project
Date: Sat, 25 Nov 2023 04:07:39 +0200

On 24/11/2023 17:50, Spencer Baugh wrote:
> Oops, sorry, that was just the old version of the patch.  Correct
> version attached.

Thanks! Installed (after renaming project--add-dir to 
project--remember-dir).

> (Perhaps I should find a better workflow for submitting patches than
> manually running format-patch and copy-pasting the resulting patch to
> attach it to a subsequent email)

Our discussion of any potential migration to alternative "forges" has 
stalled, so there's not much I can recommend, unfortunately.

If/when you have commit access, using branches for review becomes an 
option, but it comes with its own inconveniences.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 67310 <at> debbugs.gnu.org, sbaugh <at> janestreet.com, juri <at> linkov.net
Subject: Re: bug#67310: [PATCH] Include the project--list as history when
 prompting for a project
Date: Sat, 25 Nov 2023 10:42:16 +0200

> Date: Sat, 25 Nov 2023 03:54:13 +0200
> Cc: 67310 <at> debbugs.gnu.org, juri <at> linkov.net
> From: Dmitry Gutov <dmitry <at> gutov.dev>
> 
> I'll try to answer some of the questions that are still relevant to the 
> latest patch, myself.
> 
> On 23/11/2023 08:38, Eli Zaretskii wrote:
> 
> >> +            ;; Iterate in reverse order so project--name-history is in
> >> +            ;; the correct order.
> > 
> > What is the "correct" order?
> 
> Their order in project--list.
> 
> Iteration and construction of a new list with 'push' leads to the 
> reverse order, hence the use of reversion at the beginning to counteract 
> that.

Then the comment should say

  Iterate in reverse order so project--name-history is in the same
  order as project--list.

> >> -              (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))))
> > 
> > Not sure I understand why you replaced if-let with when-let here.
> 
> To reduce the amount of indentation, perhaps.

Why is that an advantage?

I generally request and expect people not to make unnecessary changes,
since doing that makes forensics harder: you see changes which don't
change the code's semantics, and need to waste time studying such
"changes" and deciding that they are no-ops.

Please everyone keep this in mind when you make changes.

> >> +        (let ((history-delete-duplicates t)
> >> +              (history-length t))
> >> +          (add-to-history 'project--list root))
> > 
> > Why are you overriding the values of these two user options?
> 
> To implement the current behavior (how additions to project--list) 
> happen. I've described that behavior in one of the earlier messages here.

I think this is not a good idea, regardless of the reasons.  Users
have these options to control how history functionality behaves in
Emacs, and here you take away that control with no "fire escape".

As for the description you allude to above, all I found is this, which
is part of Spencer's commit log:

  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.

If this is what you mean, then I don't see how this justifies the
overriding.

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 67310 <at> debbugs.gnu.org, sbaugh <at> janestreet.com, juri <at> linkov.net
Subject: Re: bug#67310: [PATCH] Include the project--list as history when
 prompting for a project
Date: Sat, 25 Nov 2023 16:05:03 +0200

On 25/11/2023 10:42, Eli Zaretskii wrote:
>> Date: Sat, 25 Nov 2023 03:54:13 +0200
>> Cc: 67310 <at> debbugs.gnu.org, juri <at> linkov.net
>> From: Dmitry Gutov <dmitry <at> gutov.dev>
>>
>> I'll try to answer some of the questions that are still relevant to the
>> latest patch, myself.
>>
>> On 23/11/2023 08:38, Eli Zaretskii wrote:
>>
>>>> +            ;; Iterate in reverse order so project--name-history is in
>>>> +            ;; the correct order.
>>>
>>> What is the "correct" order?
>>
>> Their order in project--list.
>>
>> Iteration and construction of a new list with 'push' leads to the
>> reverse order, hence the use of reversion at the beginning to counteract
>> that.
> 
> Then the comment should say
> 
>    Iterate in reverse order so project--name-history is in the same
>    order as project--list.

Sure.

>>>> -              (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))))
>>>
>>> Not sure I understand why you replaced if-let with when-let here.
>>
>> To reduce the amount of indentation, perhaps.
> 
> Why is that an advantage?
> 
> I generally request and expect people not to make unnecessary changes,
> since doing that makes forensics harder: you see changes which don't
> change the code's semantics, and need to waste time studying such
> "changes" and deciding that they are no-ops.
> 
> Please everyone keep this in mind when you make changes.

In general we don't frown in making minor cosmetic changes in the same 
area as major meaningful changes are done.

Conversely, we do frown on cosmetic changes when nothing else is added. 
Ergo, the only time to make such minor changes is when more meaningful 
changes are done.

If the latter was not the case, we could instead prefer the more common 
tactic of separating functional and cosmetic changes.

>>>> +        (let ((history-delete-duplicates t)
>>>> +              (history-length t))
>>>> +          (add-to-history 'project--list root))
>>>
>>> Why are you overriding the values of these two user options?
>>
>> To implement the current behavior (how additions to project--list)
>> happen. I've described that behavior in one of the earlier messages here.
> 
> I think this is not a good idea, regardless of the reasons.  Users
> have these options to control how history functionality behaves in
> Emacs, and here you take away that control with no "fire escape".

Actually, never mind: the latest version of the patch doesn't use 
'add-to-history', or reference these variables.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 67310 <at> debbugs.gnu.org, sbaugh <at> janestreet.com, juri <at> linkov.net
Subject: Re: bug#67310: [PATCH] Include the project--list as history when
 prompting for a project
Date: Sat, 25 Nov 2023 16:10:09 +0200

> Date: Sat, 25 Nov 2023 16:05:03 +0200
> Cc: sbaugh <at> janestreet.com, 67310 <at> debbugs.gnu.org, juri <at> linkov.net
> From: Dmitry Gutov <dmitry <at> gutov.dev>
> 
> > I generally request and expect people not to make unnecessary changes,
> > since doing that makes forensics harder: you see changes which don't
> > change the code's semantics, and need to waste time studying such
> > "changes" and deciding that they are no-ops.
> > 
> > Please everyone keep this in mind when you make changes.
> 
> In general we don't frown in making minor cosmetic changes in the same 
> area as major meaningful changes are done.

Yes, if the changes are for the better.  In this case, they seem to be
due to someone's personal stylistic preferences, so their value is
questionable at best.

> > I think this is not a good idea, regardless of the reasons.  Users
> > have these options to control how history functionality behaves in
> > Emacs, and here you take away that control with no "fire escape".
> 
> Actually, never mind: the latest version of the patch doesn't use 
> 'add-to-history', or reference these variables.

Good.

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 67310 <at> debbugs.gnu.org, sbaugh <at> janestreet.com, juri <at> linkov.net
Subject: Re: bug#67310: [PATCH] Include the project--list as history when
 prompting for a project
Date: Sat, 25 Nov 2023 17:06:06 +0200

On 25/11/2023 16:10, Eli Zaretskii wrote:
>> Date: Sat, 25 Nov 2023 16:05:03 +0200
>> Cc:sbaugh <at> janestreet.com,67310 <at> debbugs.gnu.org,juri <at> linkov.net
>> From: Dmitry Gutov<dmitry <at> gutov.dev>
>>
>>> I generally request and expect people not to make unnecessary changes,
>>> since doing that makes forensics harder: you see changes which don't
>>> change the code's semantics, and need to waste time studying such
>>> "changes" and deciding that they are no-ops.
>>>
>>> Please everyone keep this in mind when you make changes.
>> In general we don't frown in making minor cosmetic changes in the same
>> area as major meaningful changes are done.
> Yes, if the changes are for the better.  In this case, they seem to be
> due to someone's personal stylistic preferences, so their value is
> questionable at best.

I generally allow stylistic preferences when they don't run counter to 
mine. That's better for contributors' morale, for one thing.

Anyway, I made one more change in that area, hopefully to everyone's liking.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 67310 <at> debbugs.gnu.org, sbaugh <at> janestreet.com, juri <at> linkov.net
Subject: Re: bug#67310: [PATCH] Include the project--list as history when
 prompting for a project
Date: Sat, 25 Nov 2023 17:57:10 +0200

> Date: Sat, 25 Nov 2023 17:06:06 +0200
> Cc: sbaugh <at> janestreet.com, 67310 <at> debbugs.gnu.org, juri <at> linkov.net
> From: Dmitry Gutov <dmitry <at> gutov.dev>
> 
> On 25/11/2023 16:10, Eli Zaretskii wrote:
> >> Date: Sat, 25 Nov 2023 16:05:03 +0200
> >> Cc:sbaugh <at> janestreet.com,67310 <at> debbugs.gnu.org,juri <at> linkov.net
> >> From: Dmitry Gutov<dmitry <at> gutov.dev>
> >>
> >>> I generally request and expect people not to make unnecessary changes,
> >>> since doing that makes forensics harder: you see changes which don't
> >>> change the code's semantics, and need to waste time studying such
> >>> "changes" and deciding that they are no-ops.
> >>>
> >>> Please everyone keep this in mind when you make changes.
> >> In general we don't frown in making minor cosmetic changes in the same
> >> area as major meaningful changes are done.
> > Yes, if the changes are for the better.  In this case, they seem to be
> > due to someone's personal stylistic preferences, so their value is
> > questionable at best.
> 
> I generally allow stylistic preferences when they don't run counter to 
> mine. That's better for contributors' morale, for one thing.

I suggest what I think is a better principle: the stylistic
preferences of the original author always take precedence.  This is
better both for the original author's morale (as in "something will
always be left of my contribution"), and for forensics, as I explained.

And I don't think this should hurt contributors' morale, since they
will know that their preferences in the code they introduce will be
kept for as long as the code is relevant, and not overwritten the next
day.

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 67310 <at> debbugs.gnu.org, sbaugh <at> janestreet.com, juri <at> linkov.net
Subject: Re: bug#67310: [PATCH] Include the project--list as history when
 prompting for a project
Date: Sat, 25 Nov 2023 18:35:51 +0200

On 25/11/2023 17:57, Eli Zaretskii wrote:
>> Date: Sat, 25 Nov 2023 17:06:06 +0200
>> Cc:sbaugh <at> janestreet.com,67310 <at> debbugs.gnu.org,juri <at> linkov.net
>> From: Dmitry Gutov<dmitry <at> gutov.dev>
>>
>> On 25/11/2023 16:10, Eli Zaretskii wrote:
>>>> Date: Sat, 25 Nov 2023 16:05:03 +0200
>>>> Cc:sbaugh <at> janestreet.com,67310 <at> debbugs.gnu.org,juri <at> linkov.net
>>>> From: Dmitry Gutov<dmitry <at> gutov.dev>
>>>>
>>>>> I generally request and expect people not to make unnecessary changes,
>>>>> since doing that makes forensics harder: you see changes which don't
>>>>> change the code's semantics, and need to waste time studying such
>>>>> "changes" and deciding that they are no-ops.
>>>>>
>>>>> Please everyone keep this in mind when you make changes.
>>>> In general we don't frown in making minor cosmetic changes in the same
>>>> area as major meaningful changes are done.
>>> Yes, if the changes are for the better.  In this case, they seem to be
>>> due to someone's personal stylistic preferences, so their value is
>>> questionable at best.
>> I generally allow stylistic preferences when they don't run counter to
>> mine. That's better for contributors' morale, for one thing.
> I suggest what I think is a better principle: the stylistic
> preferences of the original author always take precedence.  This is
> better both for the original author's morale (as in "something will
> always be left of my contribution"), and for forensics, as I explained.

That's also a good consideration.

In our case, the contributor and the original author were the same person.

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

From: Juri Linkov <juri <at> linkov.net>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: 67310 <at> debbugs.gnu.org, Dmitry Gutov <dmitry <at> gutov.dev>, eliz <at> gnu.org
Subject: Re: bug#67310: [PATCH] Include the project--list as history when
 prompting for a project
Date: Sat, 25 Nov 2023 19:50:42 +0200

> (Perhaps I should find a better workflow for submitting patches than
> manually running format-patch and copy-pasting the resulting patch to
> attach it to a subsequent email)

There is a new command 'vc-prepare-patch' in Emacs 29, maybe it could help.

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

From: Juri Linkov <juri <at> linkov.net>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: 67310 <at> debbugs.gnu.org, Dmitry Gutov <dmitry <at> gutov.dev>, eliz <at> gnu.org
Subject: Re: bug#67310: [PATCH] Include the project--list as history when
 prompting for a project
Date: Mon, 27 Nov 2023 19:10:22 +0200

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

This change broke the order of 'C-x p p M-n M-n ...',
so I pushed this fix:

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index f7f057396e1..a81bb63fba4 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -1808,7 +1808,7 @@ project-prompt-project-name
                          (name (project-name proj)))
                 (push name project--name-history)
                 (push (cons name proj) ret)))
-            ret))
+            (reverse ret)))
          ;; XXX: Just using this for the category (for the substring
          ;; completion style).
          (table (project--file-completion-table

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Juri Linkov <juri <at> linkov.net>, Spencer Baugh <sbaugh <at> janestreet.com>
Cc: 67310 <at> debbugs.gnu.org, eliz <at> gnu.org
Subject: Re: bug#67310: [PATCH] Include the project--list as history when
 prompting for a project
Date: Sun, 10 Dec 2023 05:04:27 +0200

Hi Juri,

On 27/11/2023 19:10, Juri Linkov wrote:
>> +            ;; 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))))
> This change broke the order of 'C-x p p M-n M-n ...',
> so I pushed this fix:
> 
> diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
> index f7f057396e1..a81bb63fba4 100644
> --- a/lisp/progmodes/project.el
> +++ b/lisp/progmodes/project.el
> @@ -1808,7 +1808,7 @@ project-prompt-project-name
>                            (name (project-name proj)))
>                   (push name project--name-history)
>                   (push (cons name proj) ret)))
> -            ret))
> +            (reverse ret)))
>            ;; XXX: Just using this for the category (for the substring
>            ;; completion style).
>            (table (project--file-completion-table

Could you remind me which behavior in 'M-n M-n' the aforementioned 
change relates to? Is this supposed to be like input history as well, or 
the contents of the completions table in a certain order?

I just tried find-file, and the future history is empty there, so I 
suppose this is something we added particularly for project-find-file.

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

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 67310 <at> debbugs.gnu.org, Spencer Baugh <sbaugh <at> janestreet.com>, eliz <at> gnu.org
Subject: Re: bug#67310: [PATCH] Include the project--list as history when
 prompting for a project
Date: Sun, 10 Dec 2023 19:43:21 +0200

>> This change broke the order of 'C-x p p M-n M-n ...',
>
> Could you remind me which behavior in 'M-n M-n' the aforementioned change
> relates to? Is this supposed to be like input history as well, or the
> contents of the completions table in a certain order?

It's inappropriate to overwrite the history with the recently visited projects.
Only user input should be added to history variables because it's actually
the history of user input.  Therefore, the remaining way to access a list
of recently visited projects is the future history with 'M-n M-n'.

> I just tried find-file, and the future history is empty there, so I suppose
> this is something we added particularly for project-find-file.

Unlike with project--list, we don't keep a list of recently visited files.
Once we conducted an experiment to add all visited files to the input file history,
even when a file was visited without reading a file name in the minibuffer,
e.g. by typing RET in Dired.  But no one liked this behavior.

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Juri Linkov <juri <at> linkov.net>
Cc: 67310 <at> debbugs.gnu.org, Spencer Baugh <sbaugh <at> janestreet.com>, eliz <at> gnu.org
Subject: Re: bug#67310: [PATCH] Include the project--list as history when
 prompting for a project
Date: Sun, 10 Dec 2023 22:32:27 +0200

On 10/12/2023 19:43, Juri Linkov wrote:
>>> This change broke the order of 'C-x p p M-n M-n ...',
>> Could you remind me which behavior in 'M-n M-n' the aforementioned change
>> relates to? Is this supposed to be like input history as well, or the
>> contents of the completions table in a certain order?
> It's inappropriate to overwrite the history with the recently visited projects.
> Only user input should be added to history variables because it's actually
> the history of user input.  Therefore, the remaining way to access a list
> of recently visited projects is the future history with 'M-n M-n'.

But... we do overwrite it now, manually constructing the value of input 
history from project--list every time.

So it seems like both "past history" and "future history" show the same 
information now. If so, it might make sense to keep only one.

>> I just tried find-file, and the future history is empty there, so I suppose
>> this is something we added particularly for project-find-file.
> Unlike with project--list, we don't keep a list of recently visited files.
> Once we conducted an experiment to add all visited files to the input file history,
> even when a file was visited without reading a file name in the minibuffer,
> e.g. by typing RET in Dired.  But no one liked this behavior.

I don't remember that experiment, but the description sounds like 
recentf. Which must have its audience (and I use it through Ido's 
"virtual buffers").

Thought the way of accessing that history (only through iteration) might 
have felt limiting in that experiment.

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

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 67310 <at> debbugs.gnu.org, Spencer Baugh <sbaugh <at> janestreet.com>, eliz <at> gnu.org
Subject: Re: bug#67310: [PATCH] Include the project--list as history when
 prompting for a project
Date: Mon, 11 Dec 2023 19:12:23 +0200

>>>> This change broke the order of 'C-x p p M-n M-n ...',
>>> Could you remind me which behavior in 'M-n M-n' the aforementioned change
>>> relates to? Is this supposed to be like input history as well, or the
>>> contents of the completions table in a certain order?
>> It's inappropriate to overwrite the history with the recently visited projects.
>> Only user input should be added to history variables because it's actually
>> the history of user input.  Therefore, the remaining way to access a list
>> of recently visited projects is the future history with 'M-n M-n'.
>
> But... we do overwrite it now, manually constructing the value of input
> history from project--list every time.
>
> So it seems like both "past history" and "future history" show the same
> information now. If so, it might make sense to keep only one.

It's usually not good to overwrite past history.  So it's better
to keep only future history.

>>> I just tried find-file, and the future history is empty there, so I suppose
>>> this is something we added particularly for project-find-file.
>> Unlike with project--list, we don't keep a list of recently visited files.
>> Once we conducted an experiment to add all visited files to the input file history,
>> even when a file was visited without reading a file name in the minibuffer,
>> e.g. by typing RET in Dired.  But no one liked this behavior.
>
> I don't remember that experiment,

It was in https://debbugs.gnu.org/12915#121

#+begin_src emacs-lisp
(add-hook 'first-change-hook 'add-file-name-to-history)
(add-hook 'find-file-hook 'add-file-name-to-history)
(defun add-file-name-to-history ()
  (when (and buffer-file-name (not buffer-read-only))
    (add-to-history 'file-name-history buffer-file-name)))
#+end_src

but this clutters the file history too much.

> but the description sounds like recentf. Which must have its audience
> (and I use it through Ido's "virtual buffers").

'recentf-list' looks like the right thing to add to future history
of 'C-x C-f', but not to its past history.  This prototype works nicely:

#+begin_src emacs-lisp
(define-advice find-file (:around (ofun &rest args) recentf-list)
  (interactive (lambda (spec)
                 (minibuffer-with-setup-hook
                     (lambda ()
                       (when (featurep 'recentf)
                         (setq-local minibuffer-default-add-function
                                     (lambda () recentf-list))))
                   (advice-eval-interactive-spec spec))))
  (apply ofun args))
#+end_src

so this could be enabled by default when recentf.el is loaded.

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Juri Linkov <juri <at> linkov.net>
Cc: 67310 <at> debbugs.gnu.org, Spencer Baugh <sbaugh <at> janestreet.com>, eliz <at> gnu.org
Subject: Re: bug#67310: [PATCH] Include the project--list as history when
 prompting for a project
Date: Tue, 12 Dec 2023 02:21:07 +0200

On 11/12/2023 19:12, Juri Linkov wrote:
>>>>> This change broke the order of 'C-x p p M-n M-n ...',
>>>> Could you remind me which behavior in 'M-n M-n' the aforementioned change
>>>> relates to? Is this supposed to be like input history as well, or the
>>>> contents of the completions table in a certain order?
>>> It's inappropriate to overwrite the history with the recently visited projects.
>>> Only user input should be added to history variables because it's actually
>>> the history of user input.  Therefore, the remaining way to access a list
>>> of recently visited projects is the future history with 'M-n M-n'.
>>
>> But... we do overwrite it now, manually constructing the value of input
>> history from project--list every time.
>>
>> So it seems like both "past history" and "future history" show the same
>> information now. If so, it might make sense to keep only one.
> 
> It's usually not good to overwrite past history.  So it's better
> to keep only future history.

I'm not sure the actual input history is useful here: in most cases, it 
will be empty or almost empty. project history is different from all 
others because we almost always detect it automatically. And also 
because the total set of projects is relatively small, for each user.

And "future history" is different for every command, including the logic 
of how it's formed. Most users are also unaware of its existence, so it 
wouldn't be a good idea to rely only on it.

>>>> I just tried find-file, and the future history is empty there, so I suppose
>>>> this is something we added particularly for project-find-file.
>>> Unlike with project--list, we don't keep a list of recently visited files.
>>> Once we conducted an experiment to add all visited files to the input file history,
>>> even when a file was visited without reading a file name in the minibuffer,
>>> e.g. by typing RET in Dired.  But no one liked this behavior.
>>
>> I don't remember that experiment,
> 
> It was in https://debbugs.gnu.org/12915#121
> 
> #+begin_src emacs-lisp
> (add-hook 'first-change-hook 'add-file-name-to-history)
> (add-hook 'find-file-hook 'add-file-name-to-history)
> (defun add-file-name-to-history ()
>    (when (and buffer-file-name (not buffer-read-only))
>      (add-to-history 'file-name-history buffer-file-name)))
> #+end_src
> 
> but this clutters the file history too much.

That makes sense.

>> but the description sounds like recentf. Which must have its audience
>> (and I use it through Ido's "virtual buffers").
> 
> 'recentf-list' looks like the right thing to add to future history
> of 'C-x C-f', but not to its past history.  This prototype works nicely:
> 
> #+begin_src emacs-lisp
> (define-advice find-file (:around (ofun &rest args) recentf-list)
>    (interactive (lambda (spec)
>                   (minibuffer-with-setup-hook
>                       (lambda ()
>                         (when (featurep 'recentf)
>                           (setq-local minibuffer-default-add-function
>                                       (lambda () recentf-list))))
>                     (advice-eval-interactive-spec spec))))
>    (apply ofun args))
> #+end_src
> 
> so this could be enabled by default when recentf.el is loaded.

Interesting. This way the difference between "forward history" and the 
regular one happens due to "exotic" ways to visit a file. E.g. from 
ido's "virtual buffers", or 'M-x recentf', to some older versions of 
project-find-file. I might actually be happy if they showed up in 
find-file's history too. Though if we're talking about buffers restored 
by desktop-read, for example, then no. The problem is messing with 
history's order.

Anyway, I'm mildly positive on your suggestion above, but it's probably 
a good idea to ask somebody else as well.

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

From: sbaugh <at> catern.com
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 67310 <at> debbugs.gnu.org, Spencer Baugh <sbaugh <at> janestreet.com>, eliz <at> gnu.org,
 Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#67310: [PATCH] Include the project--list as history when
 prompting for a project
Date: Thu, 14 Dec 2023 01:02:58 +0000 (UTC)

Dmitry Gutov <dmitry <at> gutov.dev> writes:
> On 11/12/2023 19:12, Juri Linkov wrote:
>>>>>> This change broke the order of 'C-x p p M-n M-n ...',
>>>>> Could you remind me which behavior in 'M-n M-n' the aforementioned change
>>>>> relates to? Is this supposed to be like input history as well, or the
>>>>> contents of the completions table in a certain order?
>>>> It's inappropriate to overwrite the history with the recently visited projects.
>>>> Only user input should be added to history variables because it's actually
>>>> the history of user input.  Therefore, the remaining way to access a list
>>>> of recently visited projects is the future history with 'M-n M-n'.
>>>
>>> But... we do overwrite it now, manually constructing the value of input
>>> history from project--list every time.
>>>
>>> So it seems like both "past history" and "future history" show the same
>>> information now. If so, it might make sense to keep only one.
>> It's usually not good to overwrite past history.  So it's better
>> to keep only future history.
>
> I'm not sure the actual input history is useful here: in most cases,
> it will be empty or almost empty. project history is different from
> all others because we almost always detect it automatically. And also
> because the total set of projects is relatively small, for each user.
>
> And "future history" is different for every command, including the
> logic of how it's formed. Most users are also unaware of its
> existence, so it wouldn't be a good idea to rely only on it.

Yes, I think it's okay for project history to be treated as minibuffer
history even though it's not actual input history.  It's not a huge
deal, especially since the history didn't exist before.

I worry that making M-n produce project--list and M-p produce input
history will be confusing for users.  It would be like how history and
defaults in switch-to-buffer works, which is also quite confusing.  I
personally have no intuitive grasp of when I should use history and
which I should use defaults in switch-to-buffer.

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

From: Juri Linkov <juri <at> linkov.net>
To: sbaugh <at> catern.com
Cc: 67310 <at> debbugs.gnu.org, Dmitry Gutov <dmitry <at> gutov.dev>, eliz <at> gnu.org,
 Spencer Baugh <sbaugh <at> janestreet.com>
Subject: Re: bug#67310: [PATCH] Include the project--list as history when
 prompting for a project
Date: Tue, 19 Dec 2023 19:35:28 +0200

>>>>>>> This change broke the order of 'C-x p p M-n M-n ...',
>>>>>> Could you remind me which behavior in 'M-n M-n' the aforementioned change
>>>>>> relates to? Is this supposed to be like input history as well, or the
>>>>>> contents of the completions table in a certain order?
>>>>> It's inappropriate to overwrite the history with the recently visited projects.
>>>>> Only user input should be added to history variables because it's actually
>>>>> the history of user input.  Therefore, the remaining way to access a list
>>>>> of recently visited projects is the future history with 'M-n M-n'.
>>>>
>>>> But... we do overwrite it now, manually constructing the value of input
>>>> history from project--list every time.
>>>>
>>>> So it seems like both "past history" and "future history" show the same
>>>> information now. If so, it might make sense to keep only one.
>>> It's usually not good to overwrite past history.  So it's better
>>> to keep only future history.
>>
>> I'm not sure the actual input history is useful here: in most cases,
>> it will be empty or almost empty. project history is different from
>> all others because we almost always detect it automatically. And also
>> because the total set of projects is relatively small, for each user.
>>
>> And "future history" is different for every command, including the
>> logic of how it's formed. Most users are also unaware of its
>> existence, so it wouldn't be a good idea to rely only on it.
>
> Yes, I think it's okay for project history to be treated as minibuffer
> history even though it's not actual input history.  It's not a huge
> deal, especially since the history didn't exist before.

To me the distinction between input history and project history is clear.
Especially since I added a lot of define-advice that update the most
recent project by any activity on a project: such as running 'vc-dir'
adds the current vc project dir to the top of the project list,
using Dired does the same, etc.  So I always have the most recently used
project on 'C-x p p M-n'.  Whereas I expect that 'C-x p p M-p' will get
the last project that was manually selected by the previous completion.

Previous Next

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