bug#25482: 26.0.50; Allow setting `query-replace-from-to-separator` to nil

[Thierry Volpiatto]

Eli Zaretskii <eliz@gnu.org> writes:

> I'm asking whether the (fixed) behavior, whereby using setq to change
> the value to " -> " would add the ASCII string " -> " to the
> minibuffer history, would be acceptable.  If not, please tell why.

Here the patch updated adding the plain string to history when it is an
ascii string.

1 file changed, 92 insertions(+), 74 deletions(-)
lisp/replace.el | 166 +++++++++++++++++++++++++++++++-------------------------

modified   lisp/replace.el
@@ -79,15 +79,17 @@ That becomes the \"string to replace\".")
 to the minibuffer that reads the string to replace, or invoke replacements
 from Isearch by using a key sequence like `C-s C-s M-%'." "24.3")
 
-(defcustom query-replace-from-to-separator
-  (propertize (if (char-displayable-p ?→) " → " " -> ")
-              'face 'minibuffer-prompt)
-  "String that separates FROM and TO in the history of replacement pairs."
-  ;; Avoids error when attempt to autoload char-displayable-p fails
-  ;; while preparing to dump, also stops customize-rogue listing this.
-  :initialize 'custom-initialize-delay
+(defcustom query-replace-from-to-separator " → "
+  "String that separates FROM and TO in the history of replacement pairs.
+When non-nil, the pair (FROM TO) will be added to `query-replace-history' as
+a string \"FROM<separator>TO\".
+When nil the default separator \" -> \" will be used as a plain string
+and the pair will not be added to `query-replace-history'
+\(Same behavior as in emacs 24.5)."
   :group 'matching
-  :type '(choice string (sexp :tag "Display specification"))
+  :type '(choice
+          (const :tag "Disabled" nil)
+          string)
   :version "25.1")
 
 (defcustom query-replace-from-history-variable 'query-replace-history
@@ -150,14 +152,20 @@ See `replace-regexp' and `query-replace-regexp-eval'.")
   (mapconcat 'isearch-text-char-description string ""))
 
 (defun query-replace--split-string (string)
-  "Split string STRING at a character with property `separator'"
+  "Split string STRING at `query-replace-from-to-separator'.
+Split at a character with property `separator' or at place matching regexp
+`query-replace-from-to-separator'."
   (let* ((length (length string))
          (split-pos (text-property-any 0 length 'separator t string)))
-    (if (not split-pos)
-        (substring-no-properties string)
-      (cl-assert (not (text-property-any (1+ split-pos) length 'separator t 
string)))
-      (cons (substring-no-properties string 0 split-pos)
-            (substring-no-properties string (1+ split-pos) length)))))
+    (cond (split-pos
+           (cl-assert (not (text-property-any
+                            (1+ split-pos) length 'separator t string)))
+           (cons (substring-no-properties string 0 split-pos)
+            (substring-no-properties string (1+ split-pos) length)))
+          ((string-match query-replace-from-to-separator string)
+           (cons (substring-no-properties string 0 (match-beginning 0))
+                 (substring-no-properties string (match-end 0) length)))
+          (t (substring-no-properties string)))))
 
 (defun query-replace-read-from (prompt regexp-flag)
   "Query and return the `from' argument of a query-replace operation.
@@ -165,66 +173,76 @@ The return value can also be a pair (FROM . TO) 
indicating that the user
 wants to replace FROM with TO."
   (if query-replace-interactive
       (car (if regexp-flag regexp-search-ring search-ring))
-    ;; Reevaluating will check char-displayable-p that is
-    ;; unavailable while preparing to dump.
-    (custom-reevaluate-setting 'query-replace-from-to-separator)
-    (let* ((history-add-new-input nil)
-          (separator
-           (when query-replace-from-to-separator
-             (propertize "\0"
-                         'display query-replace-from-to-separator
-                         'separator t)))
-          (minibuffer-history
-           (append
-            (when separator
-              (mapcar (lambda (from-to)
-                        (concat (query-replace-descr (car from-to))
-                                separator
-                                (query-replace-descr (cdr from-to))))
-                      query-replace-defaults))
-            (symbol-value query-replace-from-history-variable)))
-          (minibuffer-allow-text-properties t) ; separator uses text-properties
-          (prompt
-           (if (and query-replace-defaults separator)
-               (format "%s (default %s): " prompt (car minibuffer-history))
-             (format "%s: " prompt)))
-          (from
-           ;; The save-excursion here is in case the user marks and copies
-           ;; a region in order to specify the minibuffer input.
-           ;; That should not clobber the region for the query-replace itself.
-           (save-excursion
-              (minibuffer-with-setup-hook
-                  (lambda ()
-                    (setq-local text-property-default-nonsticky
-                                (cons '(separator . t) 
text-property-default-nonsticky)))
-                (if regexp-flag
-                    (read-regexp prompt nil 'minibuffer-history)
-                  (read-from-minibuffer
-                   prompt nil nil nil nil
-                   (car (if regexp-flag regexp-search-ring search-ring)) t)))))
-           (to))
-      (if (and (zerop (length from)) query-replace-defaults)
-         (cons (caar query-replace-defaults)
-               (query-replace-compile-replacement
-                (cdar query-replace-defaults) regexp-flag))
-        (setq from (query-replace--split-string from))
-        (when (consp from) (setq to (cdr from) from (car from)))
-        (add-to-history query-replace-from-history-variable from nil t)
-        ;; Warn if user types \n or \t, but don't reject the input.
-        (and regexp-flag
-             (string-match "\\(\\`\\|[^\\]\\)\\(\\\\\\\\\\)*\\(\\\\[nt]\\)" 
from)
-             (let ((match (match-string 3 from)))
-               (cond
-                ((string= match "\\n")
-                 (message "Note: `\\n' here doesn't match a newline; to do 
that, type C-q C-j instead"))
-                ((string= match "\\t")
-                 (message "Note: `\\t' here doesn't match a tab; to do that, 
just type TAB")))
-               (sit-for 2)))
-        (if (not to)
-            from
-          (add-to-history query-replace-to-history-variable to nil t)
-          (add-to-history 'query-replace-defaults (cons from to) nil t)
-          (cons from (query-replace-compile-replacement to regexp-flag)))))))
+    (let ((sep-char (replace-regexp-in-string
+                     " " "" query-replace-from-to-separator)))
+      (when (stringp query-replace-from-to-separator)
+        (setq query-replace-from-to-separator
+              (propertize (if (char-displayable-p (string-to-char sep-char))
+                              query-replace-from-to-separator
+                            " -> ")
+                          'face 'minibuffer-prompt)))
+      (let* ((history-add-new-input nil)
+             (separator
+              (if (and query-replace-from-to-separator
+                       (> (string-bytes sep-char) (length sep-char)))
+                  (propertize "\0"
+                              'display query-replace-from-to-separator
+                              'separator t)
+                (propertize query-replace-from-to-separator 'separator t)))
+             (minibuffer-history
+              (append
+               (when separator
+                 (mapcar (lambda (from-to)
+                           (concat (query-replace-descr (car from-to))
+                                   separator
+                                   (query-replace-descr (cdr from-to))))
+                         query-replace-defaults))
+               (symbol-value query-replace-from-history-variable)))
+             (minibuffer-allow-text-properties t) ; separator uses 
text-properties
+             (prompt
+              (cond ((and query-replace-defaults separator)
+                     (format "%s (default %s): " prompt (car 
minibuffer-history)))
+                    (query-replace-defaults
+                     (format "%s (default %s -> %s): " prompt
+                             (query-replace-descr (caar 
query-replace-defaults))
+                             (query-replace-descr (cdar 
query-replace-defaults))))
+                    (t (format "%s: " prompt))))
+             (from
+              ;; The save-excursion here is in case the user marks and copies
+              ;; a region in order to specify the minibuffer input.
+              ;; That should not clobber the region for the query-replace 
itself.
+              (save-excursion
+                (minibuffer-with-setup-hook
+                    (lambda ()
+                      (setq-local text-property-default-nonsticky
+                                  (cons '(separator . t) 
text-property-default-nonsticky)))
+                  (if regexp-flag
+                      (read-regexp prompt nil 'minibuffer-history)
+                    (read-from-minibuffer
+                     prompt nil nil nil nil (car search-ring) t)))))
+             (to))
+        (if (and (zerop (length from)) query-replace-defaults)
+            (cons (caar query-replace-defaults)
+                  (query-replace-compile-replacement
+                   (cdar query-replace-defaults) regexp-flag))
+          (setq from (query-replace--split-string from))
+          (when (consp from) (setq to (cdr from) from (car from)))
+          (add-to-history query-replace-from-history-variable from nil t)
+          ;; Warn if user types \n or \t, but don't reject the input.
+          (and regexp-flag
+               (string-match "\\(\\`\\|[^\\]\\)\\(\\\\\\\\\\)*\\(\\\\[nt]\\)" 
from)
+               (let ((match (match-string 3 from)))
+                 (cond
+                   ((string= match "\\n")
+                    (message "Note: `\\n' here doesn't match a newline; to do 
that, type C-q C-j instead"))
+                   ((string= match "\\t")
+                    (message "Note: `\\t' here doesn't match a tab; to do 
that, just type TAB")))
+                 (sit-for 2)))
+          (if (not to)
+              from
+            (add-to-history query-replace-to-history-variable to nil t)
+            (add-to-history 'query-replace-defaults (cons from to) nil t)
+            (cons from (query-replace-compile-replacement to 
regexp-flag))))))))
 
 (defun query-replace-compile-replacement (to regexp-flag)
   "Maybe convert a regexp replacement TO to Lisp.

-- 
Thierry