Package: emacs;
Reported by: Dima Kogan <dima <at> secretsauce.net>
Date: Wed, 27 Jan 2016 22:29:01 UTC
Severity: minor
Found in version 25.0.50
Fixed in version 27.0.50
Done: Juri Linkov <juri <at> linkov.net>
Bug is archived. No further changes may be made.
Message #53 received at 22479 <at> debbugs.gnu.org (full text, mbox):
From: Juri Linkov <juri <at> linkov.net> To: Dima Kogan <dima <at> secretsauce.net> Cc: 22479 <at> debbugs.gnu.org Subject: Re: bug#22479: 25.0.50; isearch and query-replace histories do not remember if we were looking for symbols Date: Sun, 15 Apr 2018 00:22:50 +0300
[Message part 1 (text/plain, inline)]
>>> 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:
[isearch-text-properties.3.patch (text/x-diff, inline)]
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
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.