Package: emacs;
Reported by: Spencer Baugh <sbaugh <at> janestreet.com>
Date: Wed, 6 Mar 2024 14:25:01 UTC
Severity: normal
Found in version 29.2.50
Done: Dmitry Gutov <dmitry <at> gutov.dev>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 69584 in the body.
You can then email your comments to 69584 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
View this report as an mbox folder, status mbox, maintainer mbox
bug-gnu-emacs <at> gnu.org
:bug#69584
; Package emacs
.
(Wed, 06 Mar 2024 14:25:01 GMT) Full text and rfc822 format available.Spencer Baugh <sbaugh <at> janestreet.com>
:bug-gnu-emacs <at> gnu.org
.
(Wed, 06 Mar 2024 14:25:01 GMT) Full text and rfc822 format available.Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
From: Spencer Baugh <sbaugh <at> janestreet.com> To: bug-gnu-emacs <at> gnu.org Subject: 29.2.50; project-find-functions should have access to maybe-prompt Date: Wed, 06 Mar 2024 09:23:24 -0500
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-gnu-emacs <at> gnu.org
:bug#69584
; Package emacs
.
(Fri, 15 Mar 2024 01:42:01 GMT) Full text and rfc822 format available.Message #8 received at 69584 <at> debbugs.gnu.org (full text, mbox):
From: Dmitry Gutov <dmitry <at> gutov.dev> To: Spencer Baugh <sbaugh <at> janestreet.com>, 69584 <at> debbugs.gnu.org Subject: Re: bug#69584: 29.2.50; project-find-functions should have access to maybe-prompt Date: Fri, 15 Mar 2024 03:41:01 +0200
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-gnu-emacs <at> gnu.org
:bug#69584
; Package emacs
.
(Sat, 16 Mar 2024 13:33:02 GMT) Full text and rfc822 format available.Message #11 received at 69584 <at> debbugs.gnu.org (full text, mbox):
From: sbaugh <at> catern.com To: Dmitry Gutov <dmitry <at> gutov.dev> Cc: Spencer Baugh <sbaugh <at> janestreet.com>, 69584 <at> debbugs.gnu.org Subject: Re: bug#69584: 29.2.50; project-find-functions should have access to maybe-prompt Date: Sat, 16 Mar 2024 13:31:49 +0000 (UTC)
Dmitry Gutov <dmitry <at> 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-gnu-emacs <at> gnu.org
:bug#69584
; Package emacs
.
(Mon, 18 Mar 2024 22:01:02 GMT) Full text and rfc822 format available.Message #14 received at 69584 <at> debbugs.gnu.org (full text, mbox):
From: Dmitry Gutov <dmitry <at> gutov.dev> To: sbaugh <at> catern.com Cc: Spencer Baugh <sbaugh <at> janestreet.com>, 69584 <at> debbugs.gnu.org Subject: Re: bug#69584: 29.2.50; project-find-functions should have access to maybe-prompt Date: Mon, 18 Mar 2024 23:59:40 +0200
On 16/03/2024 15:31, sbaugh <at> 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-gnu-emacs <at> gnu.org
:bug#69584
; Package emacs
.
(Fri, 22 Mar 2024 13:11:01 GMT) Full text and rfc822 format available.Message #17 received at 69584 <at> debbugs.gnu.org (full text, mbox):
From: Spencer Baugh <sbaugh <at> janestreet.com> To: Dmitry Gutov <dmitry <at> gutov.dev> Cc: sbaugh <at> catern.com, 69584 <at> debbugs.gnu.org Subject: Re: bug#69584: 29.2.50; project-find-functions should have access to maybe-prompt Date: Fri, 22 Mar 2024 09:05:40 -0400
Dmitry Gutov <dmitry <at> 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-gnu-emacs <at> gnu.org
:bug#69584
; Package emacs
.
(Thu, 28 Mar 2024 03:46:02 GMT) Full text and rfc822 format available.Message #20 received at 69584 <at> debbugs.gnu.org (full text, mbox):
From: Dmitry Gutov <dmitry <at> gutov.dev> To: Spencer Baugh <sbaugh <at> janestreet.com> Cc: sbaugh <at> catern.com, 69584 <at> debbugs.gnu.org Subject: Re: bug#69584: 29.2.50; project-find-functions should have access to maybe-prompt Date: Thu, 28 Mar 2024 05:44:52 +0200
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-gnu-emacs <at> gnu.org
:bug#69584
; Package emacs
.
(Tue, 02 Apr 2024 17:55:03 GMT) Full text and rfc822 format available.Message #23 received at 69584 <at> debbugs.gnu.org (full text, mbox):
From: Spencer Baugh <sbaugh <at> janestreet.com> To: Dmitry Gutov <dmitry <at> gutov.dev> Cc: sbaugh <at> catern.com, 69584 <at> debbugs.gnu.org Subject: Re: bug#69584: 29.2.50; project-find-functions should have access to maybe-prompt Date: Tue, 02 Apr 2024 13:54:03 -0400
Spencer Baugh <sbaugh <at> 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-gnu-emacs <at> gnu.org
:bug#69584
; Package emacs
.
(Tue, 02 Apr 2024 23:12:02 GMT) Full text and rfc822 format available.Message #26 received at 69584 <at> debbugs.gnu.org (full text, mbox):
From: Dmitry Gutov <dmitry <at> gutov.dev> To: Spencer Baugh <sbaugh <at> janestreet.com> Cc: sbaugh <at> catern.com, 69584 <at> debbugs.gnu.org Subject: Re: bug#69584: 29.2.50; project-find-functions should have access to maybe-prompt Date: Wed, 3 Apr 2024 02:10:56 +0300
On 02/04/2024 20:54, Spencer Baugh wrote: > Spencer Baugh <sbaugh <at> 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-gnu-emacs <at> gnu.org
:bug#69584
; Package emacs
.
(Thu, 04 Apr 2024 14:31:03 GMT) Full text and rfc822 format available.Message #29 received at 69584 <at> debbugs.gnu.org (full text, mbox):
From: Spencer Baugh <sbaugh <at> janestreet.com> To: Dmitry Gutov <dmitry <at> gutov.dev> Cc: sbaugh <at> catern.com, 69584 <at> debbugs.gnu.org Subject: Re: bug#69584: 29.2.50; project-find-functions should have access to maybe-prompt Date: Thu, 04 Apr 2024 10:29:48 -0400
Dmitry Gutov <dmitry <at> 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-gnu-emacs <at> gnu.org
:bug#69584
; Package emacs
.
(Fri, 05 Apr 2024 00:35:03 GMT) Full text and rfc822 format available.Message #32 received at 69584 <at> debbugs.gnu.org (full text, mbox):
From: Dmitry Gutov <dmitry <at> gutov.dev> To: Spencer Baugh <sbaugh <at> janestreet.com> Cc: sbaugh <at> catern.com, 69584 <at> debbugs.gnu.org Subject: Re: bug#69584: 29.2.50; project-find-functions should have access to maybe-prompt Date: Fri, 5 Apr 2024 03:33:49 +0300
On 04/04/2024 17:29, Spencer Baugh wrote: > Dmitry Gutov<dmitry <at> 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-gnu-emacs <at> gnu.org
:bug#69584
; Package emacs
.
(Mon, 23 Sep 2024 20:18:01 GMT) Full text and rfc822 format available.Message #35 received at 69584 <at> debbugs.gnu.org (full text, mbox):
From: Spencer Baugh <sbaugh <at> janestreet.com> To: Dmitry Gutov <dmitry <at> gutov.dev> Cc: sbaugh <at> catern.com, 69584 <at> debbugs.gnu.org Subject: Re: bug#69584: 29.2.50; project-find-functions should have access to maybe-prompt Date: Mon, 23 Sep 2024 16:16:40 -0400
Dmitry Gutov <dmitry <at> gutov.dev> writes: > On 04/04/2024 17:29, Spencer Baugh wrote: >> Dmitry Gutov<dmitry <at> 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.
Dmitry Gutov <dmitry <at> gutov.dev>
:Spencer Baugh <sbaugh <at> janestreet.com>
:Message #40 received at 69584-done <at> debbugs.gnu.org (full text, mbox):
From: Dmitry Gutov <dmitry <at> gutov.dev> To: Spencer Baugh <sbaugh <at> janestreet.com> Cc: sbaugh <at> catern.com, 69584-done <at> debbugs.gnu.org Subject: Re: bug#69584: 29.2.50; project-find-functions should have access to maybe-prompt Date: Tue, 24 Sep 2024 01:23:45 +0300
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.
Debbugs Internal Request <help-debbugs <at> gnu.org>
to internal_control <at> debbugs.gnu.org
.
(Tue, 22 Oct 2024 11:24:10 GMT) Full text and rfc822 format available.
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.