bug#70577: [PATCH] New command other-project-prefix

bug#70577: [PATCH] New command other-project-prefix

[Dmitry Gutov]

X-Debbugs-Cc: Juri Linkov <juri@linkov.net>

This is based on Juri's patch in 
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=63648#161, but the idea is 
more focused: to switch the order of events, and first read the full key 
sequence, and then prompt for the project and the command arguments. 
Like we also discussed in the past.

And to try to reuse the even loop in the more natural way. 
Unfortunately, 'C-h' doesn't work here (when called in the middle of the 
sequence) - I'm not sure why. The rest of the behavior seems to work as 
expected.

So this can be a new alternative for the 'C-x p p' binding as well.

Regarding the the use of advice, I didn't find a better way to plug 
(funcall project-prompter) this late. Too complex for pre-command-hook.

Thoughts welcome.

bug#70577: [PATCH] New command other-project-prefix

[Juri Linkov]

> This is based on Juri's patch in
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=63648#161, but the idea is
> more focused: to switch the order of events, and first read the full key
> sequence, and then prompt for the project and the command arguments. Like
> we also discussed in the past.

I'm not a fan of reading the full key sequence bypassing the event loop.

> And to try to reuse the even loop in the more natural way. Unfortunately,
> 'C-h' doesn't work here (when called in the middle of the sequence) - I'm
> not sure why. The rest of the behavior seems to work as expected.

'C-h' can't work since 'C-x p p' is bound to a command.

> So this can be a new alternative for the 'C-x p p' binding as well.

I guess there could be 2 new alternative options for 'project-switch-commands':

1. read the full key sequence
2. use the event loop with set-transient-map

Although I'm already completely content with the existing option
'project-prefix-or-any-command' of 'project-switch-commands'.

bug#70577: [PATCH] New command other-project-prefix

[Dmitry Gutov]

On 26/04/2024 09:09, Juri Linkov wrote:
>> This is based on Juri's patch in
>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=63648#161, but the idea is
>> more focused: to switch the order of events, and first read the full key
>> sequence, and then prompt for the project and the command arguments. Like
>> we also discussed in the past.
> 
> I'm not a fan of reading the full key sequence bypassing the event loop.

That's what the current code does. While the patch tries to change that.

>> And to try to reuse the even loop in the more natural way. Unfortunately,
>> 'C-h' doesn't work here (when called in the middle of the sequence) - I'm
>> not sure why. The rest of the behavior seems to work as expected.
> 
> 'C-h' can't work since 'C-x p p' is bound to a command.

Hmm, I wonder how hard it'd be to change that.

>> So this can be a new alternative for the 'C-x p p' binding as well.
> 
> I guess there could be 2 new alternative options for 'project-switch-commands':
> 
> 1. read the full key sequence
> 2. use the event loop with set-transient-map
> 
> Although I'm already completely content with the existing option
> 'project-prefix-or-any-command' of 'project-switch-commands'.

This one is indeed an experiment. It just seems that being able to type 
the full sequence first is more ergonomically advantageous.

But overall this (adding a +1 alternative) is probably only worth it if 
we can make 'C-h' work like normal.

bug#70577: [PATCH] New command other-project-prefix

[Dmitry Gutov]

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

On 26/04/2024 13:59, Dmitry Gutov wrote:
> On 26/04/2024 09:09, Juri Linkov wrote:
>>> This is based on Juri's patch in
>>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=63648#161, but the idea is
>>> more focused: to switch the order of events, and first read the full key
>>> sequence, and then prompt for the project and the command arguments. 
>>> Like
>>> we also discussed in the past.
>>
>> I'm not a fan of reading the full key sequence bypassing the event loop.
> 
> That's what the current code does. While the patch tries to change that.

Sorry, I forgot to attach the actual patch.

[-- Attachment #2: other-project-prefix.diff --]
[-- Type: text/x-patch, Size: 2307 bytes --]

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index 000a05804a8..f25403c982c 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -952,6 +952,44 @@ project-other-tab-command
 (when (bound-and-true-p tab-prefix-map)
   (define-key tab-prefix-map "p" #'project-other-tab-command))
 
+;;;###autoload
+(defun other-project-prefix ()
+  "\"Switch\" to another project before running an Emacs command.
+Makes sure the next command invoked asks for the project to run it in."
+  (interactive)
+  (prefix-command-preserve-state)
+  (letrec ((depth (minibuffer-depth))
+           (echofun (lambda () "[switch-project]"))
+           (around-fun
+            (lambda (command &rest _args)
+              (advice-remove this-command around-fun)
+              (unless (or (eq this-command 'other-project-prefix)
+                          (eq last-command-event help-char))
+                (let ((root (funcall project-prompter)))
+                  (if (or (string-prefix-p "project-"
+                                           (symbol-name this-command))
+                          (get this-command 'project-aware))
+                      (let ((project-current-directory-override root))
+                        (call-interactively command))
+                    (let ((default-directory root))
+                      (call-interactively command)))))))
+           (prefun
+            (lambda ()
+              (unless (> (minibuffer-depth) depth)
+                (remove-hook 'pre-command-hook prefun)
+                (remove-hook 'prefix-command-echo-keystrokes-functions echofun)
+                (when (and this-command (symbolp this-command))
+                  (advice-add this-command :around around-fun))))))
+    (add-hook 'pre-command-hook prefun)
+    (add-hook 'prefix-command-echo-keystrokes-functions echofun)
+    (let ((map (make-sparse-keymap)))
+      (set-keymap-parent map project-prefix-map)
+      ;; Doesn't work ;-(
+      ;; (define-key map (vector help-char)
+      ;;             (lambda () (interactive) (help-form-show)))
+      (set-transient-map map))
+    (message (concat "Type " (project--keymap-prompt) " or any global key"))))
+
 (declare-function grep-read-files "grep")
 (declare-function xref--find-ignores-arguments "xref")
 

bug#70577: [PATCH] New command other-project-prefix

[Sean Whitton via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Hello,

On Fri 26 Apr 2024 at 07:20pm +03, Dmitry Gutov wrote:

> +;;;###autoload
> +(defun other-project-prefix ()
> +  "\"Switch\" to another project before running an Emacs command.
> +Makes sure the next command invoked asks for the project to run it in."
> +  (interactive)
> +  (prefix-command-preserve-state)
> +  (letrec ((depth (minibuffer-depth))
> +           (echofun (lambda () "[switch-project]"))
> +           (around-fun
> +            (lambda (command &rest _args)
> +              (advice-remove this-command around-fun)
> +              (unless (or (eq this-command 'other-project-prefix)
> +                          (eq last-command-event help-char))
> +                (let ((root (funcall project-prompter)))
> +                  (if (or (string-prefix-p "project-"
> +                                           (symbol-name this-command))
> +                          (get this-command 'project-aware))
> +                      (let ((project-current-directory-override root))
> +                        (call-interactively command))
> +                    (let ((default-directory root))
> +                      (call-interactively command)))))))
> +           (prefun
> +            (lambda ()
> +              (unless (> (minibuffer-depth) depth)
> +                (remove-hook 'pre-command-hook prefun)
> +                (remove-hook 'prefix-command-echo-keystrokes-functions echofun)
> +                (when (and this-command (symbolp this-command))
> +                  (advice-add this-command :around around-fun))))))
> +    (add-hook 'pre-command-hook prefun)
> +    (add-hook 'prefix-command-echo-keystrokes-functions echofun)
> +    (let ((map (make-sparse-keymap)))
> +      (set-keymap-parent map project-prefix-map)
> +      ;; Doesn't work ;-(
> +      ;; (define-key map (vector help-char)
> +      ;;             (lambda () (interactive) (help-form-show)))
> +      (set-transient-map map))
> +    (message (concat "Type " (project--keymap-prompt) " or any global key"))))

In passing, this pattern where you letrec a hook that removes itself,
and also consider minibuffer-depth, is now in several places.
The one I am thinking of is vc-edit-next-command but I based that on
some code of Juri's somewhere.

It would be good to factor out a macro for this pattern, I think.

-- 
Sean Whitton

bug#70577: [PATCH] New command other-project-prefix

[Dmitry Gutov]

On 28/04/2024 15:13, Sean Whitton wrote:
> In passing, this pattern where you letrec a hook that removes itself,
> and also consider minibuffer-depth, is now in several places.
> The one I am thinking of is vc-edit-next-command but I based that on
> some code of Juri's somewhere.
> 
> It would be good to factor out a macro for this pattern, I think.

Makes sense. Maybe a helper function, not necessarily a macro.

Re: bug#70577: [PATCH] New command other-project-prefix

[Dmitry Gutov]

On 28/04/2024 15:13, Sean Whitton wrote:
> In passing, this pattern where you letrec a hook that removes itself,
> and also consider minibuffer-depth, is now in several places.
> The one I am thinking of is vc-edit-next-command but I based that on
> some code of Juri's somewhere.
> 
> It would be good to factor out a macro for this pattern, I think.

Makes sense. Maybe a helper function, not necessarily a macro.

Re: bug#70577: [PATCH] New command other-project-prefix

[Juri Linkov]

>> +(defun other-project-prefix ()
>> +  "\"Switch\" to another project before running an Emacs command.
>> +Makes sure the next command invoked asks for the project to run it in."
>> +  (interactive)
>> +  (prefix-command-preserve-state)
>> +  (letrec ((depth (minibuffer-depth))
>
> In passing, this pattern where you letrec a hook that removes itself,
> and also consider minibuffer-depth, is now in several places.
> The one I am thinking of is vc-edit-next-command but I based that on
> some code of Juri's somewhere.
>
> It would be good to factor out a macro for this pattern, I think.

'display-buffer-override-next-command' doesn't contain 'letrec', but
uses the same pattern, so it could benefit from the new macro indeed.

bug#70577: [PATCH] New command other-project-prefix

[Juri Linkov]

> On 26/04/2024 13:59, Dmitry Gutov wrote:
>> On 26/04/2024 09:09, Juri Linkov wrote:
>>>> This is based on Juri's patch in
>>>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=63648#161, but the idea is
>>>> more focused: to switch the order of events, and first read the full key
>>>> sequence, and then prompt for the project and the command
>>>> arguments. Like
>>>> we also discussed in the past.
>>>
>>> I'm not a fan of reading the full key sequence bypassing the event loop.
>> That's what the current code does. While the patch tries to change that.
>
> Sorry, I forgot to attach the actual patch.

Thanks.

> +(defun other-project-prefix ()

Something is wrong here.  I bound 'other-project-prefix' to 'C-x p P'.
Then typing 'C-x p P C-x d' asked a directory name, then later
after selecting a project asked for the directory name again.
Then some advice remains unremoved.  Ok, will test more.

bug#70577: [PATCH] New command other-project-prefix

[Dmitry Gutov]

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

On 28/04/2024 19:51, Juri Linkov wrote:
>> On 26/04/2024 13:59, Dmitry Gutov wrote:
>>> On 26/04/2024 09:09, Juri Linkov wrote:
>>>>> This is based on Juri's patch in
>>>>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=63648#161, but the idea is
>>>>> more focused: to switch the order of events, and first read the full key
>>>>> sequence, and then prompt for the project and the command
>>>>> arguments. Like
>>>>> we also discussed in the past.
>>>>
>>>> I'm not a fan of reading the full key sequence bypassing the event loop.
>>> That's what the current code does. While the patch tries to change that.
>>
>> Sorry, I forgot to attach the actual patch.
> 
> Thanks.
> 
>> +(defun other-project-prefix ()
> 
> Something is wrong here.  I bound 'other-project-prefix' to 'C-x p P'.
> Then typing 'C-x p P C-x d' asked a directory name, then later
> after selecting a project asked for the directory name again.

Looks like that has to do with the interactive spec. See the attached 
next revision, it seems to behave better.

> Then some advice remains unremoved.  Ok, will test more.

I haven't noticed this particular problem, so please write down a repro 
if you find one.

[-- Attachment #2: other-project-prefix-v2.diff --]
[-- Type: text/x-patch, Size: 2416 bytes --]

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index 000a05804a8..feb31caba4d 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -952,6 +952,47 @@ project-other-tab-command
 (when (bound-and-true-p tab-prefix-map)
   (define-key tab-prefix-map "p" #'project-other-tab-command))
 
+;;;###autoload
+(defun other-project-prefix ()
+  "\"Switch\" to another project before running an Emacs command.
+Makes sure the next command invoked asks for the project to run it in."
+  (interactive)
+  (prefix-command-preserve-state)
+  (letrec ((depth (minibuffer-depth))
+           (echofun (lambda () "[switch-project]"))
+           (around-fun
+            (lambda (command &rest _args)
+              (interactive #'ignore)
+              (advice-remove this-command around-fun)
+              (unless (or (eq this-command 'other-project-prefix)
+                          (eq last-command-event help-char))
+                (let ((root (funcall project-prompter)))
+                  (if (or (string-prefix-p "project-"
+                                           (symbol-name this-command))
+                          (get this-command 'project-aware))
+                      (let ((project-current-directory-override root))
+                        (call-interactively command))
+                    (let ((default-directory root))
+                      (call-interactively command)))))))
+           (prefun
+            (lambda ()
+              (unless (> (minibuffer-depth) depth)
+                (remove-hook 'pre-command-hook prefun)
+                (remove-hook 'prefix-command-echo-keystrokes-functions echofun)
+                (when (and this-command (symbolp this-command))
+                  (advice-add this-command :around around-fun))))))
+    (add-hook 'pre-command-hook prefun)
+    (add-hook 'prefix-command-echo-keystrokes-functions echofun)
+    (let ((map (make-sparse-keymap)))
+      (set-keymap-parent map project-prefix-map)
+      ;; Doesn't work ;-(
+      ;; (define-key map (vector help-char)
+      ;;             (lambda () (interactive) (help-form-show)))
+      (set-transient-map map))
+    (message (concat "Type " (project--keymap-prompt) " or any global key"))))
+
+;; (define-key project-prefix-map (kbd "P") #'other-project-prefix)
+
 (declare-function grep-read-files "grep")
 (declare-function xref--find-ignores-arguments "xref")
 

bug#70577: [PATCH] New command other-project-prefix

[Juri Linkov]

>> Something is wrong here.  I bound 'other-project-prefix' to 'C-x p P'.
>> Then typing 'C-x p P C-x d' asked a directory name, then later
>> after selecting a project asked for the directory name again.
>
> Looks like that has to do with the interactive spec. See the attached next
> revision, it seems to behave better.

Thanks, this works now (except that it can't be debugged because of the
Lisp error: (wrong-type-argument listp ignore)).

Also 'C-h' is not a problem: 'help-form-show' does nothing
without 'help-form', but with 'help-form' works fine:

      (define-key map (vector help-char)
                  (lambda ()
                    (interactive)
                    (let ((help-form "You can use any global keybinding."))
                      (help-form-show))))

However, a much bigger problem is that unfortunately many test cases from
https://debbugs.gnu.org/63648#203 are broken.  For example,
'C-x p p C-b' fails the same way as in bug#58784.
'C-x p p f M-n' fails because it expects to read arguments
in a previous project with an old value of default-directory, etc.

Maybe this could be fixed by running 'interactive' in a previous project
by using something like:

  (around-fun
   (lambda (command &rest _args)
     (interactive (lambda (spec)
                    (let ((default-directory prev-dir))
                      (advice-eval-interactive-spec spec))))

bug#70577: [PATCH] New command other-project-prefix

[Dmitry Gutov]

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

On 02/05/2024 09:12, Juri Linkov wrote:
>>> Something is wrong here.  I bound 'other-project-prefix' to 'C-x p P'.
>>> Then typing 'C-x p P C-x d' asked a directory name, then later
>>> after selecting a project asked for the directory name again.
>>
>> Looks like that has to do with the interactive spec. See the attached next
>> revision, it seems to behave better.
> 
> Thanks, this works now (except that it can't be debugged because of the
> Lisp error: (wrong-type-argument listp ignore)).
> 
> Also 'C-h' is not a problem: 'help-form-show' does nothing
> without 'help-form', but with 'help-form' works fine:
> 
>        (define-key map (vector help-char)
>                    (lambda ()
>                      (interactive)
>                      (let ((help-form "You can use any global keybinding."))
>                        (help-form-show))))

We would want 'C-h' to show the regular buffer with key bindings, won't 
we? With similar output to the one that we get after 'C-x p C-h' or 'C-x 
v C-h'. The output might be weirder because of the composed keymap, but 
it could still be useful.

Also, with which-key-mode, C-h would do its thing.

> However, a much bigger problem is that unfortunately many test cases from
> https://debbugs.gnu.org/63648#203 are broken.  For example,
> 'C-x p p C-b' fails the same way as in bug#58784.
> 'C-x p p f M-n' fails because it expects to read arguments
> in a previous project with an old value of default-directory, etc.

Thanks for noticing. Looks like the call to project-prompter can change 
the value of this-command, and that's why the subsequent check went down 
the wrong branch. See the attached v3 with the fix.

> Maybe this could be fixed by running 'interactive' in a previous project
> by using something like:
> 
>    (around-fun
>     (lambda (command &rest _args)
>       (interactive (lambda (spec)
>                      (let ((default-directory prev-dir))
>                        (advice-eval-interactive-spec spec))))

I think the command might rather expect to be called in the "new" 
project. And also while some have interactive specs with significant 
logic inside, others don't; introducing a difference there could cause 
more problems.

[-- Attachment #2: other-project-prefix-v3.diff --]
[-- Type: text/x-patch, Size: 2478 bytes --]

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index 000a05804a8..1ee6e5848a3 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -952,6 +952,48 @@ project-other-tab-command
 (when (bound-and-true-p tab-prefix-map)
   (define-key tab-prefix-map "p" #'project-other-tab-command))
 
+;;;###autoload
+(defun other-project-prefix ()
+  "\"Switch\" to another project before running an Emacs command.
+Makes sure the next command invoked asks for the project to run it in."
+  (interactive)
+  (prefix-command-preserve-state)
+  (letrec ((depth (minibuffer-depth))
+           (echofun (lambda () "[switch-project]"))
+           (around-fun
+            (lambda (command &rest _args)
+              (interactive)
+              (advice-remove this-command around-fun)
+              (unless (or (eq this-command 'other-project-prefix)
+                          (eq last-command-event help-char))
+                (let* ((this-command-saved this-command)
+                       (root (funcall project-prompter)))
+                  (if (or (string-prefix-p "project-"
+                                           (symbol-name this-command-saved))
+                          (get this-command-saved 'project-aware))
+                      (let ((project-current-directory-override root))
+                        (call-interactively command))
+                    (let ((default-directory root))
+                      (call-interactively command)))))))
+           (prefun
+            (lambda ()
+              (unless (> (minibuffer-depth) depth)
+                (remove-hook 'pre-command-hook prefun)
+                (remove-hook 'prefix-command-echo-keystrokes-functions echofun)
+                (when (and this-command (symbolp this-command))
+                  (advice-add this-command :around around-fun))))))
+    (add-hook 'pre-command-hook prefun)
+    (add-hook 'prefix-command-echo-keystrokes-functions echofun)
+    (let ((map (make-sparse-keymap)))
+      (set-keymap-parent map project-prefix-map)
+      ;; Doesn't work ;-(
+      ;; (define-key map (vector help-char)
+      ;;             (lambda () (interactive) (help-form-show)))
+      (set-transient-map map))
+    (message (concat "Type " (project--keymap-prompt) " or any global key"))))
+
+;; (define-key project-prefix-map (kbd "P") #'other-project-prefix)
+
 (declare-function grep-read-files "grep")
 (declare-function xref--find-ignores-arguments "xref")
 

bug#70577: [PATCH] New command other-project-prefix

[Eli Zaretskii]

> Cc: 70577@debbugs.gnu.org
> Date: Sat, 4 May 2024 05:12:39 +0300
> From: Dmitry Gutov <dmitry@gutov.dev>
> 
> +(defun other-project-prefix ()
> +  "\"Switch\" to another project before running an Emacs command.
> +Makes sure the next command invoked asks for the project to run it in."

This last sentence reads awkwardly and confusingly.  Suggest to reword

  The next command you invoke will prompt for the project in which to
  run the command.

Thanks.

bug#70577: [PATCH] New command other-project-prefix

[Dmitry Gutov]

On 04/05/2024 10:24, Eli Zaretskii wrote:
>> Cc:70577@debbugs.gnu.org
>> Date: Sat, 4 May 2024 05:12:39 +0300
>> From: Dmitry Gutov<dmitry@gutov.dev>
>>
>> +(defun other-project-prefix ()
>> +  "\"Switch\" to another project before running an Emacs command.
>> +Makes sure the next command invoked asks for the project to run it in."
> This last sentence reads awkwardly and confusingly.  Suggest to reword
> 
>    The next command you invoke will prompt for the project in which to
>    run the command.

Thanks, that sounds good.

Some ideas regarding 'C-h' behaving differently from the usual would be 
welcome, too.

bug#70577: [PATCH] New command other-project-prefix

[Eli Zaretskii]

> Date: Sat, 4 May 2024 20:22:39 +0300
> Cc: juri@linkov.net, 70577@debbugs.gnu.org
> From: Dmitry Gutov <dmitry@gutov.dev>
> 
> Some ideas regarding 'C-h' behaving differently from the usual would be 
> welcome, too.

You mean, what help--append-keystrokes-help does?  For that to work,
C-h should have no binding in the last keymap, AFAIR.

bug#70577: [PATCH] New command other-project-prefix

[Dmitry Gutov]

On 04/05/2024 20:34, Eli Zaretskii wrote:
>> Date: Sat, 4 May 2024 20:22:39 +0300
>> Cc:juri@linkov.net,70577@debbugs.gnu.org
>> From: Dmitry Gutov<dmitry@gutov.dev>
>>
>> Some ideas regarding 'C-h' behaving differently from the usual would be
>> welcome, too.
> You mean, what help--append-keystrokes-help does?  For that to work,
> C-h should have no binding in the last keymap, AFAIR.

As you can see in the attached patches, I don't add a C-h binding to the 
generated map.

And the text (`C-h' for help) does get printed, but pressing this key 
combination doesn't show help. That seems like a problem.

bug#70577: [PATCH] New command other-project-prefix

[Eli Zaretskii]

> Date: Sun, 5 May 2024 03:02:27 +0300
> Cc: juri@linkov.net, 70577@debbugs.gnu.org
> From: Dmitry Gutov <dmitry@gutov.dev>
> 
> On 04/05/2024 20:34, Eli Zaretskii wrote:
> >> Date: Sat, 4 May 2024 20:22:39 +0300
> >> Cc:juri@linkov.net,70577@debbugs.gnu.org
> >> From: Dmitry Gutov<dmitry@gutov.dev>
> >>
> >> Some ideas regarding 'C-h' behaving differently from the usual would be
> >> welcome, too.
> > You mean, what help--append-keystrokes-help does?  For that to work,
> > C-h should have no binding in the last keymap, AFAIR.
> 
> As you can see in the attached patches, I don't add a C-h binding to the 
> generated map.
> 
> And the text (`C-h' for help) does get printed, but pressing this key 
> combination doesn't show help. That seems like a problem.

Can you show a recipe that I could try with the current master to
reproduce this?   Then I could take a look.

bug#70577: [PATCH] New command other-project-prefix

[Dmitry Gutov]

On 05/05/2024 08:44, Eli Zaretskii wrote:
>> Date: Sun, 5 May 2024 03:02:27 +0300
>> Cc:juri@linkov.net,70577@debbugs.gnu.org
>> From: Dmitry Gutov<dmitry@gutov.dev>
>>
>> On 04/05/2024 20:34, Eli Zaretskii wrote:
>>>> Date: Sat, 4 May 2024 20:22:39 +0300
>>>> Cc:juri@linkov.net,70577@debbugs.gnu.org
>>>> From: Dmitry Gutov<dmitry@gutov.dev>
>>>>
>>>> Some ideas regarding 'C-h' behaving differently from the usual would be
>>>> welcome, too.
>>> You mean, what help--append-keystrokes-help does?  For that to work,
>>> C-h should have no binding in the last keymap, AFAIR.
>> As you can see in the attached patches, I don't add a C-h binding to the
>> generated map.
>>
>> And the text (`C-h' for help) does get printed, but pressing this key
>> combination doesn't show help. That seems like a problem.
> Can you show a recipe that I could try with the current master to
> reproduce this?   Then I could take a look.

Thanks, I've found the problem - it was caused by the specific code in 
the function, not something general (it skipped the invocation of 
COMMAND inside AROUND-FUN).

bug#70577: [PATCH] New command other-project-prefix

[Juri Linkov]

> We would want 'C-h' to show the regular buffer with key bindings, won't we?
> With similar output to the one that we get after 'C-x p C-h' or 'C-x
> v C-h'. The output might be weirder because of the composed keymap, but it
> could still be useful.

Then maybe something like

  (define-key map (vector help-char)
              (lambda () (interactive) (describe-bindings)))

or

  (define-key map (vector help-char)
              (lambda () (interactive) (describe-keymap (cons 'keymap (current-active-maps)))))

or

  (define-key map (vector help-char)
              (lambda () (interactive) (describe-keymap (cons 'keymap project-prefix-map))))

>> However, a much bigger problem is that unfortunately many test cases from
>> https://debbugs.gnu.org/63648#203 are broken.  For example,
>> 'C-x p p C-b' fails the same way as in bug#58784.
>> 'C-x p p f M-n' fails because it expects to read arguments
>> in a previous project with an old value of default-directory, etc.
>
> Thanks for noticing. Looks like the call to project-prompter can change the
> value of this-command, and that's why the subsequent check went down the
> wrong branch. See the attached v3 with the fix.

Wow, everything works now, will test more as a primary 'C-x p p' command.

bug#70577: [PATCH] New command other-project-prefix

[Dmitry Gutov]

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

On 05/05/2024 19:40, Juri Linkov wrote:
>> We would want 'C-h' to show the regular buffer with key bindings, won't we?
>> With similar output to the one that we get after 'C-x p C-h' or 'C-x
>> v C-h'. The output might be weirder because of the composed keymap, but it
>> could still be useful.
> 
> Then maybe something like
> 
>    (define-key map (vector help-char)
>                (lambda () (interactive) (describe-bindings)))
> 
> or
> 
>    (define-key map (vector help-char)
>                (lambda () (interactive) (describe-keymap (cons 'keymap (current-active-maps)))))
> 
> or
> 
>    (define-key map (vector help-char)
>                (lambda () (interactive) (describe-keymap (cons 'keymap project-prefix-map))))

This actually seems unnecessary. See the attached latest version where 
the binding works automatically without explicit assignment.

>>> However, a much bigger problem is that unfortunately many test cases from
>>> https://debbugs.gnu.org/63648#203 are broken.  For example,
>>> 'C-x p p C-b' fails the same way as in bug#58784.
>>> 'C-x p p f M-n' fails because it expects to read arguments
>>> in a previous project with an old value of default-directory, etc.
>>
>> Thanks for noticing. Looks like the call to project-prompter can change the
>> value of this-command, and that's why the subsequent check went down the
>> wrong branch. See the attached v3 with the fix.
> 
> Wow, everything works now, will test more as a primary 'C-x p p' command.

Thanks, let me know if you find any other problems.

[-- Attachment #2: other-project-prefix-v4.diff --]
[-- Type: text/x-patch, Size: 2322 bytes --]

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index 52fe4df9080..e41a23fe13d 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -967,6 +967,45 @@ project-other-tab-command
 (when (bound-and-true-p tab-prefix-map)
   (define-key tab-prefix-map "p" #'project-other-tab-command))
 
+;;;###autoload
+(defun other-project-prefix ()
+  "\"Switch\" to another project before running an Emacs command.
+The next command you invoke will prompt for the project in which to run
+the command."
+  (interactive)
+  (prefix-command-preserve-state)
+  (letrec ((depth (minibuffer-depth))
+           (echofun (lambda () "[switch-project]"))
+           (around-fun
+            (lambda (command &rest _args)
+              (interactive)
+              (advice-remove this-command around-fun)
+              (if (or (eq this-command 'other-project-prefix)
+                          (eq last-command-event help-char))
+                  (call-interactively command)
+                (let* ((this-command-saved this-command)
+                       (root (funcall project-prompter)))
+                  (if (or (string-prefix-p "project-"
+                                           (symbol-name this-command-saved))
+                          (get this-command-saved 'project-aware))
+                      (let ((project-current-directory-override root))
+                        (call-interactively command))
+                    (let ((default-directory root))
+                      (call-interactively command)))))))
+           (prefun
+            (lambda ()
+              (unless (> (minibuffer-depth) depth)
+                (remove-hook 'pre-command-hook prefun)
+                (remove-hook 'prefix-command-echo-keystrokes-functions echofun)
+                (when (and this-command (symbolp this-command))
+                  (advice-add this-command :around around-fun))))))
+    (add-hook 'pre-command-hook prefun)
+    (add-hook 'prefix-command-echo-keystrokes-functions echofun)
+    (set-transient-map project-prefix-map)
+    (message (concat "Type " (project--keymap-prompt) " or any global key"))))
+
+;; (define-key project-prefix-map (kbd "P") #'other-project-prefix)
+
 (declare-function grep-read-files "grep")
 (declare-function xref--find-ignores-arguments "xref")
 

bug#70577: [PATCH] New command other-project-prefix

[Juri Linkov]

>> Wow, everything works now, will test more as a primary 'C-x p p' command.
>
> Thanks, let me know if you find any other problems.

I confirm that everything works nicely, thanks.  The only
problem is that after trying to use it, its order looks unnatural.
I already accustomed to this order: 1) select the project,
2) run the command.  This is handy especially with
project-switch-commands set to 'project-prefix-or-any-command'.
This order looks more logical because after selecting the
project, the user mentally switches to another project, and
then types a command with arguments in the switched project.
However, the reverse order of typing a command keys
before switching the project looks like trying
to run the command in the previous project.
Also the problem is that typing a command keys and reading
the command arguments is separated by reading a project.

Maybe many users would prefer other-project-prefix, I don't know.
But other-project-prefix can't replace project-switch-project,
only to be an alternative.

bug#70577: [PATCH] New command other-project-prefix

[Juri Linkov]

>>> Wow, everything works now, will test more as a primary 'C-x p p' command.
>>
>> Thanks, let me know if you find any other problems.
>
> I confirm that everything works nicely, thanks.  The only
> problem is that after trying to use it, its order looks unnatural.
> I already accustomed to this order: 1) select the project,
> 2) run the command.  This is handy especially with
> project-switch-commands set to 'project-prefix-or-any-command'.
> This order looks more logical because after selecting the
> project, the user mentally switches to another project, and
> then types a command with arguments in the switched project.
> However, the reverse order of typing a command keys
> before switching the project looks like trying
> to run the command in the previous project.
> Also the problem is that typing a command keys and reading
> the command arguments is separated by reading a project.
>
> Maybe many users would prefer other-project-prefix, I don't know.
> But other-project-prefix can't replace project-switch-project,
> only to be an alternative.

Sorry, I didn't realize that implementation of other-project-prefix
can be changed to read the project before reading the command
with arguments:

  (defun other-project-prefix ()
    (letrec ((root (funcall project-prompter))
             (depth (minibuffer-depth))
             (echofun (lambda () "[switch-project]"))
             (around-fun
              ...

bug#70577: [PATCH] New command other-project-prefix

[Dmitry Gutov]

On 06/05/2024 21:30, Juri Linkov wrote:
>>>> Wow, everything works now, will test more as a primary 'C-x p p' command.
>>> Thanks, let me know if you find any other problems.
>> I confirm that everything works nicely, thanks.  The only
>> problem is that after trying to use it, its order looks unnatural.
>> I already accustomed to this order: 1) select the project,
>> 2) run the command.  This is handy especially with
>> project-switch-commands set to 'project-prefix-or-any-command'.
>> This order looks more logical because after selecting the
>> project, the user mentally switches to another project, and
>> then types a command with arguments in the switched project.
>> However, the reverse order of typing a command keys
>> before switching the project looks like trying
>> to run the command in the previous project.
>> Also the problem is that typing a command keys and reading
>> the command arguments is separated by reading a project.
>>
>> Maybe many users would prefer other-project-prefix, I don't know.
>> But other-project-prefix can't replace project-switch-project,
>> only to be an alternative.
> Sorry, I didn't realize that implementation of other-project-prefix
> can be changed to read the project before reading the command
> with arguments:
> 
>    (defun other-project-prefix ()
>      (letrec ((root (funcall project-prompter))
>               (depth (minibuffer-depth))
>               (echofun (lambda () "[switch-project]"))
>               (around-fun
>                ...

Yeah, it can be made tweakable like that, although I'd suggest first 
trying to use it as-is for a little bit.

If you prefer to read the project first, would see a lot of advantage to 
the new command? I suppose 'C-h' working is the only plus.

bug#70577: [PATCH] New command other-project-prefix

[Juri Linkov]

>> can be changed to read the project before reading the command
>> with arguments:
>>    (defun other-project-prefix ()
>>      (letrec ((root (funcall project-prompter))
>>               (depth (minibuffer-depth))
>>               (echofun (lambda () "[switch-project]"))
>>               (around-fun
>>                ...
>
> Yeah, it can be made tweakable like that, although I'd suggest first trying
> to use it as-is for a little bit.

I tried but still can't use it because its sequence of reading
the project is incompatible with old 'project-switch-project',
so it's difficult to switch to the new sequence.

> If you prefer to read the project first, would see a lot of advantage to
> the new command? I suppose 'C-h' working is the only plus.

The advantage is that the new command paves the way for
implementing the support of 'C-x p P C-x 4 p f'.

bug#70577: [PATCH] New command other-project-prefix

[Dmitry Gutov]

On 06/05/2024 20:25, Juri Linkov wrote:
>>> Wow, everything works now, will test more as a primary 'C-x p p' command.
>>
>> Thanks, let me know if you find any other problems.
> 
> I confirm that everything works nicely, thanks.  The only
> problem is that after trying to use it, its order looks unnatural.
> I already accustomed to this order: 1) select the project,
> 2) run the command.  This is handy especially with
> project-switch-commands set to 'project-prefix-or-any-command'.

Could be a problem indeed, but only if we are aiming at changing the 
default. We're probably not going to do that, or at least not right away.

> This order looks more logical because after selecting the
> project, the user mentally switches to another project, and
> then types a command with arguments in the switched project.
> However, the reverse order of typing a command keys
> before switching the project looks like trying
> to run the command in the previous project.
> Also the problem is that typing a command keys and reading
> the command arguments is separated by reading a project.

My advantage (maybe?) is that I don't use the "switch project" command 
very often. But when I do and I think about it, it kinds of seem to 
stick out compared to some other commands, in particular "prefix" ones, 
that you have two separate key sequences which you need to input, 
instead of just one longer one.

That's where my main motivation for this patch comes (the other being 
that 'C-h' works with it).

Speaking of the implementation strategy, though, I think the current 
other-project-prefix implementation still doesn't work well together 
with project-other-*-command.

I.e. I'd expect 'C-x p P C-x 4 p f' to function as "find file in 
different project", but it both interrupts the key sequence before the 
last char (with a prompt), and ultimately fails to switch to that 
different project.

> Maybe many users would prefer other-project-prefix, I don't know.
> But other-project-prefix can't replace project-switch-project,
> only to be an alternative.

I think we could add the new command, and then revisit the question of 
defaults in 1-2 years. I guess the main difficulty is documenting all 
the new alternatives added in Emacs 30 adequately, so the user can make 
a confident choice which one to use.

bug#70577: [PATCH] New command other-project-prefix

[Dmitry Gutov]

On 07/05/2024 22:16, Dmitry Gutov wrote:
> Speaking of the implementation strategy, though, I think the current 
> other-project-prefix implementation still doesn't work well together 
> with project-other-*-command.

I suppose we could just blacklist some known prefix commands (*) in the 
same form where we now compare (eq this-command 'other-project-prefix), 
but it would be nice to distinguish prefix commands from "real" ones 
somehow.

(*) project-other-window-command, project-other-frame-command, 
project-other-tab-command, some others?

bug#70577: [PATCH] New command other-project-prefix

[Juri Linkov]

>> Speaking of the implementation strategy, though, I think the current
>> other-project-prefix implementation still doesn't work well together with
>> project-other-*-command.
>
> I suppose we could just blacklist some known prefix commands (*) in the
> same form where we now compare (eq this-command 'other-project-prefix), but
> it would be nice to distinguish prefix commands from "real" ones somehow.
>
> (*) project-other-window-command, project-other-frame-command,
> project-other-tab-command, some others?

I remember that other-commands worked in one of your previous patches.

bug#70577: [PATCH] New command other-project-prefix

[Dmitry Gutov]

On 09/05/2024 09:20, Juri Linkov wrote:
>>> Speaking of the implementation strategy, though, I think the current
>>> other-project-prefix implementation still doesn't work well together with
>>> project-other-*-command.
>> I suppose we could just blacklist some known prefix commands (*) in the
>> same form where we now compare (eq this-command 'other-project-prefix), but
>> it would be nice to distinguish prefix commands from "real" ones somehow.
>>
>> (*) project-other-window-command, project-other-frame-command,
>> project-other-tab-command, some others?
> I remember that other-commands worked in one of your previous patches.

Maybe only when they went down the

   (if (< emacs-major-version 30)

code path?

Like project-any-command, project--other-place-command uses 
read-key-sequence and then invokes the command, which means that 
whatever dynamic binding was in effect, stays in effect for the "next" 
command.

Conversely, calling project--other-place-prefix ends the current 
command, and the dynamic binding ends there too.

bug#70577: [PATCH] New command other-project-prefix

[Juri Linkov]

>>>> Speaking of the implementation strategy, though, I think the current
>>>> other-project-prefix implementation still doesn't work well together with
>>>> project-other-*-command.
>>> I suppose we could just blacklist some known prefix commands (*) in the
>>> same form where we now compare (eq this-command 'other-project-prefix), but
>>> it would be nice to distinguish prefix commands from "real" ones somehow.
>>>
>>> (*) project-other-window-command, project-other-frame-command,
>>> project-other-tab-command, some others?
>> I remember that other-commands worked in one of your previous patches.
>
> Maybe only when they went down the
>
>   (if (< emacs-major-version 30)
>
> code path?

Nope, only 30.  (But it's possible that I mistyped 'C-x p p' instead of 'C-x p P'.)

> Conversely, calling project--other-place-prefix ends the current command,
> and the dynamic binding ends there too.

set-transient-map has the arg KEEP-PRED that could be used to keep the map
for a sequence of commands, but its use might be tricky.

bug#70577: [PATCH] New command other-project-prefix

[Dmitry Gutov]

On 10/05/2024 09:43, Juri Linkov wrote:

>> Conversely, calling project--other-place-prefix ends the current command,
>> and the dynamic binding ends there too.
> set-transient-map has the arg KEEP-PRED that could be used to keep the map
> for a sequence of commands, but its use might be tricky.

I'm not sure if it can help: what we need to do is not keep the keymap, 
but keep the advice from applying too early (meaning, keep the 
pre-command-hook from firing and removing itself before the wrong command).

bug#70577: [PATCH] New command other-project-prefix

[Dmitry Gutov]

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

On 09/05/2024 05:22, Dmitry Gutov wrote:
> On 07/05/2024 22:16, Dmitry Gutov wrote:
>> Speaking of the implementation strategy, though, I think the current 
>> other-project-prefix implementation still doesn't work well together 
>> with project-other-*-command.
> 
> I suppose we could just blacklist some known prefix commands (*) in the 
> same form where we now compare (eq this-command 'other-project-prefix), 
> but it would be nice to distinguish prefix commands from "real" ones 
> somehow.
> 
> (*) project-other-window-command, project-other-frame-command, 
> project-other-tab-command, some others?

This can look like the attached.

Though I suppose we would use some global registry, or symbol 
properties, or etc.

[-- Attachment #2: other-project-prefix-v5.diff --]
[-- Type: text/x-patch, Size: 3147 bytes --]

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index a95d1267dd2..13e9d2c8a96 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -968,6 +968,58 @@ project-other-tab-command
 (when (bound-and-true-p tab-prefix-map)
   (define-key tab-prefix-map "p" #'project-other-tab-command))
 
+(defvar other-project-prefix-transient-commands '(project-other-window-command
+                                                  project-other-frame-command
+                                                  project-other-tab-command
+                                                  other-window-prefix
+                                                  other-frame-prefix
+                                                  other-tab-prefix)
+  "List of commands that `other-project-prefix' does not apply to.
+
+Meaning, its effect will apply to the first next command that is not in
+this list.  You would typically specify \"prefix\" commands here, ones
+that also apply some modification to the following command's behavior.")
+
+;;;###autoload
+(defun other-project-prefix ()
+  "\"Switch\" to another project before running an Emacs command.
+The next command you invoke will prompt for the project in which to run
+the command."
+  (interactive)
+  (prefix-command-preserve-state)
+  (letrec ((depth (minibuffer-depth))
+           (echofun (lambda () "[switch-project]"))
+           (around-fun
+            (lambda (command &rest _args)
+              (interactive)
+              (advice-remove this-command around-fun)
+              (if (or (eq this-command 'other-project-prefix)
+                      (eq last-command-event help-char))
+                  (call-interactively command)
+                (let* ((this-command-saved this-command)
+                       (root (funcall project-prompter)))
+                  (if (or (string-prefix-p "project-"
+                                           (symbol-name this-command-saved))
+                          (get this-command-saved 'project-aware))
+                      (let ((project-current-directory-override root))
+                        (call-interactively command))
+                    (let ((default-directory root))
+                      (call-interactively command)))))))
+           (prefun
+            (lambda ()
+              (unless (or (> (minibuffer-depth) depth)
+                          (memq this-command other-project-prefix-transient-commands))
+                (remove-hook 'pre-command-hook prefun)
+                (remove-hook 'prefix-command-echo-keystrokes-functions echofun)
+                (when (and this-command (symbolp this-command))
+                  (advice-add this-command :around around-fun))))))
+    (add-hook 'pre-command-hook prefun)
+    (add-hook 'prefix-command-echo-keystrokes-functions echofun)
+    (set-transient-map project-prefix-map)
+    (message (concat "Type " (project--keymap-prompt) " or any global key"))))
+
+;; (define-key project-prefix-map (kbd "P") #'other-project-prefix)
+
 (declare-function grep-read-files "grep")
 (declare-function xref--find-ignores-arguments "xref")
 

bug#70577: [PATCH] New command other-project-prefix

[Juri Linkov]

>>> Speaking of the implementation strategy, though, I think the current
>>> other-project-prefix implementation still doesn't work well together
>>> with project-other-*-command.
>> I suppose we could just blacklist some known prefix commands (*) in the
>> same form where we now compare (eq this-command 'other-project-prefix),
>> but it would be nice to distinguish prefix commands from "real" ones
>> somehow.
>> (*) project-other-window-command, project-other-frame-command,
>> project-other-tab-command, some others?
>
> This can look like the attached.
>
> Though I suppose we would use some global registry, or symbol properties,
> or etc.
>
> +(defvar other-project-prefix-transient-commands '(project-other-window-command
> +                                                  project-other-frame-command
> +                                                  project-other-tab-command
> +                                                  other-window-prefix
> +                                                  other-frame-prefix
> +                                                  other-tab-prefix)
> +  "List of commands that `other-project-prefix' does not apply to.

This doesn't yet support such things as 'C-x 5 p p'?

bug#70577: [PATCH] New command other-project-prefix

[Dmitry Gutov]

On 14/05/2024 09:23, Juri Linkov wrote:
>>>> Speaking of the implementation strategy, though, I think the current
>>>> other-project-prefix implementation still doesn't work well together
>>>> with project-other-*-command.
>>> I suppose we could just blacklist some known prefix commands (*) in the
>>> same form where we now compare (eq this-command 'other-project-prefix),
>>> but it would be nice to distinguish prefix commands from "real" ones
>>> somehow.
>>> (*) project-other-window-command, project-other-frame-command,
>>> project-other-tab-command, some others?
>>
>> This can look like the attached.
>>
>> Though I suppose we would use some global registry, or symbol properties,
>> or etc.
>>
>> +(defvar other-project-prefix-transient-commands '(project-other-window-command
>> +                                                  project-other-frame-command
>> +                                                  project-other-tab-command
>> +                                                  other-window-prefix
>> +                                                  other-frame-prefix
>> +                                                  other-tab-prefix)
>> +  "List of commands that `other-project-prefix' does not apply to.
> 
> This doesn't yet support such things as 'C-x 5 p p'?

I'm not sure that other-project-prefix can do that.

How does other-frame-prefix work? display-buffer-override-next-command 
sets up hooks in the very familiar fashion, so that the next command 
(and only the next command) is affected by a number of changed 
variables, which get restored after.

I suppose other-project-prefix could learn all the new variables it 
needs to "carry on", look up their values, and set them additionally for 
the next command. But that seems very ad-hoc.

It seems the "proper" way to fix that would be a cross-codebase change 
where all similar "prefix" commands themselves check whether the next 
command is a "prefix" command as well, and if so, keep the variables and 
hooks in place for the command after it. This would also mean moving the 
information from other-project-prefix-transient-commands to symbol 
properties (the alternative I've mentioned previously).

bug#70577: [PATCH] New command other-project-prefix

[Juri Linkov]

>>> +(defvar other-project-prefix-transient-commands '(project-other-window-command
>>> +                                                  project-other-frame-command
>>> +                                                  project-other-tab-command
>>> +                                                  other-window-prefix
>>> +                                                  other-frame-prefix
>>> +                                                  other-tab-prefix)
>>> +  "List of commands that `other-project-prefix' does not apply to.
>> This doesn't yet support such things as 'C-x 5 p p'?
>
> I'm not sure that other-project-prefix can do that.
>
> How does other-frame-prefix work? display-buffer-override-next-command sets
> up hooks in the very familiar fashion, so that the next command (and only
> the next command) is affected by a number of changed variables, which get
> restored after.
>
> I suppose other-project-prefix could learn all the new variables it needs
> to "carry on", look up their values, and set them additionally for the next
> command. But that seems very ad-hoc.
>
> It seems the "proper" way to fix that would be a cross-codebase change
> where all similar "prefix" commands themselves check whether the next
> command is a "prefix" command as well, and if so, keep the variables and
> hooks in place for the command after it. This would also mean moving the
> information from other-project-prefix-transient-commands to symbol
> properties (the alternative I've mentioned previously).

In https://debbugs.gnu.org/cgi/bugreport.cgi?bug=63648#95
I made an unfinished attempt to handle this by:

```
diff --git a/lisp/window.el b/lisp/window.el
index ab7dd5ced12..52ba407d9c8 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -9099,7 +9091,8 @@ display-buffer-override-next-command
 		     (> (minibuffer-depth) minibuffer-depth)
 		     ;; But don't remove immediately after
 		     ;; adding the hook by the same command below.
-		     (eq this-command command))
+		     (eq this-command command)
+		     (memq this-command '(other-project-prefix)))
               (funcall exitfun))))
     ;; Call post-function after the next command finishes (bug#49057).
     (add-hook 'post-command-hook postfun)
```

I'm not sure if this is a proper way, this needs more trial-and-error.