[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#69584: 29.2.50; project-find-functions should have access to maybe-p
|
From: |
Dmitry Gutov |
|
Subject: |
bug#69584: 29.2.50; project-find-functions should have access to maybe-prompt |
|
Date: |
Mon, 18 Mar 2024 23:59:40 +0200 |
|
User-agent: |
Mozilla Thunderbird |
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.