* bug#67310: [PATCH] Include the project--list as history when prompting for a project
@ 2023-11-20 19:58 Spencer Baugh
2023-11-21 11:06 ` Dmitry Gutov
0 siblings, 1 reply; 30+ messages in thread
From: Spencer Baugh @ 2023-11-20 19:58 UTC (permalink / raw)
To: 67310; +Cc: dmitry, eliz, juri
[-- Attachment #1: Type: text/plain, Size: 1089 bytes --]
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'
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Include-the-project-list-as-history-when-prompting-f.patch --]
[-- Type: text/patch, Size: 9830 bytes --]
From 89a4df13ca4c678bd9915e134c078607486348fe Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh@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 pass it as the history variable
when prompting for a project, so it can be used as minibuffer history.
To support this, we minorly change its in-memory format: Instead of a
list of lists, each containing a project-root, it's just a list of
project-roots directly. This lets us pass it directly to functions
completing-read and add-to-history. The persistent format (what's
saved in project-list-file) remains the same.
Since project--list is now a minibuffer history variable, if the user
has savehist enabled, project--list will be saved and restored by
savehist. To avoid that overriding our own persistence mechanism, we
need a separate project--list-initialized variable.
* lisp/progmodes/project.el (project--list): Update docstring for new
format.
(project--list-initialized): Add.
(project--ensure-read-project-list): Check project--list-initialized.
(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-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, 63 insertions(+), 36 deletions(-)
diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index 95db9d0ef4c..bba1248fd73 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -1678,11 +1678,18 @@ 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'.")
+
+(defvar project--list-initialized nil)
(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 +1698,12 @@ 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))))))
+ (setq project--list-initialized t)
(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)
@@ -1703,20 +1711,22 @@ project--read-project-list
(defun project--ensure-read-project-list ()
"Initialize `project--list' if it isn't already initialized."
- (when (eq project--list 'unset)
+ (unless project--list-initialized
(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 +1738,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 +1752,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)
@@ -1772,24 +1782,34 @@ project-prompt-project-dir
(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-delete-duplicates t)
+ (history-length t))
+ (completing-read "Select project: " choices nil t nil 'project--list))))
(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--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 +1818,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 +1892,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
^ permalink raw reply related [flat|nested] 30+ messages in thread
* bug#67310: [PATCH] Include the project--list as history when prompting for a project
2023-11-20 19:58 bug#67310: [PATCH] Include the project--list as history when prompting for a project Spencer Baugh
@ 2023-11-21 11:06 ` Dmitry Gutov
2023-11-21 11:14 ` Dmitry Gutov
0 siblings, 1 reply; 30+ messages in thread
From: Dmitry Gutov @ 2023-11-21 11:06 UTC (permalink / raw)
To: Spencer Baugh, 67310; +Cc: eliz, juri
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.
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#67310: [PATCH] Include the project--list as history when prompting for a project
2023-11-21 11:06 ` Dmitry Gutov
@ 2023-11-21 11:14 ` Dmitry Gutov
2023-11-21 15:17 ` Spencer Baugh
0 siblings, 1 reply; 30+ messages in thread
From: Dmitry Gutov @ 2023-11-21 11:14 UTC (permalink / raw)
To: Spencer Baugh, 67310; +Cc: eliz, juri
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.
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#67310: [PATCH] Include the project--list as history when prompting for a project
2023-11-21 11:14 ` Dmitry Gutov
@ 2023-11-21 15:17 ` Spencer Baugh
2023-11-22 1:40 ` Dmitry Gutov
2023-11-22 1:42 ` Dmitry Gutov
0 siblings, 2 replies; 30+ messages in thread
From: Spencer Baugh @ 2023-11-21 15:17 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 67310, eliz, juri
[-- Attachment #1: Type: text/plain, Size: 1297 bytes --]
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.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Use-the-project-list-as-history-when-prompting-for-a.patch --]
[-- Type: text/x-patch, Size: 6376 bytes --]
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
^ permalink raw reply related [flat|nested] 30+ messages in thread
* bug#67310: [PATCH] Include the project--list as history when prompting for a project
2023-11-21 15:17 ` Spencer Baugh
@ 2023-11-22 1:40 ` Dmitry Gutov
2023-11-22 16:18 ` Spencer Baugh
2023-11-22 1:42 ` Dmitry Gutov
1 sibling, 1 reply; 30+ messages in thread
From: Dmitry Gutov @ 2023-11-22 1:40 UTC (permalink / raw)
To: Spencer Baugh; +Cc: 67310, eliz, juri
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?
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#67310: [PATCH] Include the project--list as history when prompting for a project
2023-11-21 15:17 ` Spencer Baugh
2023-11-22 1:40 ` Dmitry Gutov
@ 2023-11-22 1:42 ` Dmitry Gutov
2023-11-22 16:21 ` Spencer Baugh
1 sibling, 1 reply; 30+ messages in thread
From: Dmitry Gutov @ 2023-11-22 1:42 UTC (permalink / raw)
To: Spencer Baugh; +Cc: 67310, eliz, juri
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).
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#67310: [PATCH] Include the project--list as history when prompting for a project
2023-11-22 1:40 ` Dmitry Gutov
@ 2023-11-22 16:18 ` Spencer Baugh
2023-11-22 18:44 ` Dmitry Gutov
0 siblings, 1 reply; 30+ messages in thread
From: Spencer Baugh @ 2023-11-22 16:18 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 67310, eliz, juri
Dmitry Gutov <dmitry@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.
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#67310: [PATCH] Include the project--list as history when prompting for a project
2023-11-22 1:42 ` Dmitry Gutov
@ 2023-11-22 16:21 ` Spencer Baugh
0 siblings, 0 replies; 30+ messages in thread
From: Spencer Baugh @ 2023-11-22 16:21 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 67310, eliz, juri
Dmitry Gutov <dmitry@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)
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#67310: [PATCH] Include the project--list as history when prompting for a project
2023-11-22 16:18 ` Spencer Baugh
@ 2023-11-22 18:44 ` Dmitry Gutov
2023-11-22 23:14 ` Spencer Baugh
0 siblings, 1 reply; 30+ messages in thread
From: Dmitry Gutov @ 2023-11-22 18:44 UTC (permalink / raw)
To: Spencer Baugh; +Cc: 67310, eliz, juri
On 22/11/2023 18:18, Spencer Baugh wrote:
> Dmitry Gutov <dmitry@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.
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#67310: [PATCH] Include the project--list as history when prompting for a project
2023-11-22 18:44 ` Dmitry Gutov
@ 2023-11-22 23:14 ` Spencer Baugh
2023-11-23 2:55 ` Dmitry Gutov
2023-11-23 6:38 ` Eli Zaretskii
0 siblings, 2 replies; 30+ messages in thread
From: Spencer Baugh @ 2023-11-22 23:14 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 67310, eliz, juri
[-- Attachment #1: Type: text/plain, Size: 2065 bytes --]
Dmitry Gutov <dmitry@gutov.dev> writes:
> On 22/11/2023 18:18, Spencer Baugh wrote:
>> Dmitry Gutov <dmitry@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)
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Include-the-project-list-as-history-when-prompting-f.patch --]
[-- Type: text/x-patch, Size: 10296 bytes --]
From 063fe822531d51040be53f47f3dbe35ea77f21be Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh@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
^ permalink raw reply related [flat|nested] 30+ messages in thread
* bug#67310: [PATCH] Include the project--list as history when prompting for a project
2023-11-22 23:14 ` Spencer Baugh
@ 2023-11-23 2:55 ` Dmitry Gutov
2023-11-24 15:50 ` Spencer Baugh
2023-11-23 6:38 ` Eli Zaretskii
1 sibling, 1 reply; 30+ messages in thread
From: Dmitry Gutov @ 2023-11-23 2:55 UTC (permalink / raw)
To: Spencer Baugh; +Cc: 67310, eliz, juri
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).
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#67310: [PATCH] Include the project--list as history when prompting for a project
2023-11-22 23:14 ` Spencer Baugh
2023-11-23 2:55 ` Dmitry Gutov
@ 2023-11-23 6:38 ` Eli Zaretskii
2023-11-25 1:54 ` Dmitry Gutov
1 sibling, 1 reply; 30+ messages in thread
From: Eli Zaretskii @ 2023-11-23 6:38 UTC (permalink / raw)
To: Spencer Baugh; +Cc: 67310, dmitry, juri
> From: Spencer Baugh <sbaugh@janestreet.com>
> Cc: 67310@debbugs.gnu.org, eliz@gnu.org, juri@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.
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#67310: [PATCH] Include the project--list as history when prompting for a project
2023-11-23 2:55 ` Dmitry Gutov
@ 2023-11-24 15:50 ` Spencer Baugh
2023-11-25 2:07 ` Dmitry Gutov
` (2 more replies)
0 siblings, 3 replies; 30+ messages in thread
From: Spencer Baugh @ 2023-11-24 15:50 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 67310, eliz, juri
[-- Attachment #1: Type: text/plain, Size: 1944 bytes --]
Dmitry Gutov <dmitry@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)
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Use-the-project-list-as-history-when-prompting-for-a.patch --]
[-- Type: text/x-patch, Size: 6466 bytes --]
From 6b7b82be8a9a2d218a124e8205f3627d77dbb0a1 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. (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
^ permalink raw reply related [flat|nested] 30+ messages in thread
* bug#67310: [PATCH] Include the project--list as history when prompting for a project
2023-11-23 6:38 ` Eli Zaretskii
@ 2023-11-25 1:54 ` Dmitry Gutov
2023-11-25 8:42 ` Eli Zaretskii
0 siblings, 1 reply; 30+ messages in thread
From: Dmitry Gutov @ 2023-11-25 1:54 UTC (permalink / raw)
To: Eli Zaretskii, Spencer Baugh; +Cc: 67310, juri
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'.
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#67310: [PATCH] Include the project--list as history when prompting for a project
2023-11-24 15:50 ` Spencer Baugh
@ 2023-11-25 2:07 ` Dmitry Gutov
2023-11-25 17:50 ` Juri Linkov
2023-11-27 17:10 ` Juri Linkov
2 siblings, 0 replies; 30+ messages in thread
From: Dmitry Gutov @ 2023-11-25 2:07 UTC (permalink / raw)
To: Spencer Baugh; +Cc: 67310, eliz, juri
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.
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#67310: [PATCH] Include the project--list as history when prompting for a project
2023-11-25 1:54 ` Dmitry Gutov
@ 2023-11-25 8:42 ` Eli Zaretskii
2023-11-25 14:05 ` Dmitry Gutov
0 siblings, 1 reply; 30+ messages in thread
From: Eli Zaretskii @ 2023-11-25 8:42 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 67310, sbaugh, juri
> Date: Sat, 25 Nov 2023 03:54:13 +0200
> Cc: 67310@debbugs.gnu.org, juri@linkov.net
> From: Dmitry Gutov <dmitry@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.
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#67310: [PATCH] Include the project--list as history when prompting for a project
2023-11-25 8:42 ` Eli Zaretskii
@ 2023-11-25 14:05 ` Dmitry Gutov
2023-11-25 14:10 ` Eli Zaretskii
0 siblings, 1 reply; 30+ messages in thread
From: Dmitry Gutov @ 2023-11-25 14:05 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 67310, sbaugh, juri
On 25/11/2023 10:42, Eli Zaretskii wrote:
>> Date: Sat, 25 Nov 2023 03:54:13 +0200
>> Cc: 67310@debbugs.gnu.org, juri@linkov.net
>> From: Dmitry Gutov <dmitry@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.
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#67310: [PATCH] Include the project--list as history when prompting for a project
2023-11-25 14:05 ` Dmitry Gutov
@ 2023-11-25 14:10 ` Eli Zaretskii
2023-11-25 15:06 ` Dmitry Gutov
0 siblings, 1 reply; 30+ messages in thread
From: Eli Zaretskii @ 2023-11-25 14:10 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 67310, sbaugh, juri
> Date: Sat, 25 Nov 2023 16:05:03 +0200
> Cc: sbaugh@janestreet.com, 67310@debbugs.gnu.org, juri@linkov.net
> From: Dmitry Gutov <dmitry@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.
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#67310: [PATCH] Include the project--list as history when prompting for a project
2023-11-25 14:10 ` Eli Zaretskii
@ 2023-11-25 15:06 ` Dmitry Gutov
2023-11-25 15:57 ` Eli Zaretskii
0 siblings, 1 reply; 30+ messages in thread
From: Dmitry Gutov @ 2023-11-25 15:06 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 67310, sbaugh, juri
On 25/11/2023 16:10, Eli Zaretskii wrote:
>> Date: Sat, 25 Nov 2023 16:05:03 +0200
>> Cc:sbaugh@janestreet.com,67310@debbugs.gnu.org,juri@linkov.net
>> From: Dmitry Gutov<dmitry@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.
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#67310: [PATCH] Include the project--list as history when prompting for a project
2023-11-25 15:06 ` Dmitry Gutov
@ 2023-11-25 15:57 ` Eli Zaretskii
2023-11-25 16:35 ` Dmitry Gutov
0 siblings, 1 reply; 30+ messages in thread
From: Eli Zaretskii @ 2023-11-25 15:57 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 67310, sbaugh, juri
> Date: Sat, 25 Nov 2023 17:06:06 +0200
> Cc: sbaugh@janestreet.com, 67310@debbugs.gnu.org, juri@linkov.net
> From: Dmitry Gutov <dmitry@gutov.dev>
>
> On 25/11/2023 16:10, Eli Zaretskii wrote:
> >> Date: Sat, 25 Nov 2023 16:05:03 +0200
> >> Cc:sbaugh@janestreet.com,67310@debbugs.gnu.org,juri@linkov.net
> >> From: Dmitry Gutov<dmitry@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.
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#67310: [PATCH] Include the project--list as history when prompting for a project
2023-11-25 15:57 ` Eli Zaretskii
@ 2023-11-25 16:35 ` Dmitry Gutov
0 siblings, 0 replies; 30+ messages in thread
From: Dmitry Gutov @ 2023-11-25 16:35 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 67310, sbaugh, juri
On 25/11/2023 17:57, Eli Zaretskii wrote:
>> Date: Sat, 25 Nov 2023 17:06:06 +0200
>> Cc:sbaugh@janestreet.com,67310@debbugs.gnu.org,juri@linkov.net
>> From: Dmitry Gutov<dmitry@gutov.dev>
>>
>> On 25/11/2023 16:10, Eli Zaretskii wrote:
>>>> Date: Sat, 25 Nov 2023 16:05:03 +0200
>>>> Cc:sbaugh@janestreet.com,67310@debbugs.gnu.org,juri@linkov.net
>>>> From: Dmitry Gutov<dmitry@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.
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#67310: [PATCH] Include the project--list as history when prompting for a project
2023-11-24 15:50 ` Spencer Baugh
2023-11-25 2:07 ` Dmitry Gutov
@ 2023-11-25 17:50 ` Juri Linkov
2023-11-27 17:10 ` Juri Linkov
2 siblings, 0 replies; 30+ messages in thread
From: Juri Linkov @ 2023-11-25 17:50 UTC (permalink / raw)
To: Spencer Baugh; +Cc: 67310, Dmitry Gutov, eliz
> (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.
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#67310: [PATCH] Include the project--list as history when prompting for a project
2023-11-24 15:50 ` Spencer Baugh
2023-11-25 2:07 ` Dmitry Gutov
2023-11-25 17:50 ` Juri Linkov
@ 2023-11-27 17:10 ` Juri Linkov
2023-12-10 3:04 ` Dmitry Gutov
2 siblings, 1 reply; 30+ messages in thread
From: Juri Linkov @ 2023-11-27 17:10 UTC (permalink / raw)
To: Spencer Baugh; +Cc: 67310, Dmitry Gutov, eliz
> + ;; 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
^ permalink raw reply related [flat|nested] 30+ messages in thread
* bug#67310: [PATCH] Include the project--list as history when prompting for a project
2023-11-27 17:10 ` Juri Linkov
@ 2023-12-10 3:04 ` Dmitry Gutov
2023-12-10 17:43 ` Juri Linkov
0 siblings, 1 reply; 30+ messages in thread
From: Dmitry Gutov @ 2023-12-10 3:04 UTC (permalink / raw)
To: Juri Linkov, Spencer Baugh; +Cc: 67310, eliz
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.
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#67310: [PATCH] Include the project--list as history when prompting for a project
2023-12-10 3:04 ` Dmitry Gutov
@ 2023-12-10 17:43 ` Juri Linkov
2023-12-10 20:32 ` Dmitry Gutov
0 siblings, 1 reply; 30+ messages in thread
From: Juri Linkov @ 2023-12-10 17:43 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 67310, Spencer Baugh, eliz
>> 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.
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#67310: [PATCH] Include the project--list as history when prompting for a project
2023-12-10 17:43 ` Juri Linkov
@ 2023-12-10 20:32 ` Dmitry Gutov
2023-12-11 17:12 ` Juri Linkov
0 siblings, 1 reply; 30+ messages in thread
From: Dmitry Gutov @ 2023-12-10 20:32 UTC (permalink / raw)
To: Juri Linkov; +Cc: 67310, Spencer Baugh, eliz
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.
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#67310: [PATCH] Include the project--list as history when prompting for a project
2023-12-10 20:32 ` Dmitry Gutov
@ 2023-12-11 17:12 ` Juri Linkov
2023-12-12 0:21 ` Dmitry Gutov
0 siblings, 1 reply; 30+ messages in thread
From: Juri Linkov @ 2023-12-11 17:12 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 67310, Spencer Baugh, eliz
>>>> 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.
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#67310: [PATCH] Include the project--list as history when prompting for a project
2023-12-11 17:12 ` Juri Linkov
@ 2023-12-12 0:21 ` Dmitry Gutov
2023-12-14 1:02 ` sbaugh
0 siblings, 1 reply; 30+ messages in thread
From: Dmitry Gutov @ 2023-12-12 0:21 UTC (permalink / raw)
To: Juri Linkov; +Cc: 67310, Spencer Baugh, eliz
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.
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#67310: [PATCH] Include the project--list as history when prompting for a project
2023-12-12 0:21 ` Dmitry Gutov
@ 2023-12-14 1:02 ` sbaugh
2023-12-19 17:35 ` Juri Linkov
0 siblings, 1 reply; 30+ messages in thread
From: sbaugh @ 2023-12-14 1:02 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 67310, Spencer Baugh, eliz, Juri Linkov
Dmitry Gutov <dmitry@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.
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#67310: [PATCH] Include the project--list as history when prompting for a project
2023-12-14 1:02 ` sbaugh
@ 2023-12-19 17:35 ` Juri Linkov
0 siblings, 0 replies; 30+ messages in thread
From: Juri Linkov @ 2023-12-19 17:35 UTC (permalink / raw)
To: sbaugh; +Cc: 67310, Dmitry Gutov, eliz, Spencer Baugh
>>>>>>> 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.
^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2023-12-19 17:35 UTC | newest]
Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-20 19:58 bug#67310: [PATCH] Include the project--list as history when prompting for a project Spencer Baugh
2023-11-21 11:06 ` Dmitry Gutov
2023-11-21 11:14 ` Dmitry Gutov
2023-11-21 15:17 ` Spencer Baugh
2023-11-22 1:40 ` Dmitry Gutov
2023-11-22 16:18 ` Spencer Baugh
2023-11-22 18:44 ` Dmitry Gutov
2023-11-22 23:14 ` Spencer Baugh
2023-11-23 2:55 ` Dmitry Gutov
2023-11-24 15:50 ` Spencer Baugh
2023-11-25 2:07 ` Dmitry Gutov
2023-11-25 17:50 ` Juri Linkov
2023-11-27 17:10 ` Juri Linkov
2023-12-10 3:04 ` Dmitry Gutov
2023-12-10 17:43 ` Juri Linkov
2023-12-10 20:32 ` Dmitry Gutov
2023-12-11 17:12 ` Juri Linkov
2023-12-12 0:21 ` Dmitry Gutov
2023-12-14 1:02 ` sbaugh
2023-12-19 17:35 ` Juri Linkov
2023-11-23 6:38 ` Eli Zaretskii
2023-11-25 1:54 ` Dmitry Gutov
2023-11-25 8:42 ` Eli Zaretskii
2023-11-25 14:05 ` Dmitry Gutov
2023-11-25 14:10 ` Eli Zaretskii
2023-11-25 15:06 ` Dmitry Gutov
2023-11-25 15:57 ` Eli Zaretskii
2023-11-25 16:35 ` Dmitry Gutov
2023-11-22 1:42 ` Dmitry Gutov
2023-11-22 16:21 ` Spencer Baugh
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.