On Wed, 12 May 2021 at 20:13, Juri Linkov wrote: Hi Juri, >>>>> You can avoid the timer hack by adding a guard to >>>>> isearch-post-command-hook: when at the end of the isearch command, >>>>> point is not in the minibuffer, activate the minibuffer >>>>> (assuming that isearch-from-minibuffer is t). >>>> >>>> That didn't work well, because when canceling a command called from the >>>> post-command hook one gets an ugly error message. >>> >>> How do yo cancel such a command? >> >> C-g > > Do you mean the global C-g bound to keyboard-quit, > isearch-mode's C-g bound to isearch-abort, or > minibuffer's C-g bound to minibuffer-keyboard-quit? > And what was the error message? I mean keyboard quit. You can see the error message by evaluating this and then pressing C-g: (add-hook 'post-command-hook (lambda () (read-from-minibuffer "test")) t t) Using a timer with zero delay instead works better. > >> To put it from another perspective: you said earlier that my patch could >> be boiled down to 10 lines. Well, adding lazy highlight/count to >> `isearch-edit-string' is certainly more work than that. But once this >> is in place, then yes, the minibuffer-controlled mode is a small >> addition. > > Adding lazy highlight/count should not be more work. It's simple to do > with a few lines by setting minibuffer-local isearch-message-function, > the same way like minibuffer-history-isearch-setup sets it to > minibuffer-history-isearch-message. > Setting `isearch-message-function' is of no help, because there are some tests for `(null isearch-message-function)' as well as some explicit calls to `(isearch-message)' in isearch.el. As far as I can see, there is no alternative to modifying the function `isearch-message' itself. >>>> 2. A M-s prefix is added to minibuffer-local-isearch-map, as well as a >>>> few extra commands (M->, M-<, etc.) >>> >>> The users might want to use M-< M-> to go to the beginning/end of the >>> minibuffer. >> >> This seems way less useful than going to the first/last match in the >> search buffer, since in the minibuffer C-a and C-e are usually >> sufficient. >> >> By the way, what's the idea behind `minibuffer-beginning-of-buffer'? It >> moves past the prompt, which is a useless point to go. > > It's useful when minibuffer-beginning-of-buffer-movement is customized > to t. I don't know why currently its default is nil. I see. > >> First of all, let me say that you suggestion to get rid of the >> `with-isearch-window' macro works fine. The remaining problem is with >> commands that create a minibuffer, and therefore require that we quit >> the `isearch-edit-string' minibuffer first. One example would be >> `isearch-query-replace'. >> >> So here's the the situation in more detail: >> >> - You are in an `isearch-edit-string' session >> - Then you press M-% >> - Now we are in the pre-command-hook. We check `this-command` and >> see that it will need the minibuffer. >> >> From there, how can we get rid of the minibuffer and continue running >> this-command? Calling `exit-minibuffer' now would return control to >> whoever called `isearch-edit-string', so `this-command' would never run. > > This is an interesting problem. Would it be possible after calling > exit-minibuffer to allow the caller of isearch-edit-string (mosy likely > the caller should be isearch-mode when isearch-from-minibuffer is t) > to call the right function after exiting from the minibuffer, > e.g. when this-command-keys was M-% then call isearch-query-replace > after isearch-edit-string at the end of isearch-mode. I've attached a new patch (again, in rough draft form) where the `with-isearch-window' and `with-isearch-window-quitting-edit' macros from my previous patch are replaced by some pre/post command hook logic, as you suggested. In particular, the case of commands that end the search and create a minibuffer is handled by this part of the minibuffer pre-command hook: (when (member this-command isearch-edit--quitting-commands) (run-with-timer 0 nil 'command-execute this-command) (exit-minibuffer)) While this version of the patch doesn't require wrapping several existing commands in a new macro, I find it much more convoluted. Moreover, one still has to manually keep a list of commands that need to switch to the search buffer (cf. `isearch-edit--buffer-commands') and commands that end the search (cf. `isearch-edit--quitting-commands'); I see no way around that. Therefore, I see no advantage here over the proposed `with-isearch-window' macro. I admit that the `with-isearch-window-quitting-edit' macro of my old patch seems pretty ad-hoc. However, it could be replaced by a `with-isearch-done' macro which is of more general nature. If you search isearch.el for calls to `isearch-done', you'll see that some are of the form (let (;; Set `isearch-recursive-edit' to nil to prevent calling ;; `exit-recursive-edit' in `isearch-done' that terminates ;; the execution of this command when it is non-nil. ;; We call `exit-recursive-edit' explicitly at the end below. (isearch-recursive-edit nil)) (isearch-done nil t) while a few others seem to just don't take into account that we may be ending a recursive edit. Third party packages, even the excellently well written Consult, overlook this detail too: https://github.com/minad/consult/blob/b873ceeefcb80ae0a00aa5e9ce7d70a71680aa4b/consult.el#L2161 So an `with-isearch-done' macro (which ends isearch, executes the body, then possibly ends a recursive edit, and at the same time takes care to unwind the minibuffer if we happen to be in `isearch-edit-string') would seem a reasonable addition to isearch.el. Without such a macro available, I think I would rather rely on advices to implement the minibuffer-based Isearch UI (which, I gather, would force it to be an external package). I'm not quite sure yet. Sorry for the long message :-)