bug#22479: 25.0.50; isearch and query-replace histories do not remember if we were looking for symbols

[Juri Linkov <juri@linkov.net>]

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

>>> Yes, this is a harder problem.  We have to remember meta-data for
>>> every search history element.  There are several possibilities:
>>>
>>> 4. Adding meta-data by text-properties to the strings in
>>>    ‘search-ring’ and ‘regexp-search-ring’ poses a problem of saving
>>>    the values in the desktop and restoring in another session.
>>
>> This feels like the best solution. I looked at desktop.el, and it
>> doesn't save the properties, as you say. But it could be made to. Do you
>> think some external packages would get broken by this change?
>
> This seems to be the least radical change as concluded in
> https://lists.gnu.org/archive/html/emacs-devel/2018-03/msg00296.html
> The patch makes the search mode to be remembered in isearch as well as
> in query-replace.

As predicted, the solution with using text properties caused more trouble:

1. To be able to use ‘C-s M-p’ and restore a previous symbol isearch
   it was necessary to let-bind minibuffer-allow-text-properties to t
   in isearch-edit-string.

2. To use a previous replacement with text properties required to replace
   ‘substring-no-properties’ with ‘substring’ in ‘query-replace--split-string’.

3. For the same purpose to use e.g. ‘M-% M-p RET’ also required to completely
   rewrite ‘query-replace-descr’ to keep text properties non-destructively
   in the replacement string.

I'm not sure if I found all problems or there will be more trouble,
but at least these fixes were necessary for this patch:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: isearch-text-properties.3.patch --]
[-- Type: text/x-diff, Size: 9059 bytes --]

diff --git a/lisp/isearch.el b/lisp/isearch.el
index 15a1543..c0f6f92 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -1105,7 +1105,9 @@ isearch-done
 
   (if (and (> (length isearch-string) 0) (not nopush))
       ;; Update the ring data.
-      (isearch-update-ring isearch-string isearch-regexp))
+      (isearch-update-ring isearch-string isearch-regexp
+                           `(isearch-regexp-function  ,isearch-regexp-function
+                             isearch-case-fold-search ,isearch-case-fold-search)))
 
   (let ((isearch-mode-end-hook-quit (and nopush (not edit))))
     (run-hooks 'isearch-mode-end-hook))
@@ -1121,13 +1123,14 @@ isearch-done
 
   (and (not edit) isearch-recursive-edit (exit-recursive-edit)))
 
-(defun isearch-update-ring (string &optional regexp)
+(defun isearch-update-ring (string &optional regexp properties)
   "Add STRING to the beginning of the search ring.
 REGEXP if non-nil says use the regexp search ring."
   (add-to-history
    (if regexp 'regexp-search-ring 'search-ring)
-   string
-   (if regexp regexp-search-ring-max search-ring-max)))
+   (if properties (apply 'propertize string properties) string)
+   (if regexp regexp-search-ring-max search-ring-max)
+   t))
 
 ;; Switching buffers should first terminate isearch-mode.
 ;; ;; For Emacs 19, the frame switch event is handled.
@@ -1342,6 +1345,13 @@ with-isearch-suspended
 		  multi-isearch-file-list multi-isearch-file-list-new
 		  multi-isearch-buffer-list multi-isearch-buffer-list-new)
 
+	    (when (memq 'isearch-regexp-function (text-properties-at 0 isearch-string))
+	      (setq isearch-regexp-function
+		    (get-text-property 0 'isearch-regexp-function isearch-string)))
+	    (when (memq 'isearch-case-fold-search (text-properties-at 0 isearch-string))
+	      (setq isearch-case-fold-search
+		    (get-text-property 0 'isearch-case-fold-search isearch-string)))
+
 	    ;; Restore the minibuffer message before moving point.
             (funcall (or isearch-message-function #'isearch-message) nil t)
 
@@ -1408,7 +1418,9 @@ isearch-edit-string
 	  (history-add-new-input nil)
 	  ;; Binding minibuffer-history-symbol to nil is a work-around
 	  ;; for some incompatibility with gmhist.
-	  (minibuffer-history-symbol))
+	  (minibuffer-history-symbol)
+	  ;; Search string might have meta information on text properties.
+	  (minibuffer-allow-text-properties t))
      (setq isearch-new-string
 	   (read-from-minibuffer
 	    (isearch-message-prefix nil isearch-nonincremental)
@@ -1836,7 +1848,11 @@ isearch-query-replace
 	;; `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-recursive-edit nil)
+	(isearch-string-propertized
+	 (propertize isearch-string
+		     'isearch-regexp-function isearch-regexp-function
+		     'isearch-case-fold-search isearch-case-fold-search)))
     (isearch-done nil t)
     (isearch-clean-overlays)
     (if (and isearch-other-end
@@ -1849,12 +1865,12 @@ isearch-query-replace
 			 (< (mark) (point))))))
         (goto-char isearch-other-end))
     (set query-replace-from-history-variable
-         (cons isearch-string
+         (cons isearch-string-propertized
                (symbol-value query-replace-from-history-variable)))
     (perform-replace
-     isearch-string
+     isearch-string-propertized
      (query-replace-read-to
-      isearch-string
+      isearch-string-propertized
       (concat "Query replace"
               (isearch--describe-regexp-mode (or delimited isearch-regexp-function) t)
 	      (if backward " backward" "")
@@ -2562,7 +2578,13 @@ isearch-ring-adjust1
 		      length)))
       (setq isearch-string (nth yank-pointer ring)
 	    isearch-message (mapconcat 'isearch-text-char-description
-				       isearch-string "")))))
+				       isearch-string ""))
+      (when (memq 'isearch-regexp-function (text-properties-at 0 isearch-string))
+	(setq isearch-regexp-function
+	      (get-text-property 0 'isearch-regexp-function isearch-string)))
+      (when (memq 'isearch-case-fold-search (text-properties-at 0 isearch-string))
+	(setq isearch-case-fold-search
+	      (get-text-property 0 'isearch-case-fold-search isearch-string))))))
 
 (defun isearch-ring-adjust (advance)
   ;; Helper for isearch-ring-advance and isearch-ring-retreat
@@ -2778,11 +2800,17 @@ isearch-search-fun
 
 (defun isearch--lax-regexp-function-p ()
   "Non-nil if next regexp-function call should be lax."
-  (not (or isearch-nonincremental
-           (null (car isearch-cmds))
-           (eq (length isearch-string)
-               (length (isearch--state-string
-                        (car isearch-cmds)))))))
+  ;; FIXME: maybe simpler to use this:
+  (or (memq this-command '(isearch-printing-char isearch-del-char))
+      isearch-yank-flag)
+  ;; (not (or isearch-nonincremental
+  ;;          (null (car isearch-cmds))
+  ;;          (eq (length isearch-string)
+  ;;              (length (isearch--state-string
+  ;;                       (car isearch-cmds))))
+  ;;          ;; Search string comes from the history with text properties
+  ;;          (memq 'isearch-regexp-function (text-properties-at 0 isearch-string))))
+  )
 
 (defun isearch-search-fun-default ()
   "Return default functions to use for the search."
diff --git a/lisp/replace.el b/lisp/replace.el
index 4916cb1..b943a2e 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -147,16 +147,27 @@ replace-count
 See `replace-regexp' and `query-replace-regexp-eval'.")
 
 (defun query-replace-descr (string)
-  (mapconcat 'isearch-text-char-description string ""))
+  (let ((string (copy-sequence string)))
+    (dotimes (i (length string) string)
+      (let ((c (aref string i)))
+        (cond
+         ((< c ?\s) (add-text-properties
+                     i (1+ i)
+                     `(display ,(propertize (format "^%c" (+ c 64)) 'face 'escape-glyph))
+                     string))
+         ((= c ?\^?) (add-text-properties
+	              i (1+ i)
+                      `(display ,(propertize "^?" 'face 'escape-glyph))
+                      string)))))))
 
 (defun query-replace--split-string (string)
   "Split string STRING at a substring with property `separator'."
   (let* ((length (length string))
          (split-pos (text-property-any 0 length 'separator t string)))
     (if (not split-pos)
-        (substring-no-properties string)
-      (cons (substring-no-properties string 0 split-pos)
-            (substring-no-properties
+        string
+      (cons (substring string 0 split-pos)
+            (substring
              string (or (text-property-not-all
                          (1+ split-pos) length 'separator t string)
                         length)
@@ -301,7 +312,9 @@ query-replace-read-args
 	 (to (if (consp from) (prog1 (cdr from) (setq from (car from)))
 	       (query-replace-read-to from prompt regexp-flag))))
     (list from to
-	  (and current-prefix-arg (not (eq current-prefix-arg '-)))
+	  (or (and current-prefix-arg (not (eq current-prefix-arg '-)))
+              (and (memq 'isearch-regexp-function (text-properties-at 0 from))
+                   (get-text-property 0 'isearch-regexp-function from)))
 	  (and current-prefix-arg (eq current-prefix-arg '-)))))
 
 (defun query-replace (from-string to-string &optional delimited start end backward region-noncontiguous-p)
@@ -2361,8 +2372,17 @@ perform-replace
          (message
           (if query-flag
               (apply 'propertize
-                     (substitute-command-keys
-                      "Query replacing %s with %s: (\\<query-replace-map>\\[help] for help) ")
+                     (concat "Query replacing "
+                             (if backward "backward " "")
+                             (if delimited-flag
+                                 (or (and (symbolp delimited-flag)
+                                          (get delimited-flag
+                                               'isearch-message-prefix))
+                                     "word ") "")
+                             (if regexp-flag "regexp " "")
+                             "%s with %s: "
+                             (substitute-command-keys
+                              "(\\<query-replace-map>\\[help] for help) "))
                      minibuffer-prompt-properties))))
 
     ;; Unless a single contiguous chunk is selected, operate on multiple chunks.
@@ -2580,13 +2600,13 @@ perform-replace
 			 (with-output-to-temp-buffer "*Help*"
 			   (princ
 			    (concat "Query replacing "
+				    (if backward "backward " "")
 				    (if delimited-flag
 					(or (and (symbolp delimited-flag)
 						 (get delimited-flag
                                                       'isearch-message-prefix))
 					    "word ") "")
 				    (if regexp-flag "regexp " "")
-				    (if backward "backward " "")
 				    from-string " with "
 				    next-replacement ".\n\n"
 				    (substitute-command-keys