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

[sbaugh@catern.com]

Spencer Baugh <sbaugh@janestreet.com> writes:

> Dmitry Gutov <dmitry@gutov.dev> writes:
>
>> Hi Spencer,
>>
>> Thanks for the ping.
>>
>> On 06/06/2023 18:55, Spencer Baugh wrote:
>>
>>>> But so far the patch over there is not complete yet, is it? I wouldn't
>>>> say it's a settled matter so far.
>>> Yes, I expect there are any number of alternative implementation
>>> strategies, I'm not at all tied to using
>>> project-current-directory-override.  Happy to port to whatever approach
>>> we end up with.
>>
>> Notes:
>>
>> - We're still using project-current-directory-override, not migrated
>>   to anything else yet.
>> - I've pushed my earlier patch which should fix the immediate bug as
>>   reported.
>>
>> Let's talk about yours a little bit more. I'm a little wary of adding
>> a specialized feature this way (being able to visit the file
>> corresponding to the current one only), but that patch might be the
>> most optimal still.
>>
>>>>> BTW, I asked about this before inhttps://debbugs.gnu.org/58447#127
>>>>> and then it was deemed to be not too general to handle, so I backed it out
>>>>> inhttps://debbugs.gnu.org/58447#160  with such conclusion:
>>>>>     OTOH, `C-x p f M-p' in another project is not my primary
>>>>> workflow.
>>>>>     But if someone wants to keep a plain history, this could be added
>>>>>     later in master, e.g. by a new value of project-read-file-name-function
>>>>>     and a function that is mostly a copy of project--read-file-cpd-relative.
>>>>> So maybe this could be implemented in master now?
>>>>
>>>> I think the design there was to use relative file names in history? Or
>>>> a different variable for project file name history (which would use
>>>> relative names only). I'm not ruling that out, but the patch proposed
>>>> here is a little more focused.
>>>>
>>>> OTOH, it only allows finding the "current" file in the other project,
>>>> but not other files that were previously visited too. Spencer, what do
>>>> you think about that capability? Do you also feel it is missing and
>>>> would like to look into it next? Then the current patch might be the
>>>> wrong direction.
>>> Hm, the main thing I want is to make it very easy to visit the
>>> current
>>> file in another project - I am frequently getting user requests for that
>>> feature.  (Mainly because our workflow heavily uses a "git worktree"
>>> equivalent, where users have one project for each bug/branch they're
>>> working on, all with basically the same layout, so "visit the same file
>>> in a different project" is also "visit the same file in a different
>>> branch", which is often useful.  (I actually might work on some code to
>>> help implement the same kind of workflow for Emacs development, one
>>> worktree per bug/branch))
>>
>> I suppose one other way to do that would be to create a dedicated
>> command just for that. But that's a little clunkier -- would require a
>> separate binding.
>>
>>> I'm not sure I understand the alternative - the idea would be to share
>>> project file name history between all projects?  I guess that could be
>>> nice, although I don't personally use file name history that much, and
>>> AFAIK it wouldn't solve any concrete user problems, so I'm not really
>>> motivated to implement it.
>>
>> The alternative is a little more general, e.g. propertize every such
>> history entry with the value of the root, so that they can be
>> post-processed to adapt to any other root directory.
>>
>> This shouldn't take too much work, actually. But I don't know if that
>> is indeed a necessary feature. From the discussion
>> (https://debbugs.gnu.org/58447), I had been under impression that it
>> would be wanted, but it might be just "nice to have".
>
> I would be happy to do that.  That sounds very cool actually.
>
> Can you elaborate on how exactly you imagine this happening?  I guess,
> every time someone enters a filename with project-find-file or
> project-find-dir, we would include a propertized version of that
> filename in file-name-history?  And then we would re-relativize them and
> adapt them to the current project when including them as history?

Okay, I did that.  Extremely rough patch follows.

Btw, the reason I'm interested in this shared project history is because
our workflow involves creating many new projects (one per branch); so
mostly each project has no history at all, and sharing history between
projects is the only way to get any.

But, it seems to me that this doesn't really help with having the
current file be "future history".  That's still useful when switching
between similar projects.  And all the logic of my other patch which
does that, is still required with this patch.

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index 78f9fb410c1..5752f26d229 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -1028,6 +1028,12 @@ project-read-file-name-function
   :group 'project
   :version "27.1")
 
+(defun project--expand-file-name (filename project)
+  (when-let ((old-pr (get-text-property 0 'project filename)))
+    (expand-file-name
+     (file-relative-name filename (project-root old-pr))
+     (project-root project))))
+
 (defun project--read-file-cpd-relative (prompt
                                         all-files &optional predicate
                                         hist mb-default)
@@ -1038,7 +1044,8 @@ project--read-file-cpd-relative
 
 MB-DEFAULT is used as part of \"future history\", to be inserted
 by the user at will."
-  (let* ((common-parent-directory
+  (let* ((project (project-current t))
+         (common-parent-directory
           (let ((common-prefix (try-completion "" all-files)))
             (if (> (length common-prefix) 0)
                 (file-name-directory common-prefix))))
@@ -1060,6 +1067,7 @@ project--read-file-cpd-relative
                             ((symbol-value hist)
                              (mapcan
                               (lambda (s)
+                                (setq s (or (abbreviate-file-name (project--expand-file-name s project)) s))
                                 (and (string-prefix-p abbr-cpd s)
                                      (not (eq abbr-cpd-length (length s)))
                                      (list (substring s abbr-cpd-length))))
@@ -1070,7 +1078,9 @@ project--read-file-cpd-relative
                                                      hist mb-default)))
          (absname (expand-file-name relname common-parent-directory)))
     (when (and hist history-add-new-input)
-      (add-to-history hist (abbreviate-file-name absname)))
+      (add-to-history hist (propertize
+                            (abbreviate-file-name absname)
+                            'project project)))
     absname))
 
 (defun project--read-file-absolute (prompt