bug#69584: 29.2.50; project-find-functions should have access to maybe-prompt

bug#69584: 29.2.50; project-find-functions should have access to maybe-prompt

[Spencer Baugh]

I'd like to write a project-find-function which might prompt when
called, and so to suppress that prompting I'd like to be able to check
the maybe-prompt argument that project-current received.

Possible new functions for project-find-functions which would benefit
from this:

- A local project-find-function in a version control buffer for viewing
a branch log; if the branch is not currently checked out, prompt to
check out that branch (or create a worktree for it) before returning the
project

- A local project-find-function in a buffer from a package for Git
forges; if the buffer corresponds to a repository which is not currently
cloned locally, prompt to clone the repository.

These behaviors should of course be suppressed if maybe-prompt is nil,
which is why it would be nice to be able to access maybe-prompt.

Since adding a new argument to project-find-functions is hard, maybe we
could do this by introducing a new dynamic variable
project-find-functions-may-prompt which we let-bind?  Like:

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index c7c07c3d34c..3975182b88d 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -242,8 +242,9 @@ project-current
         (setq pr (cons 'transient directory))))
     pr))
 
-(defun project--find-in-directory (dir)
-  (run-hook-with-args-until-success 'project-find-functions dir))
+(defun project--find-in-directory (dir &optional maybe-prompt)
+  (let ((project-find-functions-may-prompt maybe-prompt))
+    (run-hook-with-args-until-success 'project-find-functions dir)))
 
 (defvar project--within-roots-fallback nil)
 


In GNU Emacs 29.2.50 (build 4, x86_64-pc-linux-gnu, X toolkit, cairo
 version 1.15.12, Xaw scroll bars) of 2024-02-28 built on
 igm-qws-u22796a
Repository revision: 46e23709d37943a20faa735c97af520196a443e9
Repository branch: emacs-29
Windowing system distributor 'The X.Org Foundation', version 11.0.12011000
System Description: Rocky Linux 8.9 (Green Obsidian)

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

Configured features:
CAIRO DBUS FREETYPE GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG JSON
LIBSELINUX LIBSYSTEMD 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#69584: 29.2.50; project-find-functions should have access to maybe-prompt

[Dmitry Gutov]

Hi Spencer,

On 06/03/2024 16:23, Spencer Baugh wrote:
> 
> I'd like to write a project-find-function which might prompt when
> called, and so to suppress that prompting I'd like to be able to check
> the maybe-prompt argument that project-current received.

This is technically doable, but what looks worrying to me, is that this 
would make 'project-current' lose more of its idempotency.

Originally, the DIRECTORY argument for it was intended to make sure that 
all calls from the same directory would return the same instance. But of 
course a hook can be local, so some can find it practical to define 
buffer-local projects. This side-steps the original design, making, for 
example, the relation "files A and B are in the same project" not 
necessarily symmetric or transitive.

> Possible new functions for project-find-functions which would benefit
> from this:
> 
> - A local project-find-function in a version control buffer for viewing
> a branch log; if the branch is not currently checked out, prompt to
> check out that branch (or create a worktree for it) before returning the
> project
> 
> - A local project-find-function in a buffer from a package for Git
> forges; if the buffer corresponds to a repository which is not currently
> cloned locally, prompt to clone the repository.
> 
> These behaviors should of course be suppressed if maybe-prompt is nil,
> which is why it would be nice to be able to access maybe-prompt.

What would such project functions return when maybe-prompt is nil? Also 
nil, right? That would kind of create a separate category of projects, 
interactive-only.

With regards to caching, for example, if some caller wanted to do so 
(some related discussion: 
https://github.com/joaotavora/breadcrumb/issues/18#issuecomment-1984615275), 
then would also need to take this parameter into account.

So the first thing I'd ask is whether you see a different way to 
implement the same features. I don't see the whole usage scenarios, so 
it's hard to judge.

> Since adding a new argument to project-find-functions is hard, maybe we
> could do this by introducing a new dynamic variable
> project-find-functions-may-prompt which we let-bind?  Like:
> 
> diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
> index c7c07c3d34c..3975182b88d 100644
> --- a/lisp/progmodes/project.el
> +++ b/lisp/progmodes/project.el
> @@ -242,8 +242,9 @@ project-current
>           (setq pr (cons 'transient directory))))
>       pr))
>   
> -(defun project--find-in-directory (dir)
> -  (run-hook-with-args-until-success 'project-find-functions dir))
> +(defun project--find-in-directory (dir &optional maybe-prompt)
> +  (let ((project-find-functions-may-prompt maybe-prompt))
> +    (run-hook-with-args-until-success 'project-find-functions dir)))
>   
>   (defvar project--within-roots-fallback nil)

As far as the implementation goes, a dynamic variable can work. We could 
also try reusing an existing one: non-essential, and we'd set it to nil 
when maybe-prompt is non-nil.

I wonder how it would interact with Tramp (ask for password in 
disconnected buffers?), but that seems to fall into the same general 
category as your other cases.

bug#69584: 29.2.50; project-find-functions should have access to maybe-prompt

[sbaugh]

Dmitry Gutov <dmitry@gutov.dev> writes:
> Hi Spencer,
>
> On 06/03/2024 16:23, Spencer Baugh wrote:
>> I'd like to write a project-find-function which might prompt when
>> called, and so to suppress that prompting I'd like to be able to check
>> the maybe-prompt argument that project-current received.
>
> This is technically doable, but what looks worrying to me, is that
> this would make 'project-current' lose more of its idempotency.
>
> Originally, the DIRECTORY argument for it was intended to make sure
> that all calls from the same directory would return the same
> instance. But of course a hook can be local, so some can find it
> practical to define buffer-local projects. This side-steps the
> original design, making, for example, the relation "files A and B are
> in the same project" not necessarily symmetric or transitive.

True, that's annoying.

Here's an alternative implementation: Maybe we could have a new
project-guess-functions hook which is only run when maybe-prompt=t.  And
these functions should return either nil, or a directory in which to
find a project.  And they're allowed to prompt or error or whatever.

And project-guess-functions would only be called if a DIRECTORY wasn't
provided to project-current.  (And maybe only if default-directory is
nil?  I'm indifferent about whether project-guess-functions or
default-directory takes precedence)

Then project-find-functions could still be globally the same, so
project-current would always return the same result for the same
DIRECTORY.

>> Possible new functions for project-find-functions which would benefit
>> from this:
>> - A local project-find-function in a version control buffer for
>> viewing
>> a branch log; if the branch is not currently checked out, prompt to
>> check out that branch (or create a worktree for it) before returning the
>> project
>> - A local project-find-function in a buffer from a package for Git
>> forges; if the buffer corresponds to a repository which is not currently
>> cloned locally, prompt to clone the repository.
>> These behaviors should of course be suppressed if maybe-prompt is
>> nil,
>> which is why it would be nice to be able to access maybe-prompt.
>
> What would such project functions return when maybe-prompt is nil?
> Also nil, right? That would kind of create a separate category of
> projects, interactive-only.

They would always return nil when maybe-prompt is nil.  But when they
return something, it would be a normal vc project.

> With regards to caching, for example, if some caller wanted to do so
> (some related discussion:
> https://github.com/joaotavora/breadcrumb/issues/18#issuecomment-1984615275),
> then would also need to take this parameter into account.

True, but it's already not correct to cache when maybe-prompt=t, right?
Because the returned project may just be the one that the user choose
interactively at the prompt.

> So the first thing I'd ask is whether you see a different way to
> implement the same features. I don't see the whole usage scenarios, so
> it's hard to judge.

Let me give some context about my concrete use case.

At Jane Street we have a code review system built on top of Emacs,
called FE.  A user opens a code review in a buffer in Emacs, and can see
that review's diff.  If they want to comment on the review, or build the
code or test it or anything like that, they need to also have a local
working copy of the code in the review.  The local working copy for each
review is kept separately, like git worktrees.

(There are some other details and screenshots in
https://blog.janestreet.com/putting-the-i-back-in-ide-towards-a-github-explorer/
but this should suffice)

So my use case then is this: when a user opens code review FE-123 in a
buffer, they look at the diff and then decide they want to do something
in a working copy of the code.  Currently, to do that they run one of a
variety of internal commands which duplicate things like
project-find-file, but which are aware of whether or not there's a local
working copy, and operate the local working copy if any, and otherwise
prompt to create a local working copy and then error.

I'd like to replace those internal commands with just normal
project-find-file, and also allow other commands which use
project-current to determine the current project to just work.

(Now that I've framed it this way, I guess a good comparison is to
debbugs.  It might be cool if, when you run a project command from a
debbugs buffer, you land in a worktree which has the changes from (any?)
patches in that bug.  Although that's obviously quite a bit harder since
you need to find the patch in the thread, and find the base branch to
apply it to.)

>> Since adding a new argument to project-find-functions is hard, maybe we
>> could do this by introducing a new dynamic variable
>> project-find-functions-may-prompt which we let-bind?  Like:
>> diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
>> index c7c07c3d34c..3975182b88d 100644
>> --- a/lisp/progmodes/project.el
>> +++ b/lisp/progmodes/project.el
>> @@ -242,8 +242,9 @@ project-current
>>           (setq pr (cons 'transient directory))))
>>       pr))
>>   -(defun project--find-in-directory (dir)
>> -  (run-hook-with-args-until-success 'project-find-functions dir))
>> +(defun project--find-in-directory (dir &optional maybe-prompt)
>> +  (let ((project-find-functions-may-prompt maybe-prompt))
>> +    (run-hook-with-args-until-success 'project-find-functions dir)))
>>     (defvar project--within-roots-fallback nil)
>
> As far as the implementation goes, a dynamic variable can work. We
> could also try reusing an existing one: non-essential, and we'd set it
> to nil when maybe-prompt is non-nil.
>
> I wonder how it would interact with Tramp (ask for password in
> disconnected buffers?), but that seems to fall into the same general
> category as your other cases.

Nice idea, it does seem like we should probably already be binding
non-essential=t around project-find-functions when maybe-prompt is nil.
Since already TRAMP can cause prompting even when a programmer calls
project-current with maybe-prompt=nil.

bug#69584: 29.2.50; project-find-functions should have access to maybe-prompt

[Dmitry Gutov]

On 16/03/2024 15:31, sbaugh@catern.com wrote:

>> This is technically doable, but what looks worrying to me, is that
>> this would make 'project-current' lose more of its idempotency.
>>
>> Originally, the DIRECTORY argument for it was intended to make sure
>> that all calls from the same directory would return the same
>> instance. But of course a hook can be local, so some can find it
>> practical to define buffer-local projects. This side-steps the
>> original design, making, for example, the relation "files A and B are
>> in the same project" not necessarily symmetric or transitive.
> 
> True, that's annoying.
> 
> Here's an alternative implementation: Maybe we could have a new
> project-guess-functions hook which is only run when maybe-prompt=t.  And
> these functions should return either nil, or a directory in which to
> find a project.  And they're allowed to prompt or error or whatever.
> 
> And project-guess-functions would only be called if a DIRECTORY wasn't
> provided to project-current.  (And maybe only if default-directory is
> nil?  I'm indifferent about whether project-guess-functions or
> default-directory takes precedence)
> 
> Then project-find-functions could still be globally the same, so
> project-current would always return the same result for the same
> DIRECTORY.

That still sounds like the same semantics change in 'project-current', 
something that would reflect on its callers.

>>> Possible new functions for project-find-functions which would benefit
>>> from this:
>>> - A local project-find-function in a version control buffer for
>>> viewing
>>> a branch log; if the branch is not currently checked out, prompt to
>>> check out that branch (or create a worktree for it) before returning the
>>> project
>>> - A local project-find-function in a buffer from a package for Git
>>> forges; if the buffer corresponds to a repository which is not currently
>>> cloned locally, prompt to clone the repository.
>>> These behaviors should of course be suppressed if maybe-prompt is
>>> nil,
>>> which is why it would be nice to be able to access maybe-prompt.
>>
>> What would such project functions return when maybe-prompt is nil?
>> Also nil, right? That would kind of create a separate category of
>> projects, interactive-only.
> 
> They would always return nil when maybe-prompt is nil.  But when they
> return something, it would be a normal vc project.

All right, that's what I thought.

>> With regards to caching, for example, if some caller wanted to do so
>> (some related discussion:
>> https://github.com/joaotavora/breadcrumb/issues/18#issuecomment-1984615275),
>> then would also need to take this parameter into account.
> 
> True, but it's already not correct to cache when maybe-prompt=t, right?
> Because the returned project may just be the one that the user choose
> interactively at the prompt.

It's... a good point, but so far the main exception was the "transient" 
project, for which one could make an exception somehow, or even cache 
without major downsides, as long as buffer stays the same and the cache 
interval is low.

Disabling cache altogether with maybe-prompt=t might be a net negative, 
given that many users' interaction with project.el might be limited to 
commands that only do such invocations. But perhaps it's the price to 
pay for flexibility: as long as we're talking about external cache, it 
will be up to the callers to avoid caching where the results can be 
non-deterministic, such as after a prompt.

>> So the first thing I'd ask is whether you see a different way to
>> implement the same features. I don't see the whole usage scenarios, so
>> it's hard to judge.
> 
> Let me give some context about my concrete use case.
> 
> At Jane Street we have a code review system built on top of Emacs,
> called FE.  A user opens a code review in a buffer in Emacs, and can see
> that review's diff.  If they want to comment on the review, or build the
> code or test it or anything like that, they need to also have a local
> working copy of the code in the review.  The local working copy for each
> review is kept separately, like git worktrees.
> 
> (There are some other details and screenshots in
> https://blog.janestreet.com/putting-the-i-back-in-ide-towards-a-github-explorer/
> but this should suffice)
> 
> So my use case then is this: when a user opens code review FE-123 in a
> buffer, they look at the diff and then decide they want to do something
> in a working copy of the code.  Currently, to do that they run one of a
> variety of internal commands which duplicate things like
> project-find-file, but which are aware of whether or not there's a local
> working copy, and operate the local working copy if any, and otherwise
> prompt to create a local working copy and then error.
> 
> I'd like to replace those internal commands with just normal
> project-find-file, and also allow other commands which use
> project-current to determine the current project to just work.

If you set up a project instance in a buffer-local way, would it even 
work correctly outside of that buffer? Would project history work fine? 
When you pick a project from recently visited, you basically just apply 
its last root directory and expect the project to be "found".

I've read the article (thanks!), but I'm not sure yet of what would be 
the ergonomic savings in such scenario when instead of having a separate 
command to check out a branch and visit a file in it (perhaps bound to 
'o f' inside the major mode map for the branches list's buffer), you 
call project-find-file right away. In the former scenario such command 
would make sure the branch is checked out, so its directory has proper 
contents, and then it could delegate to project-find-file inside said 
directory. And later visits (e.g. from project-switch-project) would 
work fine until the directory is deleted.

> (Now that I've framed it this way, I guess a good comparison is to
> debbugs.  It might be cool if, when you run a project command from a
> debbugs buffer, you land in a worktree which has the changes from (any?)
> patches in that bug.  Although that's obviously quite a bit harder since
> you need to find the patch in the thread, and find the base branch to
> apply it to.)

It seems to me the same principle can apply, with the exception of the 
difficulties you mention, of course.

>>> Since adding a new argument to project-find-functions is hard, maybe we
>>> could do this by introducing a new dynamic variable
>>> project-find-functions-may-prompt which we let-bind?  Like:
>>> diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
>>> index c7c07c3d34c..3975182b88d 100644
>>> --- a/lisp/progmodes/project.el
>>> +++ b/lisp/progmodes/project.el
>>> @@ -242,8 +242,9 @@ project-current
>>>            (setq pr (cons 'transient directory))))
>>>        pr))
>>>    -(defun project--find-in-directory (dir)
>>> -  (run-hook-with-args-until-success 'project-find-functions dir))
>>> +(defun project--find-in-directory (dir &optional maybe-prompt)
>>> +  (let ((project-find-functions-may-prompt maybe-prompt))
>>> +    (run-hook-with-args-until-success 'project-find-functions dir)))
>>>      (defvar project--within-roots-fallback nil)
>>
>> As far as the implementation goes, a dynamic variable can work. We
>> could also try reusing an existing one: non-essential, and we'd set it
>> to nil when maybe-prompt is non-nil.
>>
>> I wonder how it would interact with Tramp (ask for password in
>> disconnected buffers?), but that seems to fall into the same general
>> category as your other cases.
> 
> Nice idea, it does seem like we should probably already be binding
> non-essential=t around project-find-functions when maybe-prompt is nil.
> Since already TRAMP can cause prompting even when a programmer calls
> project-current with maybe-prompt=nil.

If this change will be enough to cover your scenario, let's go ahead and 
add the 'non-essential' binding. It does seem to make sense for Tramp, 
at least.

bug#69584: 29.2.50; project-find-functions should have access to maybe-prompt

[Spencer Baugh]

Dmitry Gutov <dmitry@gutov.dev> writes:
>>> With regards to caching, for example, if some caller wanted to do so
>>> (some related discussion:
>>> https://github.com/joaotavora/breadcrumb/issues/18#issuecomment-1984615275),
>>> then would also need to take this parameter into account.
>> True, but it's already not correct to cache when maybe-prompt=t,
>> right?
>> Because the returned project may just be the one that the user choose
>> interactively at the prompt.
>
> It's... a good point, but so far the main exception was the
> "transient" project, for which one could make an exception somehow, or
> even cache without major downsides, as long as buffer stays the same
> and the cache interval is low.
>
> Disabling cache altogether with maybe-prompt=t might be a net
> negative, given that many users' interaction with project.el might be
> limited to commands that only do such invocations. But perhaps it's
> the price to pay for flexibility: as long as we're talking about
> external cache, it will be up to the callers to avoid caching where
> the results can be non-deterministic, such as after a prompt.

Hm, I'm slightly confused, isn't the problem more general than just the
transient project?  If I run (project-current t directory), and I get a
project back, I have no idea whether that project is actually for
DIRECTORY or not: if DIRECTORY is not in a project at all, the returned
project is instead some project selected by the user with
project-prompter.

>>> So the first thing I'd ask is whether you see a different way to
>>> implement the same features. I don't see the whole usage scenarios, so
>>> it's hard to judge.
>> Let me give some context about my concrete use case.
>> At Jane Street we have a code review system built on top of Emacs,
>> called FE.  A user opens a code review in a buffer in Emacs, and can see
>> that review's diff.  If they want to comment on the review, or build the
>> code or test it or anything like that, they need to also have a local
>> working copy of the code in the review.  The local working copy for each
>> review is kept separately, like git worktrees.
>> (There are some other details and screenshots in
>> https://blog.janestreet.com/putting-the-i-back-in-ide-towards-a-github-explorer/
>> but this should suffice)
>> So my use case then is this: when a user opens code review FE-123 in
>> a
>> buffer, they look at the diff and then decide they want to do something
>> in a working copy of the code.  Currently, to do that they run one of a
>> variety of internal commands which duplicate things like
>> project-find-file, but which are aware of whether or not there's a local
>> working copy, and operate the local working copy if any, and otherwise
>> prompt to create a local working copy and then error.
>> I'd like to replace those internal commands with just normal
>> project-find-file, and also allow other commands which use
>> project-current to determine the current project to just work.
>
> If you set up a project instance in a buffer-local way, would it even
> work correctly outside of that buffer?

Hm, I don't see why it wouldn't?  It's not really any different, again,
from project-prompter returning a project when DIRECTORY isn't a
project.  I'm intending for these functions to return a totally normal
vc project, to be clear - the only magic is in initially finding that vc
project, when default-directory isn't in that vc project.

> Would project history work fine? When you pick a project from recently
> visited, you basically just apply its last root directory and expect
> the project to be "found".

The project-root would be just be the normal directory that the project
actually is located in, in the filesystem.  And since it would be a
normal vc project, project-find-functions would return the same project
instance when run on its root.  So that would work fine too.

> I've read the article (thanks!), but I'm not sure yet of what would be
> the ergonomic savings in such scenario when instead of having a
> separate command to check out a branch and visit a file in it (perhaps
> bound to 'o f' inside the major mode map for the branches list's
> buffer), you call project-find-file right away. In the former scenario
> such command would make sure the branch is checked out, so its
> directory has proper contents, and then it could delegate to
> project-find-file inside said directory. And later visits (e.g. from
> project-switch-project) would work fine until the directory is
> deleted.

Consider project-vc-dir or project-dired.  The default-directory of
these directories is the project root, so if you want to operate on the
project, you can do that in these buffers.  And that's convenient and
good - you can do things like find-file or project-find-file or
whatever, because these buffers are conceptually "within" the project.

The branch overview is like project-vc-dir, but you can also open it
when there's no local working copy for a branch.

If there *is* a local working copy, the branch overview has a
default-directory in the project, so you can treat it like
project-vc-dir or project-dired.  This is the common case, this works
great.

If there isn't a local working copy, the branch overview has a
default-directory of "/" just because there's no sensible
default-directory for the buffer.  And if you open a branch overview and
you know there's no local working copy, you could run a command to
create a local working copy and only then start treating it like
project-vc-dir, running commands which operate on the project.

But, it's convenient to be able to ignore whether a given branch
overview has a local copy or not.  Indeed, there are heuristics which
pre-create local copies for branches you are likely to interact with,
e.g. branches you need to review code for.  So for normal development,
there will usually be a local working copy before you open the branch
overview, even without your intervention.  So you can get away with only
rarely explicitly creating one.

So when you open up a branch overview, you'll usually assume there's a
local copy, and so your first action will probably some command which
uses project-current.  But if there's no working copy, then you'll get
dropped to a prompt to choose a project, instead of (say) a
project-find-file prompt, which you might not immediately notice, which
is confusing, and you'll have to C-g out of it, and then run some other
command to create the working copy.  All that is a hassle.

A few other potential things I could do to solve that confusing
situation:

- My project-find-function could detect if it's running in a branch
  overview buffer without a local copy and immediately error, which
  stops project-current from running, so it can't prompt.

- I could make the branch overview buffer always have a
  default-directory of the location where the local copy *will* be
  created, even if it doesn't currently exist.  (All of the local
  working copies are created as subdirectories of one specific
  directory.)  Then my project-find-function could look at the
  default-directory string without touching the filesystem, detect that
  it's in the directory for projects managed by my package, and return a
  project instance with a project-root that doesn't actually exist, so
  then project-find-file will fail when it tries to list files for a
  nonexistent project.

I'm guessing both of those also have undesirable implications for the
project-current semantics, though?

>>>> Since adding a new argument to project-find-functions is hard, maybe we
>>>> could do this by introducing a new dynamic variable
>>>> project-find-functions-may-prompt which we let-bind?  Like:
>>>> diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
>>>> index c7c07c3d34c..3975182b88d 100644
>>>> --- a/lisp/progmodes/project.el
>>>> +++ b/lisp/progmodes/project.el
>>>> @@ -242,8 +242,9 @@ project-current
>>>>            (setq pr (cons 'transient directory))))
>>>>        pr))
>>>>    -(defun project--find-in-directory (dir)
>>>> -  (run-hook-with-args-until-success 'project-find-functions dir))
>>>> +(defun project--find-in-directory (dir &optional maybe-prompt)
>>>> +  (let ((project-find-functions-may-prompt maybe-prompt))
>>>> +    (run-hook-with-args-until-success 'project-find-functions dir)))
>>>>      (defvar project--within-roots-fallback nil)
>>>
>>> As far as the implementation goes, a dynamic variable can work. We
>>> could also try reusing an existing one: non-essential, and we'd set it
>>> to nil when maybe-prompt is non-nil.
>>>
>>> I wonder how it would interact with Tramp (ask for password in
>>> disconnected buffers?), but that seems to fall into the same general
>>> category as your other cases.
>> Nice idea, it does seem like we should probably already be binding
>> non-essential=t around project-find-functions when maybe-prompt is nil.
>> Since already TRAMP can cause prompting even when a programmer calls
>> project-current with maybe-prompt=nil.
>
> If this change will be enough to cover your scenario, let's go ahead
> and add the 'non-essential' binding. It does seem to make sense for
> Tramp, at least.

Yes, that completely covers my scenario.  (Putting aside whether my
scenario is a good idea :) )

So I would be happy with that.

bug#69584: 29.2.50; project-find-functions should have access to maybe-prompt

[Dmitry Gutov]

On 22/03/2024 15:05, Spencer Baugh wrote:

>> Disabling cache altogether with maybe-prompt=t might be a net
>> negative, given that many users' interaction with project.el might be
>> limited to commands that only do such invocations. But perhaps it's
>> the price to pay for flexibility: as long as we're talking about
>> external cache, it will be up to the callers to avoid caching where
>> the results can be non-deterministic, such as after a prompt.
> 
> Hm, I'm slightly confused, isn't the problem more general than just the
> transient project?  If I run (project-current t directory), and I get a
> project back, I have no idea whether that project is actually for
> DIRECTORY or not: if DIRECTORY is not in a project at all, the returned
> project is instead some project selected by the user with
> project-prompter.

That's a good point. The assumption was that it *would* be for DIRECTORY 
(some parent, perhaps), but that relation won't necessarily hold.

Then it becomes a question for the cache creator - whether such cached 
entry is useful, and for how long.

>>> So my use case then is this: when a user opens code review FE-123 in
>>> a
>>> buffer, they look at the diff and then decide they want to do something
>>> in a working copy of the code.  Currently, to do that they run one of a
>>> variety of internal commands which duplicate things like
>>> project-find-file, but which are aware of whether or not there's a local
>>> working copy, and operate the local working copy if any, and otherwise
>>> prompt to create a local working copy and then error.
>>> I'd like to replace those internal commands with just normal
>>> project-find-file, and also allow other commands which use
>>> project-current to determine the current project to just work.
>>
>> If you set up a project instance in a buffer-local way, would it even
>> work correctly outside of that buffer?
> 
> Hm, I don't see why it wouldn't?  It's not really any different, again,
> from project-prompter returning a project when DIRECTORY isn't a
> project.  I'm intending for these functions to return a totally normal
> vc project, to be clear - the only magic is in initially finding that vc
> project, when default-directory isn't in that vc project.

I mean, it _might not_ work. For example, if the project implementation 
does some additional buffer-local things like storing extra information 
related to the project in buffer-local variables. Which would be a valid 
thing to do for buffer-local projects, but not a particularly great one 
in the general context.

Anyway, that depends on you as the author as well - to avoid such problems.

>> Would project history work fine? When you pick a project from recently
>> visited, you basically just apply its last root directory and expect
>> the project to be "found".
> 
> The project-root would be just be the normal directory that the project
> actually is located in, in the filesystem.  And since it would be a
> normal vc project, project-find-functions would return the same project
> instance when run on its root.  So that would work fine too.

Very good.

>> I've read the article (thanks!), but I'm not sure yet of what would be
>> the ergonomic savings in such scenario when instead of having a
>> separate command to check out a branch and visit a file in it (perhaps
>> bound to 'o f' inside the major mode map for the branches list's
>> buffer), you call project-find-file right away. In the former scenario
>> such command would make sure the branch is checked out, so its
>> directory has proper contents, and then it could delegate to
>> project-find-file inside said directory. And later visits (e.g. from
>> project-switch-project) would work fine until the directory is
>> deleted.
> 
> Consider project-vc-dir or project-dired.  The default-directory of
> these directories is the project root, so if you want to operate on the
> project, you can do that in these buffers.  And that's convenient and
> good - you can do things like find-file or project-find-file or
> whatever, because these buffers are conceptually "within" the project.
> 
> The branch overview is like project-vc-dir, but you can also open it
> when there's no local working copy for a branch.
> 
> If there *is* a local working copy, the branch overview has a
> default-directory in the project, so you can treat it like
> project-vc-dir or project-dired.  This is the common case, this works
> great.

I think I get it now, thanks.

> If there isn't a local working copy, the branch overview has a
> default-directory of "/" just because there's no sensible
> default-directory for the buffer.

This detail makes me somewhat uneasy, but ultimately it's up to you to 
test this and related behaviors (any other callers of project-current 
that you use). If the important ones work out fine, then I suppose this 
can be okay.

> And if you open a branch overview and
> you know there's no local working copy, you could run a command to
> create a local working copy and only then start treating it like
> project-vc-dir, running commands which operate on the project.
> 
> But, it's convenient to be able to ignore whether a given branch
> overview has a local copy or not.  Indeed, there are heuristics which
> pre-create local copies for branches you are likely to interact with,
> e.g. branches you need to review code for.  So for normal development,
> there will usually be a local working copy before you open the branch
> overview, even without your intervention.  So you can get away with only
> rarely explicitly creating one.
> 
> So when you open up a branch overview, you'll usually assume there's a
> local copy, and so your first action will probably some command which
> uses project-current.  But if there's no working copy, then you'll get
> dropped to a prompt to choose a project, instead of (say) a
> project-find-file prompt, which you might not immediately notice, which
> is confusing, and you'll have to C-g out of it, and then run some other
> command to create the working copy.  All that is a hassle.
> 
> A few other potential things I could do to solve that confusing
> situation:
> 
> - My project-find-function could detect if it's running in a branch
>    overview buffer without a local copy and immediately error, which
>    stops project-current from running, so it can't prompt.
> 
> - I could make the branch overview buffer always have a
>    default-directory of the location where the local copy *will* be
>    created, even if it doesn't currently exist.  (All of the local
>    working copies are created as subdirectories of one specific
>    directory.)  Then my project-find-function could look at the
>    default-directory string without touching the filesystem, detect that
>    it's in the directory for projects managed by my package, and return a
>    project instance with a project-root that doesn't actually exist, so
>    then project-find-file will fail when it tries to list files for a
>    nonexistent project.
> 
> I'm guessing both of those also have undesirable implications for the
> project-current semantics, though?

In general, I would prefer the latter - as long as your 
project-find-functions element comes first, it should recognize those 
paths okay.

And the vc-aware backend returns nil silently when default-directory is 
non-existent, so the obvious problem shouldn't come up.

Anyway, the patch we settled on is agnostic to this choice, so there is 
no urgency to make this choice or change.

>> If this change will be enough to cover your scenario, let's go ahead
>> and add the 'non-essential' binding. It does seem to make sense for
>> Tramp, at least.
> 
> Yes, that completely covers my scenario.  (Putting aside whether my
> scenario is a good idea :) )
> 
> So I would be happy with that.

Now pushed to master as commit 1552f8345d8.

bug#69584: 29.2.50; project-find-functions should have access to maybe-prompt

[Spencer Baugh]

Spencer Baugh <sbaugh@janestreet.com> writes:
> So when you open up a branch overview, you'll usually assume there's a
> local copy, and so your first action will probably some command which
> uses project-current.  But if there's no working copy, then you'll get
> dropped to a prompt to choose a project, instead of (say) a
> project-find-file prompt, which you might not immediately notice, which
> is confusing, and you'll have to C-g out of it, and then run some other
> command to create the working copy.  All that is a hassle.

Oh, I've thought of a different resolution to this which may be better.

When project-current fails to find a project in default-directory, if
maybe-prompt=t, project-current will run project-prompter.

So I can just have a buffer-local project-prompter in the branch
overview buffer.  And that project-prompter knows it's in a branch
overview buffer, and can prompt "Would you like to make a working copy
for [some branch]?".

That's elegant and doesn't change the semantics of project-current at
all: project-prompter already can return an arbitrary directory.

Except... I suppose this would make project-switch-project behave worse,
because hitting C-x p p in a branch overview buffer would now prompt to
create a working copy for that branch, when what you probably want to do
is switch to a different project entirely.

I guess there are two use cases for project-prompter: one is "fallback
for project-current" and the other is "switch to a different project".
Maybe we could support them being different?

bug#69584: 29.2.50; project-find-functions should have access to maybe-prompt

[Dmitry Gutov]

On 02/04/2024 20:54, Spencer Baugh wrote:
> Spencer Baugh <sbaugh@janestreet.com> writes:
>> So when you open up a branch overview, you'll usually assume there's a
>> local copy, and so your first action will probably some command which
>> uses project-current.  But if there's no working copy, then you'll get
>> dropped to a prompt to choose a project, instead of (say) a
>> project-find-file prompt, which you might not immediately notice, which
>> is confusing, and you'll have to C-g out of it, and then run some other
>> command to create the working copy.  All that is a hassle.
> 
> Oh, I've thought of a different resolution to this which may be better.
> 
> When project-current fails to find a project in default-directory, if
> maybe-prompt=t, project-current will run project-prompter.
> 
> So I can just have a buffer-local project-prompter in the branch
> overview buffer.  And that project-prompter knows it's in a branch
> overview buffer, and can prompt "Would you like to make a working copy
> for [some branch]?".
> 
> That's elegant and doesn't change the semantics of project-current at
> all: project-prompter already can return an arbitrary directory.
> 
> Except... I suppose this would make project-switch-project behave worse,
> because hitting C-x p p in a branch overview buffer would now prompt to
> create a working copy for that branch, when what you probably want to do
> is switch to a different project entirely.
> 
> I guess there are two use cases for project-prompter: one is "fallback
> for project-current" and the other is "switch to a different project".
> Maybe we could support them being different?

I'm not sure how we'd do that. A new, optional, argument to 
project-prompter? Two different variables?

Right now we have two well-defined steps:

- Run a hook, where the current project can be detected,
- Or call the prompter, which will let the user choose a project 
manually (from the history, or from the file system).

That helps the prompter have a compact, focused interface. If it can 
also ask about other things, that makes things more complicated.

But for your own system, I suppose you could implement the described 
workflow right now with the current tools, using add-function: your 
project-find-functions element would itself return nil but add an 
around-advice for project-prompter, which would do more work, prompt the 
user, and ultimately return the intended object. And remove the advice 
as the first step, of course. What could require further testing - is 
the scenario where the prompter is called twice (when you invoke 
project-switch-project), but it'll probably work too, since the calls 
are not recursive.

To take a step back, I wonder if you have tried using the newly added 
binding for 'non-essential' yet. It seems like it should allow a simpler 
implementation.

bug#69584: 29.2.50; project-find-functions should have access to maybe-prompt

[Spencer Baugh]

Dmitry Gutov <dmitry@gutov.dev> writes:
> On 22/03/2024 15:05, Spencer Baugh wrote:
>>> If this change will be enough to cover your scenario, let's go ahead
>>> and add the 'non-essential' binding. It does seem to make sense for
>>> Tramp, at least.
>> Yes, that completely covers my scenario.  (Putting aside whether my
>> scenario is a good idea :) )
>> So I would be happy with that.
>
> Now pushed to master as commit 1552f8345d8.

Ah, I think this is not quite right, should be:

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index 1da03c7b60e..3cd6dafb409 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -229,8 +229,8 @@ project-current
 of the project instance object."
   (unless directory (setq directory (or project-current-directory-override
                                         default-directory)))
-  (let ((pr (project--find-in-directory directory))
-        (non-essential (not maybe-prompt)))
+  (let* ((non-essential (not maybe-prompt))
+         (pr (project--find-in-directory directory)))
     (cond
      (pr)
      ((unless project-current-directory-override

bug#69584: 29.2.50; project-find-functions should have access to maybe-prompt

[Dmitry Gutov]

On 04/04/2024 17:29, Spencer Baugh wrote:
> Dmitry Gutov<dmitry@gutov.dev>  writes:
>> On 22/03/2024 15:05, Spencer Baugh wrote:
>>>> If this change will be enough to cover your scenario, let's go ahead
>>>> and add the 'non-essential' binding. It does seem to make sense for
>>>> Tramp, at least.
>>> Yes, that completely covers my scenario.  (Putting aside whether my
>>> scenario is a good idea 🙂 )
>>> So I would be happy with that.
>> Now pushed to master as commit 1552f8345d8.
> Ah, I think this is not quite right, should be:
> 
> diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
> index 1da03c7b60e..3cd6dafb409 100644
> --- a/lisp/progmodes/project.el
> +++ b/lisp/progmodes/project.el
> @@ -229,8 +229,8 @@ project-current
>   of the project instance object."
>     (unless directory (setq directory (or project-current-directory-override
>                                           default-directory)))
> -  (let ((pr (project--find-in-directory directory))
> -        (non-essential (not maybe-prompt)))
> +  (let* ((non-essential (not maybe-prompt))
> +         (pr (project--find-in-directory directory)))
>       (cond
>        (pr)
>        ((unless project-current-directory-override

Right! Thanks for the correction (21f9be00531 in master).

bug#69584: 29.2.50; project-find-functions should have access to maybe-prompt

[Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Dmitry Gutov <dmitry@gutov.dev> writes:
> On 04/04/2024 17:29, Spencer Baugh wrote:
>> Dmitry Gutov<dmitry@gutov.dev>  writes:
>>> On 22/03/2024 15:05, Spencer Baugh wrote:
>>>>> If this change will be enough to cover your scenario, let's go ahead
>>>>> and add the 'non-essential' binding. It does seem to make sense for
>>>>> Tramp, at least.
>>>> Yes, that completely covers my scenario.  (Putting aside whether my
>>>> scenario is a good idea 🙂 )
>>>> So I would be happy with that.
>>> Now pushed to master as commit 1552f8345d8.
>> Ah, I think this is not quite right, should be:
>> diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
>> index 1da03c7b60e..3cd6dafb409 100644
>> --- a/lisp/progmodes/project.el
>> +++ b/lisp/progmodes/project.el
>> @@ -229,8 +229,8 @@ project-current
>>   of the project instance object."
>>     (unless directory (setq directory (or project-current-directory-override
>>                                           default-directory)))
>> -  (let ((pr (project--find-in-directory directory))
>> -        (non-essential (not maybe-prompt)))
>> +  (let* ((non-essential (not maybe-prompt))
>> +         (pr (project--find-in-directory directory)))
>>       (cond
>>        (pr)
>>        ((unless project-current-directory-override
>
> Right! Thanks for the correction (21f9be00531 in master).

This worked great for my use cases, I think we can close this bug.

bug#69584: 29.2.50; project-find-functions should have access to maybe-prompt

[Dmitry Gutov]

On 23/09/2024 23:16, Spencer Baugh wrote:
>>>      (unless directory (setq directory (or project-current-directory-override
>>>                                            default-directory)))
>>> -  (let ((pr (project--find-in-directory directory))
>>> -        (non-essential (not maybe-prompt)))
>>> +  (let* ((non-essential (not maybe-prompt))
>>> +         (pr (project--find-in-directory directory)))
>>>        (cond
>>>         (pr)
>>>         ((unless project-current-directory-override
>> Right! Thanks for the correction (21f9be00531 in master).
> This worked great for my use cases, I think we can close this bug.

Very good, closing.