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

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

[Spencer Baugh]

1. emacs -Q
2. Open a project where project-files only returns files in a certain
subdirectory.  For example, a git repo /repo where the only file is
"dir/file.txt".
3. Open dir/file.txt
4. C-x p f   ;; project-find file
5. Observe that the prompt is "Find file in /repo/dir: " which correctly
contains the common parent directory between all the paths returned by
project-files.
6. M-n       ;; next-history-element
7. The minibuffer now contains "dir/file.txt".  RET will fail to
open the file.

Instead, the common parent directory should be stripped from the "future
history" element.

(As a separate point: I ran into this while adding a feature for
switching between projects with similar directory structures.  I want to
include the relative path in the starting project in the "future
history", so that when you have a file in projectA open, you can switch
to the same file in projectB with C-x p p f M-n RET.  For example,
switching between the same file in multiple clones of Emacs.  But sadly
the future history doesn't work properly right now even in a single
project)


In GNU Emacs 29.0.90 (build 3, x86_64-pc-linux-gnu, X toolkit, cairo
 version 1.15.12, Xaw scroll bars) of 2023-05-17 built on
 igm-qws-u22796a
Repository revision: 4d08492296c2a6d2910f2b740c2d2508275458fc
Repository branch: emacs-29
Windowing system distributor 'The X.Org Foundation', version 11.0.12011000
System Description: CentOS Linux 7 (Core)

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

Configured features:
CAIRO DBUS FREETYPE GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG JSON
LIBSELINUX LIBXML2 MODULES NOTIFY INOTIFY PDUMPER PNG RSVG SECCOMP SOUND
SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS X11 XDBE XIM XINPUT2 XPM LUCID
ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

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

[Eli Zaretskii]

> From: Spencer Baugh <sbaugh@janestreet.com>
> Date: Thu, 01 Jun 2023 18:32:29 -0400
> 
> 1. emacs -Q
> 2. Open a project where project-files only returns files in a certain
> subdirectory.  For example, a git repo /repo where the only file is
> "dir/file.txt".
> 3. Open dir/file.txt
> 4. C-x p f   ;; project-find file
> 5. Observe that the prompt is "Find file in /repo/dir: " which correctly
> contains the common parent directory between all the paths returned by
> project-files.
> 6. M-n       ;; next-history-element
> 7. The minibuffer now contains "dir/file.txt".  RET will fail to
> open the file.
> 
> Instead, the common parent directory should be stripped from the "future
> history" element.

From my POV, this is a clear sign of too many kludges which override
the usual Emacs conventions of what is default-directory.  Stripping
the parent directory is IMO not the right solution; instead, the
default-directory of the command should be set so that what you want
happens automatically.  If you go the way of patching up the code
instead of fixing this fundamental problem, there will be no end to
patching up.

> (As a separate point: I ran into this while adding a feature for
> switching between projects with similar directory structures.  I want to
> include the relative path in the starting project in the "future
> history", so that when you have a file in projectA open, you can switch
> to the same file in projectB with C-x p p f M-n RET.  For example,
> switching between the same file in multiple clones of Emacs.  But sadly
> the future history doesn't work properly right now even in a single
> project)

Once again, this should work by using the right value of
default-directory; having relative filenames in the history up front
is not TRT.  Relative file names in Emacs are always interpreted
relatively to default-directory, so if you start using relative names
disregarding default-directory, you will eventually run into trouble,
as various file-related primitives will fail with ENOENT.

If the default-directory can be different for some elements of
history, perhaps each element should have the default-directory
metadata with it, e.g., as a property.  Or maybe the history should
hold absolute file names, and M-n etc. should convert it to relative
as appropriate.  Or something.  But just treating relative file names
as strings unrelated to the (implied) default-directory is simply
WRONG in Emacs.

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

[Dmitry Gutov]

Hi!

On 02/06/2023 01:32, Spencer Baugh wrote:
> 
> 1. emacs -Q
> 2. Open a project where project-files only returns files in a certain
> subdirectory.  For example, a git repo /repo where the only file is
> "dir/file.txt".
> 3. Open dir/file.txt
> 4. C-x p f   ;; project-find file
> 5. Observe that the prompt is "Find file in /repo/dir: " which correctly
> contains the common parent directory between all the paths returned by
> project-files.
> 6. M-n       ;; next-history-element
> 7. The minibuffer now contains "dir/file.txt".  RET will fail to
> open the file.

Note that if you continue pressing 'M-n', you will see the corresponding 
proper relative file names. The first one comes from the value 
constructed in project-find-file. Which indeed looks problematic, since 
we remove context in there by creating a relative name.

> Instead, the common parent directory should be stripped from the "future
> history" element.

Try the patch at the end, please. It seems to fix the scenario you 
presented. Does it help with the feature you mention below, too?

It doesn't handle the case when (thing-at-point 'filename) returns a 
relative file name that includes a directory name, relative to the 
project root (where common-parent-directory differs), but that one seems 
even more ambiguous.

> (As a separate point: I ran into this while adding a feature for
> switching between projects with similar directory structures.  I want to
> include the relative path in the starting project in the "future
> history", so that when you have a file in projectA open, you can switch
> to the same file in projectB with C-x p p f M-n RET.  For example,
> switching between the same file in multiple clones of Emacs.  But sadly
> the future history doesn't work properly right now even in a single
> project)

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index 7c51778d5d4..184f2316074 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -1008,7 +1008,7 @@ project-find-file
           (dirs (list root)))
      (project-find-file-in
       (or (thing-at-point 'filename)
-         (and buffer-file-name (file-relative-name buffer-file-name root)))
+         buffer-file-name)
       dirs pr include-all)))

  ;;;###autoload
@@ -1062,6 +1062,10 @@ project--read-file-cpd-relative
                                 (delete common-parent-directory all-files))
                           t))
           (substrings (mapcar (lambda (s) (substring s cpd-length)) 
all-files))
+         (mb-default (if (and common-parent-directory
+                              (file-name-absolute-p mb-default))
+                         (file-relative-name mb-default 
common-parent-directory)
+                       mb-default))
           (_ (when included-cpd
                (setq substrings (cons "./" substrings))))
           (new-collection (project--file-completion-table substrings))

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

[Spencer Baugh]

Dmitry Gutov <dmitry@gutov.dev> writes:
> Try the patch at the end, please. It seems to fix the scenario you
> presented. Does it help with the feature you mention below, too?

Yes, your patch works great and solves the cases I care about!

On top of your patch, I can implement the feature I mentioned with the
patch at the end.  This causes the following nice behavior:

1. Open ~/src/emacs/emacs-29/lisp/progmodes/project.el
2. C-x p p ~/src/emacs/trunk
3. f
4. M-n and the minibuffer contains "lisp/progmodes/project.el"
5. RET and we have now easily switched to the same file in another
   project

Does the implementation seem OK?

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index ac7be8dcbb2..b1e01df5314 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -1008,7 +1008,12 @@ project-find-file
          (dirs (list root)))
     (project-find-file-in
      (or (thing-at-point 'filename)
-         buffer-file-name)
+         (and buffer-file-name
+              (if-let (buffer-proj (and project-current-directory-override
+                                       (project-current nil default-directory)))
+                  (let ((buffer-root (project-root buffer-proj)))
+                    (file-name-concat root (file-relative-name buffer-file-name buffer-root)))
+                buffer-file-name)))
      dirs pr include-all)))
 
 ;;;###autoload

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

[Dmitry Gutov]

Hi Eli,

On 02/06/2023 09:47, Eli Zaretskii wrote:
>> From: Spencer Baugh <sbaugh@janestreet.com>
>> Date: Thu, 01 Jun 2023 18:32:29 -0400
>>
>> 1. emacs -Q
>> 2. Open a project where project-files only returns files in a certain
>> subdirectory.  For example, a git repo /repo where the only file is
>> "dir/file.txt".
>> 3. Open dir/file.txt
>> 4. C-x p f   ;; project-find file
>> 5. Observe that the prompt is "Find file in /repo/dir: " which correctly
>> contains the common parent directory between all the paths returned by
>> project-files.
>> 6. M-n       ;; next-history-element
>> 7. The minibuffer now contains "dir/file.txt".  RET will fail to
>> open the file.
>>
>> Instead, the common parent directory should be stripped from the "future
>> history" element.
> 
>  From my POV, this is a clear sign of too many kludges which override
> the usual Emacs conventions of what is default-directory.  Stripping
> the parent directory is IMO not the right solution; instead, the
> default-directory of the command should be set so that what you want
> happens automatically.  If you go the way of patching up the code
> instead of fixing this fundamental problem, there will be no end to
> patching up.

TBH, I'm not sure which particular change you were proposing.

Do you like the patch I posted? It could be considered somewhere in that 
direction.

Should it go to emacs-29 or master?

>> (As a separate point: I ran into this while adding a feature for
>> switching between projects with similar directory structures.  I want to
>> include the relative path in the starting project in the "future
>> history", so that when you have a file in projectA open, you can switch
>> to the same file in projectB with C-x p p f M-n RET.  For example,
>> switching between the same file in multiple clones of Emacs.  But sadly
>> the future history doesn't work properly right now even in a single
>> project)
> 
> Once again, this should work by using the right value of
> default-directory; having relative filenames in the history up front
> is not TRT.  Relative file names in Emacs are always interpreted
> relatively to default-directory, so if you start using relative names
> disregarding default-directory, you will eventually run into trouble,
> as various file-related primitives will fail with ENOENT.

The problem here is that is a different/new scenario where Spencer wants 
to have a file name from one project be applied to another project. It 
seems like using absolute names would rather go in the opposite direction.

> If the default-directory can be different for some elements of
> history, perhaps each element should have the default-directory
> metadata with it, e.g., as a property.  Or maybe the history should
> hold absolute file names, and M-n etc. should convert it to relative
> as appropriate.
The latter is mostly what happens.

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

[Eli Zaretskii]

> Date: Sat, 3 Jun 2023 15:19:55 +0300
> Cc: 63829@debbugs.gnu.org
> From: Dmitry Gutov <dmitry@gutov.dev>
> 
> On 02/06/2023 09:47, Eli Zaretskii wrote:
> >> From: Spencer Baugh <sbaugh@janestreet.com>
> >> Date: Thu, 01 Jun 2023 18:32:29 -0400
> >>
> >> 1. emacs -Q
> >> 2. Open a project where project-files only returns files in a certain
> >> subdirectory.  For example, a git repo /repo where the only file is
> >> "dir/file.txt".
> >> 3. Open dir/file.txt
> >> 4. C-x p f   ;; project-find file
> >> 5. Observe that the prompt is "Find file in /repo/dir: " which correctly
> >> contains the common parent directory between all the paths returned by
> >> project-files.
> >> 6. M-n       ;; next-history-element
> >> 7. The minibuffer now contains "dir/file.txt".  RET will fail to
> >> open the file.
> >>
> >> Instead, the common parent directory should be stripped from the "future
> >> history" element.
> > 
> >  From my POV, this is a clear sign of too many kludges which override
> > the usual Emacs conventions of what is default-directory.  Stripping
> > the parent directory is IMO not the right solution; instead, the
> > default-directory of the command should be set so that what you want
> > happens automatically.  If you go the way of patching up the code
> > instead of fixing this fundamental problem, there will be no end to
> > patching up.
> 
> TBH, I'm not sure which particular change you were proposing.

I was talking about the description of the problem, and what
conclusion it caused me to draw.

> Do you like the patch I posted? It could be considered somewhere in that 
> direction.

I don't know enough about the details to have opinion of any
importance.  But if the change goes in the direction I thought we
should go, then that's good.

> Should it go to emacs-29 or master?

Unless this is a bad problem, I'd prefer that the change goes to
master.

> >> (As a separate point: I ran into this while adding a feature for
> >> switching between projects with similar directory structures.  I want to
> >> include the relative path in the starting project in the "future
> >> history", so that when you have a file in projectA open, you can switch
> >> to the same file in projectB with C-x p p f M-n RET.  For example,
> >> switching between the same file in multiple clones of Emacs.  But sadly
> >> the future history doesn't work properly right now even in a single
> >> project)
> > 
> > Once again, this should work by using the right value of
> > default-directory; having relative filenames in the history up front
> > is not TRT.  Relative file names in Emacs are always interpreted
> > relatively to default-directory, so if you start using relative names
> > disregarding default-directory, you will eventually run into trouble,
> > as various file-related primitives will fail with ENOENT.
> 
> The problem here is that is a different/new scenario where Spencer wants 
> to have a file name from one project be applied to another project. It 
> seems like using absolute names would rather go in the opposite direction.

If some command wants to produce file names in a different directory,
then that command should do something like

   (expand-file-name (file-name-nondirectory FILENAME) NEW-DIRECTORY)

The code which produces the original FILENAME should still produce an
absolute file name (or record its directory in some other way); it
should not know/assume anything about potential uses of that file
name.

> > If the default-directory can be different for some elements of
> > history, perhaps each element should have the default-directory
> > metadata with it, e.g., as a property.  Or maybe the history should
> > hold absolute file names, and M-n etc. should convert it to relative
> > as appropriate.
> The latter is mostly what happens.

I very much hope so, because anything else is going in the wrong
direction.

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

[Dmitry Gutov]

On 03/06/2023 15:48, Eli Zaretskii wrote:

>> Do you like the patch I posted? It could be considered somewhere in that
>> direction.
> 
> I don't know enough about the details to have opinion of any
> importance.  But if the change goes in the direction I thought we
> should go, then that's good.
> 
>> Should it go to emacs-29 or master?
> 
> Unless this is a bad problem, I'd prefer that the change goes to
> master.

Maybe it's not too serious, given that it requires the user to invoke 
"future history" (not everybody knows of it), and for the project to 
have all files in one subdirectory.

>>>> (As a separate point: I ran into this while adding a feature for
>>>> switching between projects with similar directory structures.  I want to
>>>> include the relative path in the starting project in the "future
>>>> history", so that when you have a file in projectA open, you can switch
>>>> to the same file in projectB with C-x p p f M-n RET.  For example,
>>>> switching between the same file in multiple clones of Emacs.  But sadly
>>>> the future history doesn't work properly right now even in a single
>>>> project)
>>>
>>> Once again, this should work by using the right value of
>>> default-directory; having relative filenames in the history up front
>>> is not TRT.  Relative file names in Emacs are always interpreted
>>> relatively to default-directory, so if you start using relative names
>>> disregarding default-directory, you will eventually run into trouble,
>>> as various file-related primitives will fail with ENOENT.
>>
>> The problem here is that is a different/new scenario where Spencer wants
>> to have a file name from one project be applied to another project. It
>> seems like using absolute names would rather go in the opposite direction.
> 
> If some command wants to produce file names in a different directory,
> then that command should do something like
> 
>     (expand-file-name (file-name-nondirectory FILENAME) NEW-DIRECTORY)
> 
> The code which produces the original FILENAME should still produce an
> absolute file name (or record its directory in some other way); it
> should not know/assume anything about potential uses of that file
> name.

Sounds like Spencer's last patch. It's not too great that we'll make 
project-find-file aware of project-current-directory-override, though.

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

[Juri Linkov]

> On top of your patch, I can implement the feature I mentioned with the
> patch at the end.  This causes the following nice behavior:
>
> 1. Open ~/src/emacs/emacs-29/lisp/progmodes/project.el
> 2. C-x p p ~/src/emacs/trunk
> 3. f
> 4. M-n and the minibuffer contains "lisp/progmodes/project.el"
> 5. RET and we have now easily switched to the same file in another
>    project
>
> Does the implementation seem OK?
>
> diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
> index ac7be8dcbb2..b1e01df5314 100644
> --- a/lisp/progmodes/project.el
> +++ b/lisp/progmodes/project.el
> @@ -1008,7 +1008,12 @@ project-find-file
>           (dirs (list root)))
>      (project-find-file-in
>       (or (thing-at-point 'filename)
> -         buffer-file-name)
> +         (and buffer-file-name
> +              (if-let (buffer-proj (and project-current-directory-override
> +                                       (project-current nil default-directory)))

But we are going to remove project-current-directory-override in bug#63648
where default-directory will be changed to next-default-directory.

BTW, I asked about this before in https://debbugs.gnu.org/58447#127
and then it was deemed to be not too general to handle, so I backed it out
in https://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?

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

[Dmitry Gutov]

On 04/06/2023 19:36, Juri Linkov wrote:
>> On top of your patch, I can implement the feature I mentioned with the
>> patch at the end.  This causes the following nice behavior:
>>
>> 1. Open ~/src/emacs/emacs-29/lisp/progmodes/project.el
>> 2. C-x p p ~/src/emacs/trunk
>> 3. f
>> 4. M-n and the minibuffer contains "lisp/progmodes/project.el"
>> 5. RET and we have now easily switched to the same file in another
>>     project
>>
>> Does the implementation seem OK?
>>
>> diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
>> index ac7be8dcbb2..b1e01df5314 100644
>> --- a/lisp/progmodes/project.el
>> +++ b/lisp/progmodes/project.el
>> @@ -1008,7 +1008,12 @@ project-find-file
>>            (dirs (list root)))
>>       (project-find-file-in
>>        (or (thing-at-point 'filename)
>> -         buffer-file-name)
>> +         (and buffer-file-name
>> +              (if-let (buffer-proj (and project-current-directory-override
>> +                                       (project-current nil default-directory)))
> But we are going to remove project-current-directory-override in bug#63648
> where default-directory will be changed to next-default-directory.

I guess the proposed logic could be reimplemented without using 
project-current-directory-override -- just based on the changed value of 
default-directory. And using the parent directory of buffer-file-name.

But so far the patch over there is not complete yet, is it? I wouldn't 
say it's a settled matter so far.

> 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.

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

[Spencer Baugh]

Dmitry Gutov <dmitry@gutov.dev> writes:

> On 04/06/2023 19:36, Juri Linkov wrote:
>>> On top of your patch, I can implement the feature I mentioned with the
>>> patch at the end.  This causes the following nice behavior:
>>>
>>> 1. Open ~/src/emacs/emacs-29/lisp/progmodes/project.el
>>> 2. C-x p p ~/src/emacs/trunk
>>> 3. f
>>> 4. M-n and the minibuffer contains "lisp/progmodes/project.el"
>>> 5. RET and we have now easily switched to the same file in another
>>>     project
>>>
>>> Does the implementation seem OK?
>>>
>>> diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
>>> index ac7be8dcbb2..b1e01df5314 100644
>>> --- a/lisp/progmodes/project.el
>>> +++ b/lisp/progmodes/project.el
>>> @@ -1008,7 +1008,12 @@ project-find-file
>>>            (dirs (list root)))
>>>       (project-find-file-in
>>>        (or (thing-at-point 'filename)
>>> -         buffer-file-name)
>>> +         (and buffer-file-name
>>> +              (if-let (buffer-proj (and project-current-directory-override
>>> +                                       (project-current nil default-directory)))
>> But we are going to remove project-current-directory-override in bug#63648
>> where default-directory will be changed to next-default-directory.
>
> I guess the proposed logic could be reimplemented without using
> project-current-directory-override -- just based on the changed value
> of default-directory. And using the parent directory of
> buffer-file-name.
>
> 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.

>> 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'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.

However, if we did share project file name history in that way, I'd want
to still automatically prepend the "current file" as history.  Even if
we didn't navigate to the current file via project-find-file, I still
want to make it very easy to visit the current file in another project.
Just sharing project file name history doesn't provide that.

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

[sbaugh]

Spencer Baugh <sbaugh@janestreet.com> writes:
> Dmitry Gutov <dmitry@gutov.dev> writes:
>> 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'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.
>
> However, if we did share project file name history in that way, I'd want
> to still automatically prepend the "current file" as history.  Even if
> we didn't navigate to the current file via project-find-file, I still
> want to make it very easy to visit the current file in another project.
> Just sharing project file name history doesn't provide that.

Any thoughts about this and my earlier patch?  I still am interested in
providing this feature.

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

[Dmitry Gutov]

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".

Juri, are *you* okay with the functionality in Spencer's patch? No need 
for further generality?

> However, if we did share project file name history in that way, I'd want
> to still automatically prepend the "current file" as history.  Even if
> we didn't navigate to the current file via project-find-file, I still
> want to make it very easy to visit the current file in another project.
> Just sharing project file name history doesn't provide that.

Fair point.

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

[Spencer Baugh]

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?

And also, we'd always prepend the current file as "future history",
propertized in this way?

> Juri, are *you* okay with the functionality in Spencer's patch? No
> need for further generality?
>
>> However, if we did share project file name history in that way, I'd want
>> to still automatically prepend the "current file" as history.  Even if
>> we didn't navigate to the current file via project-find-file, I still
>> want to make it very easy to visit the current file in another project.
>> Just sharing project file name history doesn't provide that.
>
> Fair point.

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

[sbaugh]

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

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

[Dmitry Gutov]

On 15/08/2023 01:47, sbaugh@catern.com wrote:

>>>> 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.

Thanks! It's very close to what I was thinking of (modulo some cosmetics 
and perf optimizations).

> 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.

Now that you can have this additional capability as an option, do you 
think you will be using it as well?

> 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.

Indeed, that's a good point. So I think we'll install your first patch 
either way.

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.

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

WDYT?

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index d8b12c9c880..a32bc2dd8d3 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -1116,7 +1116,10 @@ project-find-file-in
           (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)))
+                        (if (file-name-absolute-p suggested-filename)
+                            (and (member suggested-filename all-files)
+                                 suggested-filename)
+                          suggested-filename))))
      (if (string= file "")
          (user-error "You didn't specify the file")
        (find-file file))))

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

[sbaugh]

Dmitry Gutov <dmitry@gutov.dev> writes:
> On 15/08/2023 01:47, sbaugh@catern.com wrote:
>
>>>>> 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.
>
> Thanks! It's very close to what I was thinking of (modulo some
> cosmetics and perf optimizations).
>
>> 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.
>
> 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.

>> 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.
>
> Indeed, that's a good point. So I think we'll install your first patch
> either way.
>
> 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.

> 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.

> WDYT?
>
> diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
> index d8b12c9c880..a32bc2dd8d3 100644
> --- a/lisp/progmodes/project.el
> +++ b/lisp/progmodes/project.el
> @@ -1116,7 +1116,10 @@ project-find-file-in
>           (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)))
> +                        (if (file-name-absolute-p suggested-filename)
> +                            (and (member suggested-filename all-files)
> +                                 suggested-filename)
> +                          suggested-filename))))
>      (if (string= file "")
>          (user-error "You didn't specify the file")
>        (find-file file))))

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

[Dmitry Gutov]

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

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

[Spencer Baugh]

[-- Attachment #1: Type: text/plain, Size: 3001 bytes --]

Dmitry Gutov <dmitry@gutov.dev> writes:
> 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))))

This seems good, sure.  But doesn't this make the history entries appear
twice?

Maybe we should just pull the history-adding functionality out of
project-read-file-name-function entirely.  I've tried doing that below.

Also, I realized just now that this should probably affect
project-find-dir as well, as should my previous patch adding
project-relative future history.  (I actually coincidentally just now
got a user request for "switch between projects and stay in the same
dir")

So here's a revised version of this history change which also affects
project-find-dir.  In a subsequent mail I'll send a patch for the
"future history" behavior of project-find-dir too.  (yet to be written)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Support-adjusting-file-name-history-to-the-current-p.patch --]
[-- Type: text/x-patch, Size: 5780 bytes --]

From 9cb47b7476dfbaf0e9e45001d174da848ebf904d Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh@janestreet.com>
Date: Thu, 17 Aug 2023 15:41:04 -0400
Subject: [PATCH] Support adjusting file-name-history to the current project

This add project-file-name-history-relativize which has the effect
described in its docstring.  This implements a sort of sharing of
file-name-history between projects.

* lisp/progmodes/project.el (project-file-name-history-relativize):
Add.  (bug#63829)
(project--expand-file-name): Add.
(project--read-file-cpd-relative): Move history manipulations to
project--read-file-name.
(project--read-file-name): Add and use
project-file-name-history-relativize.
(project-find-file-in): Use project--read-file-name.
(project-find-dir): Use project--read-file-name.
---
 lisp/progmodes/project.el | 62 +++++++++++++++++++++++++++++++--------
 1 file changed, 50 insertions(+), 12 deletions(-)

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index c1ce5ce7b1f..e0f1f995ff2 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -1046,6 +1046,26 @@ project-read-file-name-function
   :group 'project
   :version "27.1")
 
+(defcustom project-file-name-history-relativize nil
+  "If non-nil, paths in `file-name-history' are adjusted for the current project.
+
+When non-nil and in `project-find-file' or `project-find-dir',
+paths in `file-name-history' are adjusted to be relative to
+whatever the current project is, instead of the project which
+added those paths.  This only affects history entries added by
+earlier calls to `project-find-file' or `project-find-dir'.
+
+When `project-read-file-name-function' is
+`project--read-file-cpd-relative' (the default), this has the
+effect of sharing more history between projects.")
+
+(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)
@@ -1079,8 +1099,7 @@ project--read-file-cpd-relative
          (new-collection (project--file-completion-table substrings))
          (abbr-cpd (abbreviate-file-name common-parent-directory))
          (abbr-cpd-length (length abbr-cpd))
-         (relname (cl-letf ((history-add-new-input nil)
-                            ((symbol-value hist)
+         (relname (cl-letf (((symbol-value hist)
                              (mapcan
                               (lambda (s)
                                 (and (string-prefix-p abbr-cpd s)
@@ -1092,8 +1111,6 @@ project--read-file-cpd-relative
                                                      predicate
                                                      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)))
     absname))
 
 (defun project--read-file-absolute (prompt
@@ -1104,6 +1121,26 @@ project--read-file-absolute
                                    predicate
                                    hist mb-default))
 
+(defun project--read-file-name (project prompt
+                                        all-files &optional predicate
+                                        hist mb-default)
+  "Call `project-read-file-name-function' with project-relative history."
+  (let ((file
+         (cl-letf ((history-add-new-input nil)
+                   ((symbol-value hist)
+                    (if project-file-name-history-relativize
+                        (mapcar
+                         (lambda (f)
+                           (or (project--expand-file-name f project) f))
+                         (symbol-value hist))
+                      (symbol-value hist))))
+           (funcall project-read-file-name-function
+                    prompt all-files predicate hist mb-default))))
+    (when (and hist history-add-new-input)
+      (add-to-history hist
+                      (propertize file 'project (project-root project))))
+    file))
+
 (defun project-find-file-in (suggested-filename dirs project &optional include-all)
   "Complete a file name in DIRS in PROJECT and visit the result.
 
@@ -1124,9 +1161,10 @@ 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 (project--read-file-name
+                project "Find file"
+                all-files nil 'file-name-history
+                suggested-filename)))
     (if (string= file "")
         (user-error "You didn't specify the file")
       (find-file file))))
@@ -1158,11 +1196,11 @@ project-find-dir
          ;; https://stackoverflow.com/a/50685235/615245 for possible
          ;; implementation.
          (all-dirs (mapcar #'file-name-directory all-files))
-         (dir (funcall project-read-file-name-function
-                       "Dired"
-                       ;; Some completion UIs show duplicates.
-                       (delete-dups all-dirs)
-                       nil 'file-name-history)))
+         (dir (project--read-file-name
+               project "Dired"
+               ;; Some completion UIs show duplicates.
+               (delete-dups all-dirs)
+               nil 'file-name-history)))
     (dired dir)))
 
 ;;;###autoload
-- 
2.39.3

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

[Spencer Baugh]

[-- Attachment #1: Type: text/plain, Size: 279 bytes --]

Spencer Baugh <sbaugh@janestreet.com> writes:
> In a subsequent mail I'll send a patch for the "future history"
> behavior of project-find-dir too.  (yet to be written)

Here is that.  Seems like a straightforward generalization, and
something that a user would expect to work.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Use-current-file-name-for-more-other-project-future-.patch --]
[-- Type: text/x-patch, Size: 4913 bytes --]

From 48880a795f75d1126a0f235bf57f00f478ee90a0 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh@janestreet.com>
Date: Thu, 17 Aug 2023 16:11:01 -0400
Subject: [PATCH] Use current file name for more "other project" future history

In commit f38bcf37dc47ce172c985d1c621df3583eaad46c we supported using
the current buffer's file name as future history for project-find-file
even when switching to another project with project-switch-project.
Make this work for project-find-dir and project-or-external-find-file
too.

* lisp/progmodes/project.el (project--find-default-from): Add.
(project-find-file): Use project--find-default-from with
buffer-file-name.
(project-or-external-find-file): Use project--find-default-from with
buffer-file-name.
(project-find-dir): Use project--find-default-from with
default-directory.
---
 lisp/progmodes/project.el | 45 +++++++++++++++++++++++++++------------
 1 file changed, 31 insertions(+), 14 deletions(-)

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index e0f1f995ff2..e0d05d45a55 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -989,6 +989,23 @@ project--read-regexp
     (read-regexp "Find regexp" (and sym (regexp-quote sym))
                  project-regexp-history-variable)))
 
+(defun project--find-default-from (filename project)
+  "Ensure FILENAME is in PROJECT.
+
+Usually, just return FILENAME.  But if
+`project-current-directory-override' is set, FILENAME is probably
+relative to that project; adjust it to be relative to PROJECT instead.
+
+This supports using a relative file name from the current buffer
+when switching projects with `project-switch-project' and then
+using a command like `project-find-file'."
+  (if-let (filename-proj (and project-current-directory-override
+                            (project-current nil default-directory)))
+      ;; file-name-concat requires Emacs 28+
+      (concat (file-name-as-directory (project-root project))
+              (file-relative-name filename (project-root filename-proj)))
+    filename))
+
 ;;;###autoload
 (defun project-find-file (&optional include-all)
   "Visit a file (with completion) in the current project.
@@ -1006,16 +1023,7 @@ project-find-file
          (dirs (list root)))
     (project-find-file-in
      (or (thing-at-point 'filename)
-         (and buffer-file-name
-              (if-let (buffer-proj (and project-current-directory-override
-                                        (project-current nil default-directory)))
-                  ;; Allow using the relative file name of the current
-                  ;; buffer in "other project" as well.
-                  (let ((buffer-root (project-root buffer-proj)))
-                    ;; file-name-concat requires Emacs 28+
-                    (concat (file-name-as-directory root)
-                            (file-relative-name buffer-file-name buffer-root)))
-                buffer-file-name)))
+         (and buffer-file-name (project--find-default-from buffer-file-name pr)))
      dirs pr include-all)))
 
 ;;;###autoload
@@ -1023,7 +1031,8 @@ project-or-external-find-file
   "Visit a file (with completion) in the current project or external roots.
 
 The filename at point (determined by `thing-at-point'), if any,
-is available as part of \"future history\".
+is available as part of \"future history\".  If none, the current
+buffer's file name is used.
 
 If INCLUDE-ALL is non-nil, or with prefix argument when called
 interactively, include all files under the project root, except
@@ -1033,7 +1042,10 @@ project-or-external-find-file
          (dirs (cons
                 (project-root pr)
                 (project-external-roots pr))))
-    (project-find-file-in (thing-at-point 'filename) dirs pr include-all)))
+    (project-find-file-in
+     (or (thing-at-point 'filename)
+         (and buffer-file-name (project--find-default-from buffer-file-name pr)))
+     dirs pr include-all)))
 
 (defcustom project-read-file-name-function #'project--read-file-cpd-relative
   "Function to call to read a file name from a list.
@@ -1185,7 +1197,10 @@ project--completing-read-strict
 
 ;;;###autoload
 (defun project-find-dir ()
-  "Start Dired in a directory inside the current project."
+  "Start Dired in a directory inside the current project.
+
+The current buffer's `default-directory' is available as part of
+\"future history\"."
   (interactive)
   (let* ((project (project-current t))
          (all-files (project-files project))
@@ -1200,7 +1215,9 @@ project-find-dir
                project "Dired"
                ;; Some completion UIs show duplicates.
                (delete-dups all-dirs)
-               nil 'file-name-history)))
+               nil 'file-name-history
+               (and default-directory
+                    (project--find-default-from default-directory project)))))
     (dired dir)))
 
 ;;;###autoload
-- 
2.39.3

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

[Spencer Baugh]

BTW, one more feature in this vein (stealing this idea from Stefan)
would be if we automatically moved point to the same location in the
other file.  That might be a little too magical.  But it would be very
cool...

I guess the ideal thing we'd want is to move point to the same function,
which is a bit trickier...  could be done with imenu perhaps.  Not
sure...

Maybe the right call would be to have a keybinding in C-x p p like j or
something, which would just instantly jump you to the same file in the
other project.  So you'd just run C-x p p j and that would open the same
file in the other project, with point inside the same function (using
imenu), at the same offset in that function.

That could be helpful for other reasons too: I've often wanted "just put
me anywhere in this other project, I don't care where", and this could
be that command.  Although I suppose mostly I want that because C-x p p
isn't currently a generic prefix for any command, and if we convert it
to be that (with next-default-directory or something), I won't need
that.

Alternatively, maybe C-x p j could be an alternative to C-x p p, and
when it prompts for a project, it could prompt only for "sibling
projects" which have the same file structure.  And we could have a
built-in way to detect sibling projects: Any other worktree of the
current git repository is a sibling project.  (And we would make this
extensible too of course; maybe have both project-siblings and
vc-list-worktrees as extension points)

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

[Dmitry Gutov]

On 17/08/2023 22:41, Spencer Baugh wrote:
> Dmitry Gutov <dmitry@gutov.dev> writes:
>> -         (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))))
> 
> This seems good, sure.  But doesn't this make the history entries appear
> twice?

It doesn't, since whatever modification to file-name-history is done 
inside project-read-file-name-function is erased when the surrounding 
'let' form (pre-altering its value) returns.

> Maybe we should just pull the history-adding functionality out of
> project-read-file-name-function entirely.  I've tried doing that below.

Just for project-find-dir, right? That makes sense.

> Also, I realized just now that this should probably affect
> project-find-dir as well, as should my previous patch adding
> project-relative future history.  (I actually coincidentally just now
> got a user request for "switch between projects and stay in the same
> dir")

^^

> So here's a revised version of this history change which also affects
> project-find-dir.  In a subsequent mail I'll send a patch for the
> "future history" behavior of project-find-dir too.  (yet to be written)

That ones looks good too (I'll go over the cosmetics a little later).

Regarding project-file-name-history-relativize, I wanted to ask about a 
shorter name, but... it seems like there aren't many to be had.

Also originally I wanted to just enable the feature and then see what 
actual modifications people will want. Perhaps some will ask for 
find-file and project-find-file histories to be totally separate 
instead? Or maybe not.

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

[Dmitry Gutov]

On 18/08/2023 23:57, Spencer Baugh wrote:
> 
> BTW, one more feature in this vein (stealing this idea from Stefan)
> would be if we automatically moved point to the same location in the
> other file.  That might be a little too magical.  But it would be very
> cool...
> 
> I guess the ideal thing we'd want is to move point to the same function,
> which is a bit trickier...  could be done with imenu perhaps.  Not
> sure...

We don't know how much these files could be different. One could be 
empty (a newly-created one), or having totally different contents.

There is a way to detect a useful offset if the files are similar enough 
(using 'diff -u''s output), I think we have that in diff-hl. But calling 
'diff' when any file is visited seems like it will be minority 
preference... I think.

> Maybe the right call would be to have a keybinding in C-x p p like j or
> something, which would just instantly jump you to the same file in the
> other project.  So you'd just run C-x p p j and that would open the same
> file in the other project, with point inside the same function (using
> imenu), at the same offset in that function.

Sure, why not. As a part of your personal (or company-wise) settings?

> That could be helpful for other reasons too: I've often wanted "just put
> me anywhere in this other project, I don't care where", and this could
> be that command.  Although I suppose mostly I want that because C-x p p
> isn't currently a generic prefix for any command, and if we convert it
> to be that (with next-default-directory or something), I won't need
> that.

Just in case you were not aware: project-switch-commands can also be set 
to a single symbol, then just that command will be invoked.

> Alternatively, maybe C-x p j could be an alternative to C-x p p, and
> when it prompts for a project, it could prompt only for "sibling
> projects" which have the same file structure.  And we could have a
> built-in way to detect sibling projects: Any other worktree of the
> current git repository is a sibling project.  (And we would make this
> extensible too of course; maybe have both project-siblings and
> vc-list-worktrees as extension points)

I'm not convinced yet by the idea of 'project-siblings' generic (need 
more uses/users), but the rest sounds good and very doable anyway.

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

[sbaugh]

Dmitry Gutov <dmitry@gutov.dev> writes:

> On 17/08/2023 22:41, Spencer Baugh wrote:
>> Dmitry Gutov <dmitry@gutov.dev> writes:
>>> -         (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))))
>> This seems good, sure.  But doesn't this make the history entries
>> appear
>> twice?
>
> It doesn't, since whatever modification to file-name-history is done
> inside project-read-file-name-function is erased when the surrounding
> 'let' form (pre-altering its value) returns.
>
>> Maybe we should just pull the history-adding functionality out of
>> project-read-file-name-function entirely.  I've tried doing that below.
>
> Just for project-find-dir, right? That makes sense.
>
>> Also, I realized just now that this should probably affect
>> project-find-dir as well, as should my previous patch adding
>> project-relative future history.  (I actually coincidentally just now
>> got a user request for "switch between projects and stay in the same
>> dir")
>
> ^^
>
>> So here's a revised version of this history change which also affects
>> project-find-dir.  In a subsequent mail I'll send a patch for the
>> "future history" behavior of project-find-dir too.  (yet to be written)
>
> That ones looks good too (I'll go over the cosmetics a little later).
>
> Regarding project-file-name-history-relativize, I wanted to ask about
> a shorter name, but... it seems like there aren't many to be had.
>
> Also originally I wanted to just enable the feature and then see what
> actual modifications people will want. Perhaps some will ask for
> find-file and project-find-file histories to be totally separate
> instead? Or maybe not.

Sure, if it's something you think is reasonable to enable by default
that's totally fine for me.

A modification that makes some sense to me (although it's hard) is
actually to merge find-file and project-find-file history *more*.  Right
now a path in the history can only be adjusted for the current project
if it was originally added to the history by project-find-file.  It
might be nice if the adjustment worked for paths added by find-file too,
although that is tricky to do efficiently since they don't (yet?) have
the project embedded in them with a text property.

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

[Juri Linkov]

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

and

> +      (add-to-history 'file-name-history
> +                      (propertize file 'project (project-root project))))

Then maybe better to simply add only the relative file name for optimization?
Like this

         (add-to-history 'file-name-history
                         (propertize file 'project-relative-name
                          (file-relative-name file (project-root project)))))

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

[Juri Linkov]

> +  (if-let (filename-proj (and project-current-directory-override
> +                            (project-current nil default-directory)))
> +      ;; file-name-concat requires Emacs 28+
> +      (concat (file-name-as-directory (project-root project))
> +              (file-relative-name filename (project-root filename-proj)))
> +    filename))

I wonder why you prepend (file-name-as-directory (project-root project))?
This is unnecessary because the project root is already displayed
in the prompt, so only a relative file name is sufficient in the minibuffer:

  Find file in /project/root/: relative/file.name

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

[Juri Linkov]

> +(defcustom project-file-name-history-relativize nil
> +  "If non-nil, paths in `file-name-history' are adjusted for the current project.
> +
> +When non-nil and in `project-find-file' or `project-find-dir',
> +paths in `file-name-history' are adjusted to be relative to
> +whatever the current project is, instead of the project which
> +added those paths.  This only affects history entries added by
> +earlier calls to `project-find-file' or `project-find-dir'.
> +
> +When `project-read-file-name-function' is
> +`project--read-file-cpd-relative' (the default), this has the
> +effect of sharing more history between projects.")

Instead of, or in addition to 'project-file-name-history-relativize',
another option may be needed to define whether to check the
existence of the constructed file name.  This could help
to filter out irrelevant file names constructed from
different projects.

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

[Juri Linkov]

> BTW, one more feature in this vein (stealing this idea from Stefan)
> would be if we automatically moved point to the same location in the
> other file.  That might be a little too magical.  But it would be very
> cool...

One possible implementation would be with the help of saveplace.el.
There are already some similarities between these two packages:
for example, the same fix that you proposed in bug#64088
to use file-remote-p before abbreviating project file names,
should be applied to save-place-abbreviation-file-names
in bug#65055 as well.

> Maybe the right call would be to have a keybinding in C-x p p like j or
> something, which would just instantly jump you to the same file in the
> other project.  So you'd just run C-x p p j and that would open the same
> file in the other project, with point inside the same function (using
> imenu), at the same offset in that function.

A dedicated command with 'C-x p p j' or just 'C-x p j' that will ask
for another project would be the best thing to do.

> Alternatively, maybe C-x p j could be an alternative to C-x p p, and
> when it prompts for a project, it could prompt only for "sibling
> projects" which have the same file structure.  And we could have a
> built-in way to detect sibling projects: Any other worktree of the
> current git repository is a sibling project.  (And we would make this
> extensible too of course; maybe have both project-siblings and
> vc-list-worktrees as extension points)

For switching to a sibling file in another project, I'm using a new
nice command 'find-sibling-file'.  Extending it specifically for projects
looks line a good direction for development.

> That could be helpful for other reasons too: I've often wanted "just put
> me anywhere in this other project, I don't care where", and this could
> be that command.  Although I suppose mostly I want that because C-x p p
> isn't currently a generic prefix for any command, and if we convert it
> to be that (with next-default-directory or something), I won't need
> that.

Probably next-default-directory will reuse the same keymaps,
but let's see where it goes in bug#63648.

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

[Dmitry Gutov]

On 20/08/2023 20:16, Juri Linkov wrote:
>> +  (if-let (filename-proj (and project-current-directory-override
>> +                            (project-current nil default-directory)))
>> +      ;; file-name-concat requires Emacs 28+
>> +      (concat (file-name-as-directory (project-root project))
>> +              (file-relative-name filename (project-root filename-proj)))
>> +    filename))
> 
> I wonder why you prepend (file-name-as-directory (project-root project))?
> This is unnecessary because the project root is already displayed
> in the prompt, so only a relative file name is sufficient in the minibuffer:
> 
>    Find file in /project/root/: relative/file.name

The absolute name is required down in the callee because that 
distinguishes it from a file name fragment picked up by things-at-point.

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

[Dmitry Gutov]

On 20/08/2023 20:13, Juri Linkov wrote:
> Then maybe better to simply add only the relative file name for optimization?
> Like this
> 
>           (add-to-history 'file-name-history
>                           (propertize file 'project-relative-name
>                            (file-relative-name file (project-root project)))))

Then it won't work in find-file (or might even break it).

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

[Dmitry Gutov]

On 20/08/2023 20:20, Juri Linkov wrote:
>> +(defcustom project-file-name-history-relativize nil
>> +  "If non-nil, paths in `file-name-history' are adjusted for the current project.
>> +
>> +When non-nil and in `project-find-file' or `project-find-dir',
>> +paths in `file-name-history' are adjusted to be relative to
>> +whatever the current project is, instead of the project which
>> +added those paths.  This only affects history entries added by
>> +earlier calls to `project-find-file' or `project-find-dir'.
>> +
>> +When `project-read-file-name-function' is
>> +`project--read-file-cpd-relative' (the default), this has the
>> +effect of sharing more history between projects.")
> Instead of, or in addition to 'project-file-name-history-relativize',
> another option may be needed to define whether to check the
> existence of the constructed file name.  This could help
> to filter out irrelevant file names constructed from
> different projects.

It could indeed. But it could also prevent the user from easily creating 
a new file with the name from a "sister" project.

We've discussed this before in this thread, including the idea of some 
predicate checking the projects for compatibility, etc. What behavior 
would you actually choose yourself?

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

[Dmitry Gutov]

On 19/08/2023 15:00, sbaugh@catern.com wrote:

>> Regarding project-file-name-history-relativize, I wanted to ask about
>> a shorter name, but... it seems like there aren't many to be had.
>>
>> Also originally I wanted to just enable the feature and then see what
>> actual modifications people will want. Perhaps some will ask for
>> find-file and project-find-file histories to be totally separate
>> instead? Or maybe not.
> 
> Sure, if it's something you think is reasonable to enable by default
> that's totally fine for me.

I'm being a tad skittish about it because once we add the var, it will 
likely have to stay around for a long time. And a long name is both 
unwieldy and less prone to extensibility.

One of the ways to make it shorter, is to thing around different 
non-intersecting behaviors that could be enabled by it. If e.g. it could 
have values "default" (don't do anything) and "relativize" (and ... 
"relativize existing"? as Juri brought up), then the var could be called

   project-file-history-behavior

with values t, 'relativize and 'relativize-existing. Or something like 
that. We could still drop the option for now and enable the new behavior 
by default, unless somebody objects.

> A modification that makes some sense to me (although it's hard) is
> actually to merge find-file and project-find-file history *more*.  Right
> now a path in the history can only be adjusted for the current project
> if it was originally added to the history by project-find-file.  It
> might be nice if the adjustment worked for paths added by find-file too,
> although that is tricky to do efficiently since they don't (yet?) have
> the project embedded in them with a text property.

I don't know, it seems like we'd do extra work up front, going through 
file-name-history, while most people won't take advantage of it. If we 
could do that lazily (with a generator function of some sort), that 
would of course be preferable. As it is, though, one could just run a 
small script once, and convert all the entries to use later.

What's the scenario when this doesn't work? People using 'find-file' to 
quickly jump to a file in the same dir and then wanting to use it in 
history during project-find-file in another project?

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

[Juri Linkov]

>> Then maybe better to simply add only the relative file name for optimization?
>> Like this
>>           (add-to-history 'file-name-history
>>                           (propertize file 'project-relative-name
>>                            (file-relative-name file (project-root project)))))
>
> Then it won't work in find-file (or might even break it).

But find-file doesn't use the property 'project-relative-name'.

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

[Juri Linkov]

>>> +(defcustom project-file-name-history-relativize nil
>>> +  "If non-nil, paths in `file-name-history' are adjusted for the current project.
>>> +
>> Instead of, or in addition to 'project-file-name-history-relativize',
>> another option may be needed to define whether to check the
>> existence of the constructed file name.  This could help
>> to filter out irrelevant file names constructed from
>> different projects.
>
> It could indeed. But it could also prevent the user from easily creating
> a new file with the name from a "sister" project.

It would be unfortunate to lose this useful feature.

> We've discussed this before in this thread, including the idea of some
> predicate checking the projects for compatibility, etc. What behavior would
> you actually choose yourself?

I guess introducing a notion of "sibling projects" is unavoiable
to exclude such situations where relative file names from one project
are proposed in an unrelated project:

  Find file in /project/root/: relative/filename/from/unrelated/project

Then one possibility is to add an option to define sibling projects.
Like 'find-sibling-rules', but maybe simply to contain alists of
sibling projects.  Or projects in 'project--list' could have
a new property 'group' where a project belongs to.

Then sibling projects could share the common file history.

And a new command e.g. 'project-find-sibling-file' could
jump to a sibling project file immediately in case of
two sibling projects, or ask to select with completion
for more projects.

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

[Dmitry Gutov]

On 21/08/2023 09:58, Juri Linkov wrote:
>>> Then maybe better to simply add only the relative file name for optimization?
>>> Like this
>>>            (add-to-history 'file-name-history
>>>                            (propertize file 'project-relative-name
>>>                             (file-relative-name file (project-root project)))))
>> Then it won't work in find-file (or might even break it).
> But find-file doesn't use the property 'project-relative-name'.

Yes, it just uses the absolute name. It won't be able to use the 
relative one.

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

[Dmitry Gutov]

On 21/08/2023 10:06, Juri Linkov wrote:
>>>> +(defcustom project-file-name-history-relativize nil
>>>> +  "If non-nil, paths in `file-name-history' are adjusted for the current project.
>>>> +
>>> Instead of, or in addition to 'project-file-name-history-relativize',
>>> another option may be needed to define whether to check the
>>> existence of the constructed file name.  This could help
>>> to filter out irrelevant file names constructed from
>>> different projects.
>>
>> It could indeed. But it could also prevent the user from easily creating
>> a new file with the name from a "sister" project.
> 
> It would be unfortunate to lose this useful feature.
> 
>> We've discussed this before in this thread, including the idea of some
>> predicate checking the projects for compatibility, etc. What behavior would
>> you actually choose yourself?
> 
> I guess introducing a notion of "sibling projects" is unavoiable
> to exclude such situations where relative file names from one project
> are proposed in an unrelated project:
> 
>    Find file in /project/root/: relative/filename/from/unrelated/project
> 
> Then one possibility is to add an option to define sibling projects.
> Like 'find-sibling-rules', but maybe simply to contain alists of
> sibling projects.  Or projects in 'project--list' could have
> a new property 'group' where a project belongs to.
> 
> Then sibling projects could share the common file history.
> 
> And a new command e.g. 'project-find-sibling-file' could
> jump to a sibling project file immediately in case of
> two sibling projects, or ask to select with completion
> for more projects.

That would require a new generic (project-sibling-projects) and a proper 
description of what a "sibling" project is supposed to be. And probably 
still not going to work for VC-aware backend OOTB (it'll require some 
backend specific hook for the users to define that logic).

Sounds a little complicated, so given that one can still reach almost 
the same functionality through history completion ('C-x p p ... RET f 
C-n RET'), maybe it's a bit of an overkill? I'm open to further 
arguments, though.

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

[Dmitry Gutov]

On 17/08/2023 23:12, Spencer Baugh wrote:
> Spencer Baugh<sbaugh@janestreet.com>  writes:
>> In a subsequent mail I'll send a patch for the "future history"
>> behavior of project-find-dir too.  (yet to be written)
> Here is that.  Seems like a straightforward generalization, and
> something that a user would expect to work.

Thanks! Installed as-is, with a small rewrite of the commit message.

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

[Dmitry Gutov]

On 17/08/2023 22:41, Spencer Baugh wrote:
> Also, I realized just now that this should probably affect
> project-find-dir as well, as should my previous patch adding
> project-relative future history.  (I actually coincidentally just now
> got a user request for "switch between projects and stay in the same
> dir")
> 
> So here's a revised version of this history change which also affects
> project-find-dir.  In a subsequent mail I'll send a patch for the
> "future history" behavior of project-find-dir too.  (yet to be written)

Installed, with a few alterations:

- project--expand-file-name -> project--transplant-file-name (seems more 
explicit for understanding)
- The new option renamed to project-file-history-behavior with values t 
or 'relativize. I thought about removing it, but after all, the change 
is a bit exotic, so there's bound to be people who would want to disable 
it. And the new name is also more extensible (extra behaviors I could 
think of by now: 'relativize-when-exists or 'separate -- the latter 
could mean to use separate history var other than file-name-history). No 
hurry to implement any of those, though.
- project-or-external-find-file needs some special handling of the 
relativization when external file names are chosen. Better solutions 
welcome.
- Announcement in NEWS. :-)

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

[Juri Linkov]

> - The new option renamed to project-file-history-behavior with values t or
>   'relativize. I thought about removing it, but after all, the change is
>   a bit exotic, so there's bound to be people who would want to disable
>   it. And the new name is also more extensible (extra behaviors I could
>   think of by now: 'relativize-when-exists or 'separate -- the latter could
>   mean to use separate history var other than file-name-history). No hurry
>  to implement any of those, though.
> - project-or-external-find-file needs some special handling of the
>   relativization when external file names are chosen. Better solutions
>   welcome.
> - Announcement in NEWS. :-)

A typo in NEWS?  'relative' -> 'relativize'

Also to reduce confusion for everyone who will look at it,
better to rename the property to 'project-root' in:

    (propertize file 'project (project-root project))))

PS: The docstring mentions the limitation: "This only affects
history entries added by earlier calls to `project-find-file'".
There is no way to remove this limitation?  Maybe some clever way
to match every file name from the history against the list
of all known project roots to find the root on the file name
without property.  This will also work when the file history
is restored from the desktop file or by savehist.el.

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

[Dmitry Gutov]

On 23/08/2023 20:52, Juri Linkov wrote:
>> - The new option renamed to project-file-history-behavior with values t or
>>    'relativize. I thought about removing it, but after all, the change is
>>    a bit exotic, so there's bound to be people who would want to disable
>>    it. And the new name is also more extensible (extra behaviors I could
>>    think of by now: 'relativize-when-exists or 'separate -- the latter could
>>    mean to use separate history var other than file-name-history). No hurry
>>   to implement any of those, though.
>> - project-or-external-find-file needs some special handling of the
>>    relativization when external file names are chosen. Better solutions
>>    welcome.
>> - Announcement in NEWS. :-)
> 
> A typo in NEWS?  'relative' -> 'relativize'

Fixed, thanks.

> Also to reduce confusion for everyone who will look at it,
> better to rename the property to 'project-root' in:
> 
>      (propertize file 'project (project-root project))))

It seemed shorter and just as obvious when looking at the propertized 
value. But if everyone thinks the change should be made, I don't mind.

> PS: The docstring mentions the limitation: "This only affects
> history entries added by earlier calls to `project-find-file'".
> There is no way to remove this limitation?  Maybe some clever way
> to match every file name from the history against the list
> of all known project roots to find the root on the file name
> without property.  This will also work when the file history
> is restored from the desktop file or by savehist.el.

That is doable, at the cost of having imprecise results (and some 
odd-looking history entries from time to time). Is that cost low enough?