all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#70577: [PATCH] New command other-project-prefix
@ 2024-04-26  3:01 Dmitry Gutov
  2024-04-26  6:09 ` Juri Linkov
  0 siblings, 1 reply; 24+ messages in thread
From: Dmitry Gutov @ 2024-04-26  3:01 UTC (permalink / raw)
  To: 70577; +Cc: juri linkov

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.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#70577: [PATCH] New command other-project-prefix
  2024-04-26  3:01 bug#70577: [PATCH] New command other-project-prefix Dmitry Gutov
@ 2024-04-26  6:09 ` Juri Linkov
  2024-04-26 10:59   ` Dmitry Gutov
  0 siblings, 1 reply; 24+ messages in thread
From: Juri Linkov @ 2024-04-26  6:09 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 70577

> 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'.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#70577: [PATCH] New command other-project-prefix
  2024-04-26  6:09 ` Juri Linkov
@ 2024-04-26 10:59   ` Dmitry Gutov
  2024-04-26 16:20     ` Dmitry Gutov
  0 siblings, 1 reply; 24+ messages in thread
From: Dmitry Gutov @ 2024-04-26 10:59 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 70577

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.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#70577: [PATCH] New command other-project-prefix
  2024-04-26 10:59   ` Dmitry Gutov
@ 2024-04-26 16:20     ` Dmitry Gutov
  2024-04-28 12:13       ` Sean Whitton via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-04-28 16:51       ` Juri Linkov
  0 siblings, 2 replies; 24+ messages in thread
From: Dmitry Gutov @ 2024-04-26 16:20 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 70577

[-- 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")
 

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* bug#70577: [PATCH] New command other-project-prefix
  2024-04-26 16:20     ` Dmitry Gutov
@ 2024-04-28 12:13       ` Sean Whitton via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-04-28 15:56         ` Dmitry Gutov
                           ` (2 more replies)
  2024-04-28 16:51       ` Juri Linkov
  1 sibling, 3 replies; 24+ messages in thread
From: Sean Whitton via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-04-28 12:13 UTC (permalink / raw)
  To: Dmitry Gutov, Juri Linkov, emacs-devel; +Cc: 70577

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





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#70577: [PATCH] New command other-project-prefix
  2024-04-28 12:13       ` Sean Whitton via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-04-28 15:56         ` Dmitry Gutov
  2024-04-28 15:56         ` Dmitry Gutov
  2024-04-28 16:46         ` Juri Linkov
  2 siblings, 0 replies; 24+ messages in thread
From: Dmitry Gutov @ 2024-04-28 15:56 UTC (permalink / raw)
  To: Sean Whitton, Juri Linkov, emacs-devel; +Cc: 70577

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.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: bug#70577: [PATCH] New command other-project-prefix
  2024-04-28 12:13       ` Sean Whitton via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-04-28 15:56         ` Dmitry Gutov
@ 2024-04-28 15:56         ` Dmitry Gutov
  2024-04-28 16:46         ` Juri Linkov
  2 siblings, 0 replies; 24+ messages in thread
From: Dmitry Gutov @ 2024-04-28 15:56 UTC (permalink / raw)
  To: Sean Whitton, Juri Linkov, emacs-devel; +Cc: 70577

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.



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: bug#70577: [PATCH] New command other-project-prefix
  2024-04-28 12:13       ` Sean Whitton via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-04-28 15:56         ` Dmitry Gutov
  2024-04-28 15:56         ` Dmitry Gutov
@ 2024-04-28 16:46         ` Juri Linkov
  2 siblings, 0 replies; 24+ messages in thread
From: Juri Linkov @ 2024-04-28 16:46 UTC (permalink / raw)
  To: Sean Whitton; +Cc: Dmitry Gutov, emacs-devel

>> +(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.



^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#70577: [PATCH] New command other-project-prefix
  2024-04-26 16:20     ` Dmitry Gutov
  2024-04-28 12:13       ` Sean Whitton via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-04-28 16:51       ` Juri Linkov
  2024-04-28 21:40         ` Dmitry Gutov
  1 sibling, 1 reply; 24+ messages in thread
From: Juri Linkov @ 2024-04-28 16:51 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 70577

> 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.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#70577: [PATCH] New command other-project-prefix
  2024-04-28 16:51       ` Juri Linkov
@ 2024-04-28 21:40         ` Dmitry Gutov
  2024-05-02  6:12           ` Juri Linkov
  0 siblings, 1 reply; 24+ messages in thread
From: Dmitry Gutov @ 2024-04-28 21:40 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 70577

[-- 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")
 

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* bug#70577: [PATCH] New command other-project-prefix
  2024-04-28 21:40         ` Dmitry Gutov
@ 2024-05-02  6:12           ` Juri Linkov
  2024-05-04  2:12             ` Dmitry Gutov
  0 siblings, 1 reply; 24+ messages in thread
From: Juri Linkov @ 2024-05-02  6:12 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 70577

>> 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))))





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#70577: [PATCH] New command other-project-prefix
  2024-05-02  6:12           ` Juri Linkov
@ 2024-05-04  2:12             ` Dmitry Gutov
  2024-05-04  7:24               ` Eli Zaretskii
  2024-05-05 16:40               ` Juri Linkov
  0 siblings, 2 replies; 24+ messages in thread
From: Dmitry Gutov @ 2024-05-04  2:12 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 70577

[-- 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")
 

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* bug#70577: [PATCH] New command other-project-prefix
  2024-05-04  2:12             ` Dmitry Gutov
@ 2024-05-04  7:24               ` Eli Zaretskii
  2024-05-04 17:22                 ` Dmitry Gutov
  2024-05-05 16:40               ` Juri Linkov
  1 sibling, 1 reply; 24+ messages in thread
From: Eli Zaretskii @ 2024-05-04  7:24 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 70577, juri

> 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.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#70577: [PATCH] New command other-project-prefix
  2024-05-04  7:24               ` Eli Zaretskii
@ 2024-05-04 17:22                 ` Dmitry Gutov
  2024-05-04 17:34                   ` Eli Zaretskii
  0 siblings, 1 reply; 24+ messages in thread
From: Dmitry Gutov @ 2024-05-04 17:22 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 70577, juri

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.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#70577: [PATCH] New command other-project-prefix
  2024-05-04 17:22                 ` Dmitry Gutov
@ 2024-05-04 17:34                   ` Eli Zaretskii
  2024-05-05  0:02                     ` Dmitry Gutov
  0 siblings, 1 reply; 24+ messages in thread
From: Eli Zaretskii @ 2024-05-04 17:34 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 70577, juri

> 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.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#70577: [PATCH] New command other-project-prefix
  2024-05-04 17:34                   ` Eli Zaretskii
@ 2024-05-05  0:02                     ` Dmitry Gutov
  2024-05-05  5:44                       ` Eli Zaretskii
  0 siblings, 1 reply; 24+ messages in thread
From: Dmitry Gutov @ 2024-05-05  0:02 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 70577, juri

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.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#70577: [PATCH] New command other-project-prefix
  2024-05-05  0:02                     ` Dmitry Gutov
@ 2024-05-05  5:44                       ` Eli Zaretskii
  2024-05-05 18:26                         ` Dmitry Gutov
  0 siblings, 1 reply; 24+ messages in thread
From: Eli Zaretskii @ 2024-05-05  5:44 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 70577, juri

> 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.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#70577: [PATCH] New command other-project-prefix
  2024-05-04  2:12             ` Dmitry Gutov
  2024-05-04  7:24               ` Eli Zaretskii
@ 2024-05-05 16:40               ` Juri Linkov
  2024-05-05 18:55                 ` Dmitry Gutov
  1 sibling, 1 reply; 24+ messages in thread
From: Juri Linkov @ 2024-05-05 16:40 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 70577

> 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.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#70577: [PATCH] New command other-project-prefix
  2024-05-05  5:44                       ` Eli Zaretskii
@ 2024-05-05 18:26                         ` Dmitry Gutov
  0 siblings, 0 replies; 24+ messages in thread
From: Dmitry Gutov @ 2024-05-05 18:26 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 70577, juri

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).





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#70577: [PATCH] New command other-project-prefix
  2024-05-05 16:40               ` Juri Linkov
@ 2024-05-05 18:55                 ` Dmitry Gutov
  2024-05-06 17:25                   ` Juri Linkov
  0 siblings, 1 reply; 24+ messages in thread
From: Dmitry Gutov @ 2024-05-05 18:55 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 70577

[-- 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")
 

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* bug#70577: [PATCH] New command other-project-prefix
  2024-05-05 18:55                 ` Dmitry Gutov
@ 2024-05-06 17:25                   ` Juri Linkov
  2024-05-06 18:30                     ` Juri Linkov
  2024-05-07 19:16                     ` Dmitry Gutov
  0 siblings, 2 replies; 24+ messages in thread
From: Juri Linkov @ 2024-05-06 17:25 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 70577

>> 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.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#70577: [PATCH] New command other-project-prefix
  2024-05-06 17:25                   ` Juri Linkov
@ 2024-05-06 18:30                     ` Juri Linkov
  2024-05-07 19:23                       ` Dmitry Gutov
  2024-05-07 19:16                     ` Dmitry Gutov
  1 sibling, 1 reply; 24+ messages in thread
From: Juri Linkov @ 2024-05-06 18:30 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 70577

>>> 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
              ...





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#70577: [PATCH] New command other-project-prefix
  2024-05-06 17:25                   ` Juri Linkov
  2024-05-06 18:30                     ` Juri Linkov
@ 2024-05-07 19:16                     ` Dmitry Gutov
  1 sibling, 0 replies; 24+ messages in thread
From: Dmitry Gutov @ 2024-05-07 19:16 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 70577

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.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* bug#70577: [PATCH] New command other-project-prefix
  2024-05-06 18:30                     ` Juri Linkov
@ 2024-05-07 19:23                       ` Dmitry Gutov
  0 siblings, 0 replies; 24+ messages in thread
From: Dmitry Gutov @ 2024-05-07 19:23 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 70577

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.





^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2024-05-07 19:23 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-26  3:01 bug#70577: [PATCH] New command other-project-prefix Dmitry Gutov
2024-04-26  6:09 ` Juri Linkov
2024-04-26 10:59   ` Dmitry Gutov
2024-04-26 16:20     ` Dmitry Gutov
2024-04-28 12:13       ` Sean Whitton via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-04-28 15:56         ` Dmitry Gutov
2024-04-28 15:56         ` Dmitry Gutov
2024-04-28 16:46         ` Juri Linkov
2024-04-28 16:51       ` Juri Linkov
2024-04-28 21:40         ` Dmitry Gutov
2024-05-02  6:12           ` Juri Linkov
2024-05-04  2:12             ` Dmitry Gutov
2024-05-04  7:24               ` Eli Zaretskii
2024-05-04 17:22                 ` Dmitry Gutov
2024-05-04 17:34                   ` Eli Zaretskii
2024-05-05  0:02                     ` Dmitry Gutov
2024-05-05  5:44                       ` Eli Zaretskii
2024-05-05 18:26                         ` Dmitry Gutov
2024-05-05 16:40               ` Juri Linkov
2024-05-05 18:55                 ` Dmitry Gutov
2024-05-06 17:25                   ` Juri Linkov
2024-05-06 18:30                     ` Juri Linkov
2024-05-07 19:23                       ` Dmitry Gutov
2024-05-07 19:16                     ` Dmitry Gutov

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.