GNU bug report logs - #63648
29.0.90; project.el: with switch-use-entire-map, switch-project errors on non-project commands

Previous Next

Full log

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

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: Spencer Baugh <sbaugh <at> janestreet.com>, 63648 <at> debbugs.gnu.org,
 sbaugh <at> catern.com
Subject: Re: bug#63648: 29.0.90; project.el: with switch-use-entire-map,
 switch-project errors on non-project commands
Date: Wed, 13 Sep 2023 09:47:09 +0300

>> So here is a complete tested patch that maintains backward-compatibility
>> with older versions, and is localized to project.el without the need to
>> discuss more fundamental changes on emacs-devel, and handles 100% of
>> known cases such as reported in bug#58784, bug#63829, etc.
>
> It sounds like a possibly concise solution, but I'm still wrapping my head
> around it.
>
>> +(make-obsolete-variable
>> + 'project-current-directory-override
>> + 'project-current-directory-old
>> + "30.1")
>
> Aren't those variables sufficiently different that making one an alias for
> another would confuse things?

Indeed, during development I mistakenly made one an alias for another
with 'define-obsolete-variable-alias', and this broke both variables.
So I replaced it with 'make-obsolete-variable' that doesn't make an alias.
It just designates it as obsolete to show the intention to remove it
in later versions.

>> +(defvar-local project-current-directory-old nil
>> +  "Value to use instead of `default-directory' when detecting the project.
>> +For the next command after switching the project, this buffer-local
>> +variable contains the original value of `default-directory'.
>> +Whereas the buffer-local `default-directory' is temporarily set
>> +to the root directory of the switched project.
>> +When it is non-nil, `project-current' will always skip prompting too.")
>
> The docstring is valuable, but I wonder how it looks to somebody from the
> outside trying to write code that would use it.

I don't believe that someone might want to use this variable in more commands
than the current project commands that rely on 'project-buffers'.  But we could
add a few more lines with explanations for them too.

>> +                   (or (buffer-local-value 'project-current-directory-old buf)
>> +                       (buffer-local-value 'default-directory buf))))
> ...
>> +                (or (buffer-local-value 'project-current-directory-old buf)
>> +                    (buffer-local-value 'default-directory buf))))
>
> So, this part looks like what we would be paying for this solution: any
> code looking to decide whether a buffer "belongs" to the project, would
> have to reproduce this exact expression. Wouldn't it?

Then maybe this code should be moved to a separate function?

>> +      (setq-local project-current-directory-old default-directory)
>> +      (setq-local default-directory dir)
>
> Could you explain: if we can just set these here and then clean up in
> postfun, couldn't we likewise set (and then later clean up) the value of
> project-current-directory-override?

We need to set 'default-directory' to support non-project commands.
I already started to use this feature, and it becomes indispensable.
For example, the most often used commands are vc commands such as
'C-x p p C-x v L' to see a vc log in another project, and
'C-x p p C-x v d' to open vc-dir, etc.

>> -		     (eq this-command command))
>> +		     (eq this-command command)
>> +		     (eq this-command 'other-project-prefix))
>
> Did this part of the patch get in by accident? If it's "localized to
> project.el". Or do we have further plans to "generalize" that place
> somehow? Just making sure.

This part is optional and could be generalized later.

Previous Next

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