* 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; 41+ 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] 41+ 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; 41+ 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] 41+ 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; 41+ 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] 41+ 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; 41+ 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] 41+ 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; 41+ 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] 41+ 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; 41+ 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] 41+ 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; 41+ 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] 41+ 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; 41+ 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] 41+ 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; 41+ 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] 41+ 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; 41+ 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] 41+ 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; 41+ 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] 41+ 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; 41+ 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] 41+ 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; 41+ 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] 41+ 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; 41+ 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] 41+ 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; 41+ 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] 41+ 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; 41+ 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] 41+ 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; 41+ 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] 41+ 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; 41+ 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] 41+ 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; 41+ 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] 41+ 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; 41+ 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] 41+ 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; 41+ 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] 41+ 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; 41+ 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] 41+ 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 2024-05-09 6:24 ` Juri Linkov 0 siblings, 1 reply; 41+ 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] 41+ messages in thread
* bug#70577: [PATCH] New command other-project-prefix 2024-05-07 19:23 ` Dmitry Gutov @ 2024-05-09 6:24 ` Juri Linkov 0 siblings, 0 replies; 41+ messages in thread From: Juri Linkov @ 2024-05-09 6:24 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 70577 >> 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'. ^ permalink raw reply [flat|nested] 41+ 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 2024-05-09 2:22 ` Dmitry Gutov 1 sibling, 1 reply; 41+ 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] 41+ messages in thread
* bug#70577: [PATCH] New command other-project-prefix 2024-05-07 19:16 ` Dmitry Gutov @ 2024-05-09 2:22 ` Dmitry Gutov 2024-05-09 6:20 ` Juri Linkov 2024-05-12 18:33 ` Dmitry Gutov 0 siblings, 2 replies; 41+ messages in thread From: Dmitry Gutov @ 2024-05-09 2:22 UTC (permalink / raw) To: Juri Linkov; +Cc: 70577 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? ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#70577: [PATCH] New command other-project-prefix 2024-05-09 2:22 ` Dmitry Gutov @ 2024-05-09 6:20 ` Juri Linkov 2024-05-10 1:46 ` Dmitry Gutov 2024-05-12 18:33 ` Dmitry Gutov 1 sibling, 1 reply; 41+ messages in thread From: Juri Linkov @ 2024-05-09 6:20 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 70577 >> 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. ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#70577: [PATCH] New command other-project-prefix 2024-05-09 6:20 ` Juri Linkov @ 2024-05-10 1:46 ` Dmitry Gutov 2024-05-10 6:43 ` Juri Linkov 0 siblings, 1 reply; 41+ messages in thread From: Dmitry Gutov @ 2024-05-10 1:46 UTC (permalink / raw) To: Juri Linkov; +Cc: 70577 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. ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#70577: [PATCH] New command other-project-prefix 2024-05-10 1:46 ` Dmitry Gutov @ 2024-05-10 6:43 ` Juri Linkov 2024-05-10 15:09 ` Dmitry Gutov 0 siblings, 1 reply; 41+ messages in thread From: Juri Linkov @ 2024-05-10 6:43 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 70577 >>>> 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. ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#70577: [PATCH] New command other-project-prefix 2024-05-10 6:43 ` Juri Linkov @ 2024-05-10 15:09 ` Dmitry Gutov 0 siblings, 0 replies; 41+ messages in thread From: Dmitry Gutov @ 2024-05-10 15:09 UTC (permalink / raw) To: Juri Linkov; +Cc: 70577 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). ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#70577: [PATCH] New command other-project-prefix 2024-05-09 2:22 ` Dmitry Gutov 2024-05-09 6:20 ` Juri Linkov @ 2024-05-12 18:33 ` Dmitry Gutov 2024-05-14 6:23 ` Juri Linkov 1 sibling, 1 reply; 41+ messages in thread From: Dmitry Gutov @ 2024-05-12 18:33 UTC (permalink / raw) To: Juri Linkov; +Cc: 70577 [-- 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") ^ permalink raw reply related [flat|nested] 41+ messages in thread
* bug#70577: [PATCH] New command other-project-prefix 2024-05-12 18:33 ` Dmitry Gutov @ 2024-05-14 6:23 ` Juri Linkov 2024-05-14 20:02 ` Dmitry Gutov 0 siblings, 1 reply; 41+ messages in thread From: Juri Linkov @ 2024-05-14 6:23 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 70577 >>> 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'? ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#70577: [PATCH] New command other-project-prefix 2024-05-14 6:23 ` Juri Linkov @ 2024-05-14 20:02 ` Dmitry Gutov 2024-05-15 6:46 ` Juri Linkov 0 siblings, 1 reply; 41+ messages in thread From: Dmitry Gutov @ 2024-05-14 20:02 UTC (permalink / raw) To: Juri Linkov; +Cc: 70577 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). ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#70577: [PATCH] New command other-project-prefix 2024-05-14 20:02 ` Dmitry Gutov @ 2024-05-15 6:46 ` Juri Linkov 2024-05-21 2:31 ` Dmitry Gutov 0 siblings, 1 reply; 41+ messages in thread From: Juri Linkov @ 2024-05-15 6:46 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 70577 >>> +(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. ^ permalink raw reply related [flat|nested] 41+ messages in thread
* bug#70577: [PATCH] New command other-project-prefix 2024-05-15 6:46 ` Juri Linkov @ 2024-05-21 2:31 ` Dmitry Gutov 2024-05-21 6:08 ` Juri Linkov 0 siblings, 1 reply; 41+ messages in thread From: Dmitry Gutov @ 2024-05-21 2:31 UTC (permalink / raw) To: Juri Linkov; +Cc: 70577 [-- Attachment #1: Type: text/plain, Size: 2903 bytes --] On 15/05/2024 09:46, Juri Linkov wrote: >>>> +(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. Looks like you were thinking along similar lines. Here's a patch using symbol properties that makes the prefix commands work combined in arbitrary order. At least according to my limited testing - and only the commands that have this property set, of course. Something to discuss: - A better name for the property? Maybe something longer would be more obvious for an accidental reader. - Applying it through 'declare' forms could be a more self-contained approach. [-- Attachment #2: other-project-prefix-v6.diff --] [-- Type: text/x-patch, Size: 5013 bytes --] diff --git a/lisp/frame.el b/lisp/frame.el index d2376f1e339..1e18d41ac31 100644 --- a/lisp/frame.el +++ b/lisp/frame.el @@ -1108,6 +1108,8 @@ other-frame-prefix nil "[other-frame]") (message "Display next command buffer in a new frame...")) +(put 'other-frame-prefix 'prefix-command t) + (defun iconify-or-deiconify-frame () "Iconify the selected frame, or deiconify if it's currently an icon." (interactive) diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el index a95d1267dd2..7298ffe19c8 100644 --- a/lisp/progmodes/project.el +++ b/lisp/progmodes/project.el @@ -934,6 +934,8 @@ project-other-window-command project-other-window-map) (project--other-place-prefix 'window project-other-window-map))) +(put 'project-other-window-command 'prefix-command t) + ;;;###autoload (define-key ctl-x-4-map "p" #'project-other-window-command) ;;;###autoload @@ -950,6 +952,8 @@ project-other-frame-command project-other-frame-map) (project--other-place-prefix 'frame project-other-frame-map))) +(put 'project-other-frame-command 'prefix-command t) + ;;;###autoload (define-key ctl-x-5-map "p" #'project-other-frame-command) ;;;###autoload @@ -964,10 +968,54 @@ project-other-tab-command (project--other-place-command '((display-buffer-in-new-tab))) (project--other-place-prefix 'tab))) +(put 'project-other-tab-command 'prefix-command t) + ;;;###autoload (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 (or (> (minibuffer-depth) depth) + (get this-command 'prefix-command)) + (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")))) + +(put 'other-project-prefix 'prefix-command t) + +;; (define-key project-prefix-map (kbd "P") #'other-project-prefix) + (declare-function grep-read-files "grep") (declare-function xref--find-ignores-arguments "xref") diff --git a/lisp/tab-bar.el b/lisp/tab-bar.el index dac57ce2070..9a16d20be73 100644 --- a/lisp/tab-bar.el +++ b/lisp/tab-bar.el @@ -2855,6 +2855,8 @@ other-tab-prefix nil "[other-tab]") (message "Display next command buffer in a new tab...")) +(put 'other-tab-prefix 'prefix-command t) + \f ;;; Short aliases and keybindings diff --git a/lisp/window.el b/lisp/window.el index 8feeba0d83e..7d2c7f9977d 100644 --- a/lisp/window.el +++ b/lisp/window.el @@ -4044,6 +4044,8 @@ other-window-prefix nil "[other-window]") (message "Display next command buffer in a new window...")) +(put 'other-window-prefix 'prefix-command t) + (defun same-window-prefix () "Display the buffer of the next command in the same window. The next buffer is the buffer displayed by the next command invoked @@ -9269,7 +9271,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) + (get this-command 'prefix-command)) (funcall exitfun)))) ;; Call post-function after the next command finishes (bug#49057). (add-hook 'post-command-hook postfun) ^ permalink raw reply related [flat|nested] 41+ messages in thread
* bug#70577: [PATCH] New command other-project-prefix 2024-05-21 2:31 ` Dmitry Gutov @ 2024-05-21 6:08 ` Juri Linkov 2024-05-21 20:16 ` Dmitry Gutov 0 siblings, 1 reply; 41+ messages in thread From: Juri Linkov @ 2024-05-21 6:08 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 70577 > Here's a patch using symbol properties that makes the prefix commands work > combined in arbitrary order. At least according to my limited testing - and > only the commands that have this property set, of course. This works until I moved project-prompter to the beginning, then 'C-x 5 p p' doesn't work with this: (letrec ((root (funcall project-prompter)) (depth (minibuffer-depth)) (echofun (lambda () "[switch-project]")) (around-fun Maybe there is a better place? > Something to discuss: > - A better name for the property? Maybe something longer would be more > obvious for an accidental reader. If it makes sense to use it outside of project.el then 'prefix-command' is fine. > - Applying it through 'declare' forms could be a more self-contained > approach. This could be added later as well. ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#70577: [PATCH] New command other-project-prefix 2024-05-21 6:08 ` Juri Linkov @ 2024-05-21 20:16 ` Dmitry Gutov 2024-05-22 6:12 ` Juri Linkov 0 siblings, 1 reply; 41+ messages in thread From: Dmitry Gutov @ 2024-05-21 20:16 UTC (permalink / raw) To: Juri Linkov; +Cc: 70577 On 21/05/2024 09:08, Juri Linkov wrote: >> Here's a patch using symbol properties that makes the prefix commands work >> combined in arbitrary order. At least according to my limited testing - and >> only the commands that have this property set, of course. > > This works until I moved project-prompter to the beginning, > then 'C-x 5 p p' doesn't work with this: > > (letrec ((root (funcall project-prompter)) > (depth (minibuffer-depth)) > (echofun (lambda () "[switch-project]")) > (around-fun > > Maybe there is a better place? Last I checked, the project-prompter can change the value of this-command. Perhaps you can try a let-binding for this-command around the call to project-prompter, so that it's restored at the end. Something to also be concerned about is having any of the display-buffer modifications, or other-project advices, get applied to one of the commands inside the project-prompter UI (if it's implemented using a sequence of commands). Perhaps I haven't triggered this case mostly by luck so far. How to guard against that? Maybe a dynamic variable of some sort. But then it'd also have to be checked uniformly in all such functions (hooks that prefix commands install). >> Something to discuss: >> - A better name for the property? Maybe something longer would be more >> obvious for an accidental reader. > > If it makes sense to use it outside of project.el > then 'prefix-command' is fine. > >> - Applying it through 'declare' forms could be a more self-contained >> approach. > > This could be added later as well. ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#70577: [PATCH] New command other-project-prefix 2024-05-21 20:16 ` Dmitry Gutov @ 2024-05-22 6:12 ` Juri Linkov 2024-05-23 6:24 ` Juri Linkov 0 siblings, 1 reply; 41+ messages in thread From: Juri Linkov @ 2024-05-22 6:12 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 70577 >> This works until I moved project-prompter to the beginning, >> then 'C-x 5 p p' doesn't work with this: >> (letrec ((root (funcall project-prompter)) >> (depth (minibuffer-depth)) >> (echofun (lambda () "[switch-project]")) >> (around-fun >> Maybe there is a better place? > > Last I checked, the project-prompter can change the value of this-command. > > Perhaps you can try a let-binding for this-command around the call to > project-prompter, so that it's restored at the end. This helps: (letrec ((root (let ((this-command this-command)) (funcall project-prompter))) > Something to also be concerned about is having any of the display-buffer > modifications, or other-project advices, get applied to one of the commands > inside the project-prompter UI (if it's implemented using a sequence of > commands). Perhaps I haven't triggered this case mostly by luck so far. How > to guard against that? Maybe a dynamic variable of some sort. But then it'd > also have to be checked uniformly in all such functions (hooks that prefix > commands install). This also helps: diff --git a/lisp/window.el b/lisp/window.el index 4147d7e6ebb..a4577d509b8 100644 --- a/lisp/window.el +++ b/lisp/window.el @@ -9252,7 +9279,7 @@ display-buffer-override-next-command ;; after the first display-buffer action (bug#39722). (funcall clearfun) new-window)))) - (command this-command) + (command this-original-command) (echofun (when echo (lambda () echo))) (exitfun (lambda () @@ -9274,7 +9301,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-original-command command) + (get command 'prefix-command)) (funcall exitfun)))) ;; Call post-function after the next command finishes (bug#49057). (add-hook 'post-command-hook postfun) But not sure about the latter, it might break some cases. ^ permalink raw reply related [flat|nested] 41+ messages in thread
* bug#70577: [PATCH] New command other-project-prefix 2024-05-22 6:12 ` Juri Linkov @ 2024-05-23 6:24 ` Juri Linkov 2024-05-26 2:38 ` Dmitry Gutov 0 siblings, 1 reply; 41+ messages in thread From: Juri Linkov @ 2024-05-23 6:24 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 70577 > - (eq this-command command)) > + (eq this-original-command command) Actually the real problem is that in project--other-place-prefix prefix-command-preserve-state changes this-command to last-command. ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#70577: [PATCH] New command other-project-prefix 2024-05-23 6:24 ` Juri Linkov @ 2024-05-26 2:38 ` Dmitry Gutov 2024-05-26 6:52 ` Juri Linkov 0 siblings, 1 reply; 41+ messages in thread From: Dmitry Gutov @ 2024-05-26 2:38 UTC (permalink / raw) To: Juri Linkov; +Cc: 70577 On 23/05/2024 09:24, Juri Linkov wrote: >> - (eq this-command command)) >> + (eq this-original-command command) > Actually the real problem is that in project--other-place-prefix > prefix-command-preserve-state changes this-command to last-command. This one might not be so bad (the idea, as documented, seems sensible). Changing this-read-command seems more suspect. But I guess it really means more checks would need to be done on this-original-command instead. :-/ The (eq this-original-command command) check could probably be dropped, but otherwise your addition looks good (I don't know any cases where this-original-command would be wrong, though apparently there might be some -- remappings of the prefix commands? seems an odd thing to do). It might also be possible to rewrite display-buffer-override-next-command in a way that the installation of the "advice" (not actual advice in its case) happens in pre-command-hook - then at that point the current command hasn't had a chance to alter this-command. prefun would check whether it needs to be applied, if yet, add the cleanup function to post-command-hook, and run the setup. The modification of display-buffer-overriding-action might also be better done there, so it doesn't alter any prompter UI in the next prefix command that might be invoked. Not an urgent change, just something to consider. Have you had a chance to run with the modified patch a little? Any edge new edge cases crop up? ^ permalink raw reply [flat|nested] 41+ messages in thread
* bug#70577: [PATCH] New command other-project-prefix 2024-05-26 2:38 ` Dmitry Gutov @ 2024-05-26 6:52 ` Juri Linkov 0 siblings, 0 replies; 41+ messages in thread From: Juri Linkov @ 2024-05-26 6:52 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 70577 >>> - (eq this-command command)) >>> + (eq this-original-command command) >> Actually the real problem is that in project--other-place-prefix >> prefix-command-preserve-state changes this-command to last-command. > > This one might not be so bad (the idea, as documented, seems sensible). > > Changing this-read-command seems more suspect. > > But I guess it really means more checks would need to be done on > this-original-command instead. :-/ > > The (eq this-original-command command) check could probably be dropped, but > otherwise your addition looks good (I don't know any cases where > this-original-command would be wrong, though apparently there might be some > -- remappings of the prefix commands? seems an odd thing to do). > > It might also be possible to rewrite display-buffer-override-next-command > in a way that the installation of the "advice" (not actual advice in its > case) happens in pre-command-hook - then at that point the current command > hasn't had a chance to alter this-command. > > prefun would check whether it needs to be applied, if yet, add the cleanup > function to post-command-hook, and run the setup. The modification of > display-buffer-overriding-action might also be better done there, so it > doesn't alter any prompter UI in the next prefix command that might be > invoked. > > Not an urgent change, just something to consider. > > Have you had a chance to run with the modified patch a little? Any edge new > edge cases crop up? I tried to use the patch for a while, with and without this-original-command, but it often leaves the postfun hook in display-buffer-override-next-command active infinitely, thus needed to restart Emacs too often, so I just removed the patch without debugging what part causes this. ^ permalink raw reply [flat|nested] 41+ messages in thread
end of thread, other threads:[~2024-05-26 6:52 UTC | newest] Thread overview: 41+ 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-09 6:24 ` Juri Linkov 2024-05-07 19:16 ` Dmitry Gutov 2024-05-09 2:22 ` Dmitry Gutov 2024-05-09 6:20 ` Juri Linkov 2024-05-10 1:46 ` Dmitry Gutov 2024-05-10 6:43 ` Juri Linkov 2024-05-10 15:09 ` Dmitry Gutov 2024-05-12 18:33 ` Dmitry Gutov 2024-05-14 6:23 ` Juri Linkov 2024-05-14 20:02 ` Dmitry Gutov 2024-05-15 6:46 ` Juri Linkov 2024-05-21 2:31 ` Dmitry Gutov 2024-05-21 6:08 ` Juri Linkov 2024-05-21 20:16 ` Dmitry Gutov 2024-05-22 6:12 ` Juri Linkov 2024-05-23 6:24 ` Juri Linkov 2024-05-26 2:38 ` Dmitry Gutov 2024-05-26 6:52 ` Juri Linkov
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.