bug#67171: 30.0.50; (At least) some VC commands fail with project-prefix-or-any-command

bug#67171: 30.0.50; (At least) some VC commands fail with project-prefix-or-any-command

[Sean Whitton]

X-debbugs-cc: dmitry@gutov.dev, juri@linkov.net, sbaugh@catern.com

Hello,

1. cd /home/spwhitton/src/some-project && emacs -q
2. (setopt project-switch-commands #'project-prefix-or-any-command)
3. (project-remember-project (project-current nil "/home/spwhitton/src/emacs/trunk/))
4. C-x p p /home/spwhitton/src/emacs/trunk RET C-x v L

VC prints the log of /home/spwhitton/some-project, not that of /home/spwhitton/src/emacs/trunk.

-- 
Sean Whitton

bug#67171: 30.0.50; (At least) some VC commands fail with project-prefix-or-any-command

[Dmitry Gutov]

Hi!

On 14/11/2023 15:13, Sean Whitton wrote:
> X-debbugs-cc: dmitry@gutov.dev, juri@linkov.net, sbaugh@catern.com
> 
> Hello,
> 
> 1. cd /home/spwhitton/src/some-project && emacs -q
> 2. (setopt project-switch-commands #'project-prefix-or-any-command)
> 3. (project-remember-project (project-current nil "/home/spwhitton/src/emacs/trunk/))
> 4. C-x p p /home/spwhitton/src/emacs/trunk RET C-x v L
> 
> VC prints the log of /home/spwhitton/some-project, not that of /home/spwhitton/src/emacs/trunk.

I'm having difficulty reproducing this.

On step 4, I'm asked for the project root (because *scratch* doesn't 
have a current VC backend), but the input defaults to the directory 
chosen in steps 3 and 4. And if I open a file-visiting buffer first, 
then the prompt is skipped, and I do see the log of the other project. 
Not the one used in step 1.

bug#67171: 30.0.50; (At least) some VC commands fail with project-prefix-or-any-command

[Sean Whitton]

Hello,

On Mon 20 Nov 2023 at 04:17am +02, Dmitry Gutov wrote:

> Hi!
>
> On 14/11/2023 15:13, Sean Whitton wrote:
>> X-debbugs-cc: dmitry@gutov.dev, juri@linkov.net, sbaugh@catern.com
>> Hello,
>> 1. cd /home/spwhitton/src/some-project && emacs -q
>> 2. (setopt project-switch-commands #'project-prefix-or-any-command)
>> 3. (project-remember-project (project-current nil "/home/spwhitton/src/emacs/trunk/))
>> 4. C-x p p /home/spwhitton/src/emacs/trunk RET C-x v L
>> VC prints the log of /home/spwhitton/some-project, not that of
>> /home/spwhitton/src/emacs/trunk.
>
> I'm having difficulty reproducing this.

Hmm.  I can't reproduce it with 'emacs -q', but I can still reproduce
with my init.el.  I'm not sure whether something changed or whether I
was wrong before.

> On step 4, I'm asked for the project root (because *scratch* doesn't
> have a current VC backend), but the input defaults to the directory
> chosen in steps 3 and 4. And if I open a file-visiting buffer first,
> then the prompt is skipped, [...]

If I select *Messages*, which has a default-directory of "~/", then
'C-x p p RET ~/src/emacs/trunk RET C-x v L' prompts me for a directory,
but the default value is "~/".  Before I start bisecting my init, does
anything else occur to you?

As an aside, it doesn't seem good that whether or not step four prompts
you for a directory depends on whether the current buffer happens to be
visiting a file or not.  Can we improve that, or is it inherent to the
design of the new feature?

Thanks.

-- 
Sean Whitton

bug#67171: 30.0.50; (At least) some VC commands fail with project-prefix-or-any-command

[Juri Linkov]

> As an aside, it doesn't seem good that whether or not step four prompts
> you for a directory depends on whether the current buffer happens to be
> visiting a file or not.  Can we improve that, or is it inherent to the
> design of the new feature?

This was improved recently in bug#67145, so now you can set
'vc-deduce-backend-nonvc-modes' to nil to avoid this prompt.
I'm not sure if nil should be the default value.

bug#67171: 30.0.50; (At least) some VC commands fail with project-prefix-or-any-command

[Dmitry Gutov]

On 23/11/2023 19:20, Juri Linkov wrote:
>> As an aside, it doesn't seem good that whether or not step four prompts
>> you for a directory depends on whether the current buffer happens to be
>> visiting a file or not.  Can we improve that, or is it inherent to the
>> design of the new feature?
> This was improved recently in bug#67145, so now you can set
> 'vc-deduce-backend-nonvc-modes' to nil to avoid this prompt.
> I'm not sure if nil should be the default value.

I wonder if the "deduce in all modes" value should be t rather than nil.

bug#67171: 30.0.50; (At least) some VC commands fail with project-prefix-or-any-command

[Dmitry Gutov]

On 23/11/2023 17:21, Sean Whitton wrote:

>> On 14/11/2023 15:13, Sean Whitton wrote:
>>> X-debbugs-cc: dmitry@gutov.dev, juri@linkov.net, sbaugh@catern.com
>>> Hello,
>>> 1. cd /home/spwhitton/src/some-project && emacs -q
>>> 2. (setopt project-switch-commands #'project-prefix-or-any-command)
>>> 3. (project-remember-project (project-current nil "/home/spwhitton/src/emacs/trunk/))
>>> 4. C-x p p /home/spwhitton/src/emacs/trunk RET C-x v L
>>> VC prints the log of /home/spwhitton/some-project, not that of
>>> /home/spwhitton/src/emacs/trunk.
>>
>> I'm having difficulty reproducing this.
> 
> Hmm.  I can't reproduce it with 'emacs -q', but I can still reproduce
> with my init.el.  I'm not sure whether something changed or whether I
> was wrong before.
> 
>> On step 4, I'm asked for the project root (because *scratch* doesn't
>> have a current VC backend), but the input defaults to the directory
>> chosen in steps 3 and 4. And if I open a file-visiting buffer first,
>> then the prompt is skipped, [...]
> 
> If I select *Messages*, which has a default-directory of "~/", then
> 'C-x p p RET ~/src/emacs/trunk RET C-x v L' prompts me for a directory,
> but the default value is "~/".  Before I start bisecting my init, does
> anything else occur to you?

That's the thing: I've tried a few different things which could have 
helped reproduce this, but they didn't.

> As an aside, it doesn't seem good that whether or not step four prompts
> you for a directory depends on whether the current buffer happens to be
> visiting a file or not.  Can we improve that, or is it inherent to the
> design of the new feature?

More like it's inherent to how the command you are calling is designed: 
it checks for the backend of the current buffer and behaves differently 
depending on whether one is found.

Since project-prefix-or-any-command works in the current buffer, it 
cannot really reset every such local variable. OTOH, it could 
temporarily switch to an empty buffer. In that case, however, any 
thing-at-point functionality won't work (and the invoked command might 
want to use the symbol at point as some default input or etc). At the 
time we discussed this, the latter seemed like a bigger problem.

Like Juri said, this particular disparity can be configured away, but 
different oddities in that class are bound to come up again.

bug#67171: 30.0.50; (At least) some VC commands fail with project-prefix-or-any-command

[Juri Linkov]

>>> As an aside, it doesn't seem good that whether or not step four prompts
>>> you for a directory depends on whether the current buffer happens to be
>>> visiting a file or not.  Can we improve that, or is it inherent to the
>>> design of the new feature?
>> This was improved recently in bug#67145, so now you can set
>> 'vc-deduce-backend-nonvc-modes' to nil to avoid this prompt.
>> I'm not sure if nil should be the default value.
>
> I wonder if the "deduce in all modes" value should be t rather than nil.

Good idea since nil could be used to disable deduction completely.
Then maybe it should be defcustom?

bug#67171: 30.0.50; (At least) some VC commands fail with project-prefix-or-any-command

[Dmitry Gutov]

On 24/11/2023 09:46, Juri Linkov wrote:
>>>> As an aside, it doesn't seem good that whether or not step four prompts
>>>> you for a directory depends on whether the current buffer happens to be
>>>> visiting a file or not.  Can we improve that, or is it inherent to the
>>>> design of the new feature?
>>> This was improved recently in bug#67145, so now you can set
>>> 'vc-deduce-backend-nonvc-modes' to nil to avoid this prompt.
>>> I'm not sure if nil should be the default value.
>> I wonder if the "deduce in all modes" value should be t rather than nil.
> Good idea since nil could be used to disable deduction completely.
> Then maybe it should be defcustom?

Why not.

bug#67171: 30.0.50; (At least) some VC commands fail with project-prefix-or-any-command

[Juri Linkov]

[-- Attachment #1: Type: text/plain, Size: 678 bytes --]

>>>>> As an aside, it doesn't seem good that whether or not step four prompts
>>>>> you for a directory depends on whether the current buffer happens to be
>>>>> visiting a file or not.  Can we improve that, or is it inherent to the
>>>>> design of the new feature?
>>>> This was improved recently in bug#67145, so now you can set
>>>> 'vc-deduce-backend-nonvc-modes' to nil to avoid this prompt.
>>>> I'm not sure if nil should be the default value.
>>> I wonder if the "deduce in all modes" value should be t rather than nil.
>> Good idea since nil could be used to disable deduction completely.
>> Then maybe it should be defcustom?
>
> Why not.

Maybe something like this:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vc-deduce-backend-nonvc-modes.patch --]
[-- Type: text/x-diff, Size: 1326 bytes --]

diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index 1bd9ecb2193..bde22536d1f 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -1071,19 +1071,23 @@ log-edit-vc-backend
 (defvar diff-vc-backend)
 (defvar diff-vc-revisions)
 
-;; Maybe we could even use comint-mode rather than shell-mode?
-(defvar vc-deduce-backend-nonvc-modes
+(defcustom vc-deduce-backend-nonvc-modes
+  ;; Maybe we could even use comint-mode rather than shell-mode?
   '(dired-mode shell-mode eshell-mode compilation-mode)
   "List of modes not supported by VC where backend should be deduced.
 In these modes the backend is deduced based on `default-directory'.
-When nil, the backend is deduced in all modes.")
+When t, the backend is deduced in all modes."
+  :type '(choice (const :tag "None" nil)
+		 hook
+		 (const :tag "All" t))
+  :version "30.1")
 
 (defun vc-deduce-backend ()
   (cond ((derived-mode-p 'vc-dir-mode)   vc-dir-backend)
 	((derived-mode-p 'log-view-mode) log-view-vc-backend)
 	((derived-mode-p 'log-edit-mode) log-edit-vc-backend)
 	((derived-mode-p 'diff-mode)     diff-vc-backend)
-	((or (null vc-deduce-backend-nonvc-modes)
+	((or (eq vc-deduce-backend-nonvc-modes t)
 	     (derived-mode-p vc-deduce-backend-nonvc-modes))
 	 (ignore-errors (vc-responsible-backend default-directory)))
 	(vc-mode (vc-backend buffer-file-name))))

bug#67171: 30.0.50; (At least) some VC commands fail with project-prefix-or-any-command

[Dmitry Gutov]

On 25/11/2023 20:39, Juri Linkov wrote:
> +		 hook

This one looks a bit out of place, but if it simply means "a function", 
that's ok.

bug#67171: 30.0.50; (At least) some VC commands fail with project-prefix-or-any-command

[Juri Linkov]

>> +		 hook
>
> This one looks a bit out of place, but if it simply means "a function",
> that's ok.

Actually, it's a shorthand for "a list of functions".

bug#67171: 30.0.50; (At least) some VC commands fail with project-prefix-or-any-command

[Eli Zaretskii]

> Cc: 67171@debbugs.gnu.org, sbaugh@catern.com,
>  Sean Whitton <spwhitton@spwhitton.name>
> From: Juri Linkov <juri@linkov.net>
> Date: Sat, 25 Nov 2023 20:39:37 +0200
> 
> +(defcustom vc-deduce-backend-nonvc-modes
> +  ;; Maybe we could even use comint-mode rather than shell-mode?
>    '(dired-mode shell-mode eshell-mode compilation-mode)
>    "List of modes not supported by VC where backend should be deduced.
>  In these modes the backend is deduced based on `default-directory'.
> -When nil, the backend is deduced in all modes.")
> +When t, the backend is deduced in all modes."

Please don't use "when" in these contexts; use "if" instead.  "When"
could be confusing because it has the meaning of time, not only of
condition.

Also, it is better to say "If the value is t" or "If this is t",
instead of just "If t".

bug#67171: 30.0.50; (At least) some VC commands fail with project-prefix-or-any-command

[Dmitry Gutov]

On 25/11/2023 21:10, Juri Linkov wrote:
>>> +		 hook
>> This one looks a bit out of place, but if it simply means "a function",
>> that's ok.
> Actually, it's a shorthand for "a list of functions".

Ah, perfect.

bug#67171: 30.0.50; (At least) some VC commands fail with project-prefix-or-any-command

[Juri Linkov]

>> +(defcustom vc-deduce-backend-nonvc-modes
>> +  ;; Maybe we could even use comint-mode rather than shell-mode?
>>    '(dired-mode shell-mode eshell-mode compilation-mode)
>>    "List of modes not supported by VC where backend should be deduced.
>>  In these modes the backend is deduced based on `default-directory'.
>> -When nil, the backend is deduced in all modes.")
>> +When t, the backend is deduced in all modes."
>
> Please don't use "when" in these contexts; use "if" instead.  "When"
> could be confusing because it has the meaning of time, not only of
> condition.
>
> Also, it is better to say "If the value is t" or "If this is t",
> instead of just "If t".

Now changed to defcustom with these fixes.