GNU bug report logs - #63829
29.0.90; project-find-file's future history breaks with common-parent-directory

Previous Next

Full log

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: sbaugh <at> catern.com
Cc: Spencer Baugh <sbaugh <at> janestreet.com>, Juri Linkov <juri <at> linkov.net>,
 63829 <at> debbugs.gnu.org
Subject: Re: bug#63829: 29.0.90; project-find-file's future history breaks
 with common-parent-directory
Date: Thu, 17 Aug 2023 05:14:03 +0300

On 16/08/2023 05:57, sbaugh <at> catern.com wrote:

>> Now that you can have this additional capability as an option, do you
>> think you will be using it as well?
> 
> Yes, probably I will enable this shared project history mode for my
> users.

Nice.

>> Before we do that, small (or not so small) question: do you think we
>> should test that the current buffer exists in the other project too?
>> We could do that with file-exists-p (but that's an extra round-trip
>> over Tramp), or by checking against the full list like in below.
> 
> No, I don't think that's necessary.  It produces more consistent
> behavior to not check whether the file exists.  And anyway, it could
> maybe be helpful to be able to create the same file in another project.

Fair point. Creating new files it is, then.

>> Relatedly, with the cross-project history, we should ask the same
>> question: will we check that the "transplanted" history entries
>> correspond to existing files in the other project (and filter out
>> those that don't).
> 
> Likewise I don't think that's necessary.
> 
> Although it might be nice to support a user-supplied predicate which,
> given the current project and a path in the history (which contains as a
> property the originating project), determines whether to show that path.
> Then the user could filter the history to only paths in "sibling
> projects" with similar content.  Not required though.

OK, we can easily add such a predicate later, if somebody asks.

I'm pushed the first of your patches, but the second needed some 
adjustments. Chiefly because we need to make sure it works with any 
value of project-read-file-name-function, so the impl can't be 
concentrated in just one of them.

Check out the amended patch below. Any suggestions on how to do it more 
elegantly (without duplicating the add-to-history call) are welcome too.

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index e1d14474323..d810d8d9605 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -1046,6 +1046,13 @@ project-read-file-name-function
   :group 'project
   :version "27.1")

+(defun project--expand-file-name (filename project)
+  (when-let ((old-root (get-text-property 0 'project filename)))
+    (abbreviate-file-name
+     (expand-file-name
+      (file-relative-name filename old-root)
+      (project-root project)))))
+
 (defun project--read-file-cpd-relative (prompt
                                         all-files &optional predicate
                                         hist mb-default)
@@ -1124,9 +1131,18 @@ project-find-file-in
                dirs)
             (project-files project dirs)))
          (completion-ignore-case read-file-name-completion-ignore-case)
-         (file (funcall project-read-file-name-function
-                        "Find file" all-files nil 'file-name-history
-                        suggested-filename)))
+         (file
+          (let ((file-name-history (mapcar
+                                    (lambda (f)
+                                      (or (project--expand-file-name f 
project) f))
+                                    file-name-history)))
+            (funcall project-read-file-name-function
+                     "Find file" all-files nil 'file-name-history
+                     suggested-filename))))
+    (when history-add-new-input
+      ;; Have to re-add it here because of the let-binding above.
+      (add-to-history 'file-name-history
+                      (propertize file 'project (project-root project))))
     (if (string= file "")
         (user-error "You didn't specify the file")
       (find-file file))))

Previous Next

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