unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Dmitry Gutov <dmitry@gutov.dev>
To: sbaugh@catern.com
Cc: Spencer Baugh <sbaugh@janestreet.com>,
	Juri Linkov <juri@linkov.net>,
	63829@debbugs.gnu.org
Subject: 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	[thread overview]
Message-ID: <f447ea97-c299-1a53-465d-063690e717d9@gutov.dev> (raw)
In-Reply-To: <875y5fitiq.fsf@catern.com>

On 16/08/2023 05:57, sbaugh@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))))






  reply	other threads:[~2023-08-17  2:14 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-01 22:32 bug#63829: 29.0.90; project-find-file's future history breaks with common-parent-directory Spencer Baugh
2023-06-02  6:47 ` Eli Zaretskii
2023-06-03 12:19   ` Dmitry Gutov
2023-06-03 12:48     ` Eli Zaretskii
2023-06-03 13:48       ` Dmitry Gutov
2023-06-03  2:30 ` Dmitry Gutov
2023-06-03 11:00   ` Spencer Baugh
2023-06-04 16:36     ` Juri Linkov
2023-06-06  1:40       ` Dmitry Gutov
2023-06-06 15:55         ` Spencer Baugh
2023-08-10 12:02           ` sbaugh
2023-08-12  1:23           ` Dmitry Gutov
2023-08-14 20:12             ` Spencer Baugh
2023-08-14 22:47               ` sbaugh
2023-08-16  1:49                 ` Dmitry Gutov
2023-08-16  2:57                   ` sbaugh
2023-08-17  2:14                     ` Dmitry Gutov [this message]
2023-08-17 19:41                       ` Spencer Baugh
2023-08-17 20:12                         ` Spencer Baugh
2023-08-18 20:57                           ` Spencer Baugh
2023-08-19  2:14                             ` Dmitry Gutov
2023-08-20 17:23                             ` Juri Linkov
2023-08-20 17:16                           ` Juri Linkov
2023-08-21  1:15                             ` Dmitry Gutov
2023-08-23  2:13                           ` Dmitry Gutov
2023-08-19  2:08                         ` Dmitry Gutov
2023-08-19 12:00                           ` sbaugh
2023-08-21  1:51                             ` Dmitry Gutov
2023-08-20 17:20                         ` Juri Linkov
2023-08-21  1:43                           ` Dmitry Gutov
2023-08-21  7:06                             ` Juri Linkov
2023-08-23  0:37                               ` Dmitry Gutov
2023-08-23  2:26                         ` Dmitry Gutov
2023-08-23 17:52                           ` Juri Linkov
2023-08-23 18:25                             ` Dmitry Gutov
2023-08-20 17:13                       ` Juri Linkov
2023-08-21  1:17                         ` Dmitry Gutov
2023-08-21  6:58                           ` Juri Linkov
2023-08-23  0:27                             ` Dmitry Gutov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f447ea97-c299-1a53-465d-063690e717d9@gutov.dev \
    --to=dmitry@gutov.dev \
    --cc=63829@debbugs.gnu.org \
    --cc=juri@linkov.net \
    --cc=sbaugh@catern.com \
    --cc=sbaugh@janestreet.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).