GNU bug report logs - #58784
28.2; project-buffers incorrect under let-bound default-directory

Previous Next

Full log

Message #11 received at 58784 <at> debbugs.gnu.org (full text, mbox):

From: Sean Devlin <spd <at> toadstyle.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 58784 <at> debbugs.gnu.org
Subject: Re: bug#58784: 28.2; project-buffers incorrect under let-bound
 default-directory
Date: Mon, 31 Oct 2022 13:47:20 -0400

Hi Dmitry,

> On Oct 28, 2022, at 8:49 PM, Dmitry Gutov <dgutov <at> yandex.ru> wrote:
> 
> Hi!
> 
> On 26.10.2022 03:13, Sean Devlin wrote:
>> The project.el function project-buffers can return incorrect results
>> when invoked while a let-binding for default-directory is in
>> effect. This is because project-buffers (both the default and vc-based
>> implementations) does its work by inspecting the local value of
>> default-directory in each buffer, and the let-binding temporarily
>> affects this value.
>> To see this in action, start emacs with -Q and evaluate the following
>> forms in order:
>> (require 'project)
>> (find-file-noselect "/tmp/tmpfile")
>> (setq my-project '(transient . "/tmp/"))
>> ;; just the tmpfile
>> (project-buffers my-project)
>> ;; both tmpfile and scratch
>> (let ((default-directory "/tmp/"))
>>   (project-buffers my-project))
> 
> Thanks for the report, I can see the problem.
> 
>> In the last form, project-buffers includes the current buffer (i.e. the
>> scratch buffer in our example) with the results. (This is true even if
>> the current buffer is visiting a file in some unrelated directory.)
>> This matters because the command project-switch-project let-binds
>> default-directory before calling project-switch-commands. This means
>> that if you set project-switch-commands to some function that calls
>> project-buffers, you will get incorrect results.
>> For example, evaluate the following forms in order:
>> (defun my-list-project-buffers ()
>>   "List the current project's buffers."
>>   (interactive)
>>   (let ((buffer-list (project-buffers (project-current t)))
>> 	(buffer-name (project-prefixed-buffer-name "my-project-buffer-list")))
>>     (with-current-buffer (get-buffer-create buffer-name)
>>       (erase-buffer)
>>       (save-excursion
>> 	(dolist (buffer buffer-list)
>> 	  (insert (buffer-name buffer))
>> 	  (insert ?\n))))
>>     (switch-to-buffer buffer-name)))
>> (setq project-switch-commands #'my-list-project-buffers)
> 
> Looks like a reimplementation of projectile-ibuffer, seems useful.

Yeah, in my real configuration, I defined this as an ibuffer filter, but I thought a simpler example would be better for the bug report.

> 
>> ;; list tmpfile but also scratch
>> (project-switch-project "/tmp/")
> 
> Not sure how to fix this, though. In bug#53626 we discussed a somewhat similar problem, and a let-binding seems impossible to "escape".
> 
> What else can we do? One option is to change the signature of every compatible command to take the project object as its first argument. Might have been more realistic when the package was first written, too much breakage now, probably.
> 
> Another would be to add a new var to help override the project choice without touch default-directory.
> 
> Something like the attached. Please try it out.<project-current-directory-override.diff>

I agree, the fix is not obvious.

A workaround I used in my configuration is to add an advice to project-switch-project to wrap it in a with-temp-buffer form. This way, it’s only the default-directory of the temp buffer that gets corrupted. Not a very clean solution, but it seems to work. Unfortunately, you need to remember to do this in many places; for example, I had to do the same thing in my project-ibuffer command.

Your solution looks cleaner. I gave it a try (along with disabling my advice), and it seems to work pretty well. Thanks for the fix!

There might be some more places where it needs to be applied. For example, project-prefixed-buffer-name still inspects the default-directory. (Maybe project-prefixed-buffer-name should just call project-root or similar?)

I think there’s still some fragility in the project-buffers function, since any callers need to be careful not to bind default-directory. It might be useful to call this out in the doc string or in the manual.

Thanks for your help!

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.