Subject: Re: bug#32990: 26.1.50; isearch-forward + t-m-m/mark-active doc

From: [email-masked] (Charles A. Roelli)
Date: Wed, 21 Nov 2018 21:14:30 +0100

> Date: Tue, 20 Nov 2018 20:49:39 +0100
> From: [email-masked] (Charles A. Roelli)
>
> > >From tmm I tried "C==>Cancel last input item DEL", but it does nothing.
> > Do you see the same?
> 
> Yes.  This may be `with-isearch-suspended's doing (as used in
> `isearch-tmm-menubar'): it pushes a new state of Isearch after running
> its arg BODY, so we will have to compensate for that somehow.

Should be fixed in the following change based on the latest master.
In with-isearch-suspended, the state of Isearch is now only pushed if
the new state is not `equal' to the state of Isearch we had before
running the body (which should be in (car isearch-cmds), I think).

----
* lisp/tmm.el (tmm-menubar-keymap): New function factored out from
'tmm-menubar'.
(tmm-menubar): Use 'tmm-menubar-keymap'.
(tmm-prompt): New optional argument 'no-execute'.

* lisp/isearch.el: Declare the new, non-autoloaded function
'tmm-menubar-keymap'.
(isearch-tmm-menubar): New function.
(isearch-menu-bar-yank-map, isearch-menu-bar-map): New variables.
(isearch-mode-map): Define a menu-bar search menu and remap
'tmm-menubar' bindings to point to 'isearch-tmm-menubar'.
(isearch-tool-bar-old-map): New variable.
(isearch-tool-bar-image): New function.
(isearch-tool-bar-map): New variable.
(minor-mode-map-alist): Add an entry for Isearch so that
'isearch-menu-bar-map' shows during search.
(isearch-mode, isearch-done): Save and restore possible
buffer-local 'tool-bar-map' using 'isearch-tool-bar-old-map'.
(isearch-mouse-leave-buffer): Allow 'mouse-minor-mode-menu'.
(with-isearch-suspended): Only push state of Isearch if it has changed
since running the body of this macro.
(isearch-pre-command-hook): Additionally allow bindings in
'isearch-tool-bar-map' to pass through, as well as commands
'isearch-tmm-menubar', 'menu-bar-open' and
'mouse-minor-mode-menu'.
(isearch-post-command-hook): Call 'force-mode-line-update' at its
end to make sure the menu- and tool-bars are up-to-date.

----
diff --git a/lisp/isearch.el b/lisp/isearch.el
index 6d94ef6..eb3cca7 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -54,6 +54,7 @@
 ;;; Code:
 
 (eval-when-compile (require 'cl-lib))
+(declare-function tmm-menubar-keymap "tmm.el")
 
 ;; Some additional options and constants.
 
@@ -489,6 +490,160 @@ 'isearch-mode-help
 
 ;; Define isearch-mode keymap.
 
+(defun isearch-tmm-menubar ()
+  "Run `tmm-menubar' while `isearch-mode' is enabled."
+  (interactive)
+  (require 'tmm)
+  (run-hooks 'menu-bar-update-hook)
+  (let ((command nil))
+    (let ((menu-bar (tmm-menubar-keymap)))
+      (with-isearch-suspended
+       (setq command (let ((isearch-mode t)) ; Show bindings from
+                                             ; `isearch-mode-map' in
+                                             ; tmm's prompt.
+                       (tmm-prompt menu-bar nil nil t)))))
+    (call-interactively command)))
+
+(defvar isearch-menu-bar-yank-map
+  (let ((map (make-sparse-keymap)))
+    (define-key map [isearch-yank-pop]
+      '(menu-item "Previous kill" isearch-yank-pop
+                  :help "Replace previous yanked kill on search string"))
+    (define-key map [isearch-yank-kill]
+      '(menu-item "Current kill" isearch-yank-kill
+                  :help "Append current kill to search string"))
+    (define-key map [isearch-yank-line]
+      '(menu-item "Rest of line" isearch-yank-line
+                  :help "Yank the rest of the current line on search string"))
+    (define-key map [isearch-yank-symbol-or-char]
+      '(menu-item "Symbol/char"
+                  isearch-yank-symbol-or-char
+                  :help "Yank next symbol or char on search string"))
+    (define-key map [isearch-yank-word-or-char]
+      '(menu-item "Word/char"
+                  isearch-yank-word-or-char
+                  :help "Yank next word or char on search string"))
+    (define-key map [isearch-yank-char]
+      '(menu-item "Char" isearch-yank-char
+                  :help "Yank char at point on search string"))
+    map))
+
+(defvar isearch-menu-bar-map
+  (let ((map (make-sparse-keymap "Isearch")))
+    (define-key map [isearch-complete]
+      '(menu-item "Complete current search string" isearch-complete
+                  :help "Complete current search string over search history"))
+    (define-key map [isearch-complete-separator]
+      '(menu-item "--"))
+    (define-key map [isearch-query-replace-regexp]
+      '(menu-item "Replace search string as regexp" isearch-query-replace-regexp
+                  :help "Replace matches for current search string as regexp"))
+    (define-key map [isearch-query-replace]
+      '(menu-item "Replace search string" isearch-query-replace
+                  :help "Replace matches for current search string"))
+    (define-key map [isearch-occur]
+      '(menu-item "Show all matches for search string" isearch-occur
+                  :help "Show all matches for current search string"))
+    (define-key map [isearch-highlight-regexp]
+      '(menu-item "Highlight all matches for search string"
+                  isearch-highlight-regexp
+                  :help "Highlight all matches for current search string"))
+    (define-key map [isearch-search-replace-separator]
+      '(menu-item "--"))
+    (define-key map [isearch-toggle-specified-input-method]
+      '(menu-item "Turn on specific input method"
+                  isearch-toggle-specified-input-method
+                  :help "Turn on specific input method for search"))
+    (define-key map [isearch-toggle-input-method]
+      '(menu-item "Toggle input method" isearch-toggle-input-method
+                  :help "Toggle input method for search"))
+    (define-key map [isearch-input-method-separator]
+      '(menu-item "--"))
+    (define-key map [isearch-char-by-name]
+      '(menu-item "Search for char by name" isearch-char-by-name
+                  :help "Search for character by name"))
+    (define-key map [isearch-quote-char]
+      '(menu-item "Search for literal char" isearch-quote-char
+                  :help "Search for literal char"))
+    (define-key map [isearch-special-char-separator]
+      '(menu-item "--"))
+    (define-key map [isearch-toggle-word]
+      '(menu-item "Word matching" isearch-toggle-word
+                  :help "Word matching"
+                  :button (:toggle
+                           . (eq isearch-regexp-function 'word-search-regexp))))
+    (define-key map [isearch-toggle-symbol]
+      '(menu-item "Symbol matching" isearch-toggle-symbol
+                  :help "Symbol matching"
+                  :button (:toggle
+                           . (eq isearch-regexp-function
+                                 'isearch-symbol-regexp))))
+    (define-key map [isearch-toggle-regexp]
+      '(menu-item "Regexp matching" isearch-toggle-regexp
+                  :help "Regexp matching"
+                  :button (:toggle . isearch-regexp)))
+    (define-key map [isearch-toggle-invisible]
+      '(menu-item "Invisible text matching" isearch-toggle-invisible
+                  :help "Invisible text matching"
+                  :button (:toggle . isearch-invisible)))
+    (define-key map [isearch-toggle-char-fold]
+      '(menu-item "Character folding matching" isearch-toggle-char-fold
+                  :help "Character folding matching"
+                  :button (:toggle
+                           . (eq isearch-regexp-function
+                                 'char-fold-to-regexp))))
+    (define-key map [isearch-toggle-case-fold]
+      '(menu-item "Case folding matching" isearch-toggle-case-fold
+                  :help "Case folding matching"
+                  :button (:toggle . isearch-case-fold-search)))
+    (define-key map [isearch-toggle-lax-whitespace]
+      '(menu-item "Lax whitespace matching" isearch-toggle-lax-whitespace
+                  :help "Lax whitespace matching"
+                  :button (:toggle . isearch-lax-whitespace)))
+    (define-key map [isearch-toggle-separator]
+      '(menu-item "--"))
+    (define-key map [isearch-yank-menu]
+      `(menu-item "Yank on search string" ,isearch-menu-bar-yank-map))
+    (define-key map [isearch-edit-string]
+      '(menu-item "Edit current search string" isearch-edit-string
+                  :help "Edit current search string"))
+    (define-key map [isearch-ring-retreat]
+      '(menu-item "Edit previous search string" isearch-ring-retreat
+                  :help "Edit previous search string in Isearch history"))
+    (define-key map [isearch-ring-advance]
+      '(menu-item "Edit next search string" isearch-ring-advance
+                  :help "Edit next search string in Isearch history"))
+    (define-key map [isearch-del-char]
+      '(menu-item "Delete last char from search string" isearch-del-char
+                  :help "Delete last character from search string"))
+    (define-key map [isearch-delete-char]
+      '(menu-item "Cancel last input item" isearch-delete-char
+                  :help "Cancel the last Isearch command"))
+    (define-key map [isearch-repeat-backward]
+      '(menu-item "Repeat search backward" isearch-repeat-backward
+                  :help "Repeat current search backward"))
+    (define-key map [isearch-repeat-forward]
+      '(menu-item "Repeat search forward" isearch-repeat-forward
+                  :help "Repeat current search forward"))
+    (define-key map [isearch-nonincremental]
+      '(menu-item "Nonincremental search" isearch-exit
+                  :help "Start nonincremental search"
+                  :visible (string-equal isearch-string "")))
+    (define-key map [isearch-exit]
+      '(menu-item "Finish search" isearch-exit
+                  :help "Finish search leaving point where it is"
+                  :visible (not (string-equal isearch-string ""))))
+    (define-key map [isearch-abort]
+      '(menu-item "Remove characters not found" isearch-abort
+                  :help "Quit current search"
+                  :visible (not isearch-success)))
+    (define-key map [isearch-cancel]
+      `(menu-item "Cancel search" isearch-cancel
+                  :help "Cancel current search and return to starting point"
+                  :filter ,(lambda (binding)
+                             (if isearch-success 'isearch-abort binding))))
+    map))
+
 (defvar isearch-mode-map
   (let ((i 0)
 	(map (make-keymap)))
@@ -595,9 +750,59 @@ isearch-mode-map
     ;; characters to the search string.  See iso-transl.el.
     (define-key map "\C-x8\r" 'isearch-char-by-name)
 
+    (define-key map [menu-bar search-menu]
+      (list 'menu-item "Isearch" isearch-menu-bar-map))
+    (define-key map [remap tmm-menubar] 'isearch-tmm-menubar)
+
     map)
   "Keymap for `isearch-mode'.")
 
+(defvar isearch-tool-bar-old-map nil
+  "Variable holding the old local value of `tool-bar-map', if any.")
+
+(defun isearch-tool-bar-image (image-name)
+  "Return an image specification for IMAGE-NAME."
+  (eval (tool-bar--image-expression image-name)))
+
+(defvar isearch-tool-bar-map
+  (let ((map (make-sparse-keymap)))
+    (define-key map [isearch-describe-mode]
+      (list 'menu-item "Help" 'isearch-describe-mode
+            :help "Get help for Isearch"
+            :image '(isearch-tool-bar-image "help")))
+    (define-key map [isearch-occur]
+      (list 'menu-item "Show hits" 'isearch-occur
+            :help "Show each search hit"
+            :image '(isearch-tool-bar-image "index")))
+    (define-key map [isearch-query-replace]
+      (list 'menu-item "Replace" 'isearch-query-replace
+            :help "Replace search string"
+            :image '(isearch-tool-bar-image "search-replace")))
+    (define-key map [isearch-delete-char]
+      (list 'menu-item "Undo" 'isearch-delete-char
+            :help "Undo last input item"
+            :image '(isearch-tool-bar-image "undo")))
+    (define-key map [isearch-exit]
+      (list 'menu-item "Finish" 'isearch-exit
+            :help "Finish search leaving point where it is"
+            :image '(isearch-tool-bar-image "exit")
+            :visible '(not (string-equal isearch-string ""))))
+    (define-key map [isearch-cancel]
+      (list 'menu-item "Abort" 'isearch-cancel
+            :help "Abort search"
+            :image '(isearch-tool-bar-image "close")
+            :filter (lambda (binding)
+                      (if isearch-success 'isearch-abort binding))))
+    (define-key map [isearch-repeat-forward]
+      (list 'menu-item "Repeat forward" 'isearch-repeat-forward
+            :help "Repeat search forward"
+            :image '(isearch-tool-bar-image "right-arrow")))
+    (define-key map [isearch-repeat-backward]
+      (list 'menu-item "Repeat backward" 'isearch-repeat-backward
+            :help "Repeat search backward"
+            :image '(isearch-tool-bar-image "left-arrow")))
+    map))
+
 (defvar minibuffer-local-isearch-map
   (let ((map (make-sparse-keymap)))
     (set-keymap-parent map minibuffer-local-map)
@@ -728,11 +933,19 @@ isearch--saved-overriding-local-map
 
 ;; Minor-mode-alist changes - kind of redundant with the
 ;; echo area, but if isearching in multiple windows, it can be useful.
+;; Also, clicking the mode-line indicator pops up
+;; `isearch-menu-bar-map'.
 
 (or (assq 'isearch-mode minor-mode-alist)
     (nconc minor-mode-alist
 	   (list '(isearch-mode isearch-mode))))
 
+;; We add an entry for `isearch-mode' to `minor-mode-map-alist' so
+;; that `isearch-menu-bar-map' can show on the menu bar.
+(or (assq 'isearch-mode minor-mode-map-alist)
+    (nconc minor-mode-map-alist
+           (list (cons 'isearch-mode isearch-mode-map))))
+
 (defvar-local isearch-mode nil) ;; Name of the minor mode, if non-nil.
 
 (define-key global-map "\C-s" 'isearch-forward)
@@ -981,6 +1194,10 @@ isearch-mode
 	isearch-original-minibuffer-message-timeout minibuffer-message-timeout
 	minibuffer-message-timeout nil)
 
+  (if (local-variable-p 'tool-bar-map)
+      (setq isearch-tool-bar-old-map tool-bar-map))
+  (setq-local tool-bar-map isearch-tool-bar-map)
+
   ;; We must bypass input method while reading key.  When a user type
   ;; printable character, appropriate input method is turned on in
   ;; minibuffer to read multibyte characters.
@@ -1147,6 +1364,12 @@ isearch-done
       (setq input-method-function isearch-input-method-function)
     (kill-local-variable 'input-method-function))
 
+  (if isearch-tool-bar-old-map
+      (progn
+        (setq-local tool-bar-map isearch-tool-bar-old-map)
+        (setq isearch-tool-bar-old-map nil))
+    (kill-local-variable 'tool-bar-map))
+
   (force-mode-line-update)
 
   ;; If we ended in the middle of some intangible text,
@@ -1181,7 +1404,8 @@ isearch-done
 
 (defun isearch-mouse-leave-buffer ()
   "Exit Isearch unless the mouse command is allowed in Isearch."
-  (unless (eq (get this-command 'isearch-scroll) t)
+  (unless (or (eq this-command 'mouse-minor-mode-menu)
+              (eq (get this-command 'isearch-scroll) t))
     (isearch-done)))
 
 (defun isearch-update-ring (string &optional regexp)
@@ -1449,7 +1673,9 @@ with-isearch-suspended
 
 	;; Reinvoke the pending search.
 	(isearch-search)
-	(isearch-push-state)		; this pushes the correct state
+        (unless (and isearch-cmds
+                     (equal (car isearch-cmds) (isearch--get-state)))
+          (isearch-push-state))        ; this pushes the correct state
 	(isearch-update)
 	(if isearch-nonincremental
 	    (progn
@@ -2535,7 +2761,14 @@ isearch-pre-command-hook
      ;; `set-transient-map' thingy like `universal-argument--mode'.
      ((not (eq overriding-terminal-local-map isearch--saved-overriding-local-map)))
      ;; Don't exit Isearch for isearch key bindings.
-     ((commandp (lookup-key isearch-mode-map key nil)))
+     ((or (commandp (lookup-key isearch-mode-map key nil))
+          (commandp
+           (lookup-key
+            `(keymap (tool-bar menu-item nil ,isearch-tool-bar-map)) key))))
+     ;; Allow key bindings that open a menubar.
+     ((memq this-command '(isearch-tmm-menubar
+                           menu-bar-open
+                           mouse-minor-mode-menu)))
      ;; Optionally edit the search string instead of exiting.
      ((eq search-exit-option 'edit)
       (setq this-command 'isearch-edit-string))
@@ -2599,7 +2832,8 @@ isearch-post-command-hook
         (when isearch-forward
           (goto-char isearch-pre-move-point))
         (isearch-search-and-update)))
-    (setq isearch-pre-move-point nil))))
+    (setq isearch-pre-move-point nil)))
+  (force-mode-line-update))
 
 (defun isearch-quote-char (&optional count)
   "Quote special characters for incremental search.
diff --git a/lisp/tmm.el b/lisp/tmm.el
index ff62774..4e3f254 100644
--- a/lisp/tmm.el
+++ b/lisp/tmm.el
@@ -42,6 +42,23 @@ tmm-km-list
 (defvar tmm-next-shortcut-digit)
 (defvar tmm-table-undef)
 
+(defun tmm-menubar-keymap ()
+  "Return the current menu-bar keymap.
+
+The ordering of the return value respects `menu-bar-final-items'."
+  (let ((menu-bar '())
+        (menu-end '()))
+    (map-keymap
+     (lambda (key binding)
+       (push (cons key binding)
+             ;; If KEY is the name of an item that we want to put last,
+             ;; move it to the end.
+             (if (memq key menu-bar-final-items)
+                 menu-end
+               menu-bar)))
+     (tmm-get-keybind [menu-bar]))
+    `(keymap ,@(nreverse menu-bar) ,@(nreverse menu-end))))
+
 ;;;###autoload (define-key global-map "\M-`" 'tmm-menubar)
 ;;;###autoload (define-key global-map [menu-bar mouse-1] 'tmm-menubar-mouse)
 
@@ -58,19 +75,8 @@ tmm-menubar
   (interactive)
   (run-hooks 'menu-bar-update-hook)
   ;; Obey menu-bar-final-items; put those items last.
-  (let ((menu-bar '())
-        (menu-end '())
+  (let ((menu-bar (tmm-menubar-keymap))
 	menu-bar-item)
-    (map-keymap
-     (lambda (key binding)
-       (push (cons key binding)
-             ;; If KEY is the name of an item that we want to put last,
-             ;; move it to the end.
-             (if (memq key menu-bar-final-items)
-                 menu-end
-               menu-bar)))
-     (tmm-get-keybind [menu-bar]))
-    (setq menu-bar `(keymap ,@(nreverse menu-bar) ,@(nreverse menu-end)))
     (if x-position
 	(let ((column 0)
               prev-key)
@@ -154,7 +160,7 @@ tmm--completion-table
 (defvar tmm--history nil)
 
 ;;;###autoload
-(defun tmm-prompt (menu &optional in-popup default-item)
+(defun tmm-prompt (menu &optional in-popup default-item no-execute)
   "Text-mode emulation of calling the bindings in keymap.
 Creates a text-mode menu of possible choices.  You can access the elements
 in the menu in two ways:
@@ -165,7 +171,9 @@ tmm-prompt
 MENU is like the MENU argument to `x-popup-menu': either a
 keymap or an alist of alists.
 DEFAULT-ITEM, if non-nil, specifies an initial default choice.
-Its value should be an event that has a binding in MENU."
+Its value should be an event that has a binding in MENU.
+NO-EXECUTE, if non-nil, means to return the command the user selects
+instead of executing it."
   ;; If the optional argument IN-POPUP is t,
   ;; then MENU is an alist of elements of the form (STRING . VALUE).
   ;; That is used for recursive calls only.
@@ -268,7 +276,7 @@ tmm-prompt
 	   ;; We just did the inner level of a -popup menu.
 	   choice)
 	  ;; We just did the outer level.  Do the inner level now.
-	  (not-menu (tmm-prompt choice t))
+	  (not-menu (tmm-prompt choice t nil no-execute))
 	  ;; We just handled a menu keymap and found another keymap.
 	  ((keymapp choice)
 	   (if (symbolp choice)
@@ -276,11 +284,11 @@ tmm-prompt
 	   (condition-case nil
 	       (require 'mouse)
 	     (error nil))
-	   (tmm-prompt choice))
+	   (tmm-prompt choice nil nil no-execute))
 	  ;; We just handled a menu keymap and found a command.
 	  (choice
 	   (if chosen-string
-	       (progn
+	       (if no-execute choice
 		 (setq last-command-event chosen-string)
 		 (call-interactively choice))
 	     choice)))))