bug#69242: project-any-command with overriding-local-map

bug#69242: project-any-command with overriding-local-map

[Juri Linkov]

With 'project-switch-commands' customized to 'project-prefix-or-any-command'
tried to do such a useful thing as applying a diff to a sibling project
with the same source tree:

C-x v =
C-x p p RET
C-c RET a     (diff-apply-buffer)

But it failed with an undefined key.

And indeed evaluating in a *vc-diff* buffer:

  M-: (let ((overriding-local-map project-prefix-map)) (key-binding (read-key-sequence "? ") t))

returns nil for any longer than 1-key binding such as 'C-c C-a' or 'C-c RET a'.

Then found this patch fixes the problem completely (no idea why):

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index aa92a73336e..fe866312931 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -1871,7 +1874,7 @@ project-any-command
   (interactive)
   (let* ((pr (project-current t))
          (prompt-format (or prompt-format "[execute in %s]:"))
-         (command (let ((overriding-local-map overriding-map))
+         (command (let ((overriding-terminal-local-map overriding-map))
                     (key-binding (read-key-sequence
                                   (format prompt-format (project-root pr)))
                                  t)))

bug#69242: project-any-command with overriding-local-map

[Dmitry Gutov]

On 17/02/2024 19:17, Juri Linkov wrote:
> And indeed evaluating in a*vc-diff*  buffer:
> 
>    M-: (let ((overriding-local-map project-prefix-map)) (key-binding (read-key-sequence "? ") t))
> 
> returns nil for any longer than 1-key binding such as 'C-c C-a' or 'C-c RET a'.

I suppose that's because you are entering a sequence that belongs to a 
local map (diff-mode-map), and overriding-local-map overrides them 
entirely ("INSTEAD OF" in the docstring), rather than having a higher 
priority, which is overriding-terminal-local-map does.

> Then found this patch fixes the problem completely (no idea why):
> 
> diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
> index aa92a73336e..fe866312931 100644
> --- a/lisp/progmodes/project.el
> +++ b/lisp/progmodes/project.el
> @@ -1871,7 +1874,7 @@ project-any-command
>     (interactive)
>     (let* ((pr (project-current t))
>            (prompt-format (or prompt-format "[execute in %s]:"))
> -         (command (let ((overriding-local-map overriding-map))
> +         (command (let ((overriding-terminal-local-map overriding-map))
>                       (key-binding (read-key-sequence
>                                     (format prompt-format (project-root pr)))
>                                    t)))

LGTM, thanks. Let's see if this one triggers any other edge cases we 
didn't test for.

bug#69242: project-any-command with overriding-local-map

[Juri Linkov]

>> And indeed evaluating in a*vc-diff*  buffer:
>>    M-: (let ((overriding-local-map project-prefix-map)) (key-binding
>> (read-key-sequence "? ") t))
>> returns nil for any longer than 1-key binding such as 'C-c C-a' or 'C-c
>> RET a'.
>
> I suppose that's because you are entering a sequence that belongs to
> a local map (diff-mode-map), and overriding-local-map overrides them
> entirely ("INSTEAD OF" in the docstring), rather than having a higher
> priority, which is overriding-terminal-local-map does.
>
>> Then found this patch fixes the problem completely (no idea why):
>> @@ -1871,7 +1874,7 @@ project-any-command
>>     (interactive)
>>     (let* ((pr (project-current t))
>>            (prompt-format (or prompt-format "[execute in %s]:"))
>> -         (command (let ((overriding-local-map overriding-map))
>> +         (command (let ((overriding-terminal-local-map overriding-map))
>>                       (key-binding (read-key-sequence
>>                                     (format prompt-format (project-root pr)))
>>                                    t)))
>
> LGTM, thanks. Let's see if this one triggers any other edge cases we didn't
> test for.

There is another use of 'overriding-local-map' in project.el:
in 'project--switch-project-command'.  Not sure if this should be
replaced with 'overriding-terminal-local-map' as well.

Also not sure how 'overriding-terminal-local-map' will affect
'universal-argument' that relies on 'overriding-terminal-local-map'.
Maybe project.el should use

  (internal-push-keymap map 'overriding-terminal-local-map)

like in 'set-transient-map'.  Ok, need to try and test this more.

bug#69242: project-any-command with overriding-local-map

[Dmitry Gutov]

On 20/02/2024 09:49, Juri Linkov wrote:
> There is another use of 'overriding-local-map' in project.el:
> in 'project--switch-project-command'.  Not sure if this should be
> replaced with 'overriding-terminal-local-map' as well.

Probably not: there is no intention to use any of the local maps there, 
AFAIR.

> Also not sure how 'overriding-terminal-local-map' will affect
> 'universal-argument' that relies on 'overriding-terminal-local-map'.
> Maybe project.el should use
> 
>    (internal-push-keymap map 'overriding-terminal-local-map)
> 
> like in 'set-transient-map'.  Ok, need to try and test this more.

Please do.

bug#69242: project-any-command with overriding-local-map

[Juri Linkov]

close 69242 30.0.50
thanks

> LGTM, thanks. Let's see if this one triggers any other edge cases we didn't
> test for.

Ok, now pushed to master.

>> Also not sure how 'overriding-terminal-local-map' will affect
>> 'universal-argument' that relies on 'overriding-terminal-local-map'.
>> Maybe project.el should use
>>    (internal-push-keymap map 'overriding-terminal-local-map)
>> like in 'set-transient-map'.  Ok, need to try and test this more.
>
> Please do.

Regarding 'universal-argument', I discovered that currently e.g.
'C-x p p RET C-u C-c C-a' keeps the argument, but loses default-directory
(i.e. sets default-directory back to the old directory).

So tried a patch below, and it keeps default-directory,
but loses the argument.

@@ -1883,7 +1886,11 @@ project-any-command
           (let ((project-current-directory-override root))
             (call-interactively command))
         (let ((default-directory root))
-          (call-interactively command))))))
+          (call-interactively command)))
+      (when (memq command
+                  '( universal-argument universal-argument-more
+                     digit-argument negative-argument))
+        (project-any-command overriding-map prompt-format)))))

Then tried 'C-u C-x p p RET C-c C-a' and it keeps default-directory
and also keeps the C-u argument for 'C-c C-a' after switching the project.

This means no more changes needed because the above key sequence works nicely.
So now closing.