bug#23127: 25.0.92; Assertion failure when editing history in query-replace in cc-mode

[Alan Mackenzie]

Hello, Richard.

In article <mailman.8469.1459108386.843.bug-gnu-emacs@gnu.org> you wrote:
> In cc-mode, while doing query-replace, edit a previous replacement in
> the minibuffer, and insert characters after the " → " separator. The
> subsequent emacs-lisp chokes on the text properties of the resulting
> string, with an assertion failure.

> Recipe starting from 'emacs -Q':

> M-x c-mode RET
> M-<
> C-% a RET b RET !
> M-<
> C-% M-p C-e <backspace> c RET

> ==> error "cl--assertion-failed: Assertion failed: (not
> (text-property-any (1+ split-pos) length (quote separator) t string))"

> The callstack looks like this (except that here I've replaced a NUL with 
> "^@"):

> Debugger entered: ((cl-assertion-failed (not (text-property-any (1+
> split-pos) length (quote separator) t string)) nil))
>   cl--assertion-failed((not (text-property-any (1+ split-pos) length
> (quote separator) t string)))
>   query-replace--split-string(#("a^@c" 1 2 (display #(" → " 0 3 (face
> minibuffer-prompt)) separator t) 2 3 (separator t)))
>   query-replace-read-from("Query replace" nil)
>   query-replace-read-args("Query replace" nil)
>   (let ((common (query-replace-read-args (concat "Query replace" (if
> current-prefix-arg (if (eq current-prefix-arg ...) " backward" "
> word") "") (if (use-region-p) " in region" "")) nil))) (list (nth 0
> common) (nth 1 common) (nth 2 common) (if (use-region-p)
> (region-beginning)) (if (use-region-p) (region-end)) (nth 3 common)
> (if (use-region-p) (region-noncontiguous-p))))
>   call-interactively(query-replace nil nil)
>   command-execute(query-replace)

> As you can see, the "c" inherited the separator property, and
> query-replace--split-string choked on that.
> It doesn't happen in all modes. It does happen in c-mode and c++-mode.

Thanks for such a high quality bug report.  The cause was a critical
variable being buffer local in C Mode and C++ Mode.  This buffer local
binding was not available in `read-from-minibuffer', with the result that
the `separator' text property got spread to the newly inserted 'c'.

The following patch should fix the bug.  Would you please try it out and
confirm that the bug is indeed fixed, or report what's still wrong.
Thanks!

> In GNU Emacs 25.0.92.1 (x86_64-w64-mingw32)
>  of 2016-03-21 built on MACHINE
> Repository revision: 76ef52267cf887e3e1aa6d25b3b16dd0601dd459
> Windowing system distributor 'Microsoft Corp.', version 10.0.10586
> Configured using:
>  'configure --prefix /c/emacs/emacs-20160321-102640
>  --without-imagemagick --disable-dependency-tracking
>  --enable-locallisppath=%emacs_dir%/../site-lisp 'CFLAGS=-Og -g -ggdb''



diff --git a/lisp/replace.el b/lisp/replace.el
index 428be3c..412f827 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -167,8 +167,6 @@ query-replace-read-from
     ;; unavailable while preparing to dump.
     (custom-reevaluate-setting 'query-replace-from-to-separator)
     (let* ((history-add-new-input nil)
-          (text-property-default-nonsticky
-           (cons '(separator . t) text-property-default-nonsticky))
           (separator
            (when query-replace-from-to-separator
              (propertize "\0"
@@ -195,9 +193,16 @@ query-replace-read-from
            (save-excursion
              (if regexp-flag
                  (read-regexp prompt nil 'query-replace-from-to-history)
-               (read-from-minibuffer
-                prompt nil nil nil 'query-replace-from-to-history
-                (car (if regexp-flag regexp-search-ring search-ring)) t))))
+                ;; The `with-current-buffer' ensures that the binding
+                ;; for `text-property-default-nonsticky' isn't a
+                ;; buffer local binding in the current buffer, which
+                ;; `read-from-minibuffer' wouldn't see.
+                (with-current-buffer (window-buffer (minibuffer-window))
+                  (let ((text-property-default-nonsticky
+                         (cons '(separator . t) 
text-property-default-nonsticky)))
+                    (read-from-minibuffer
+                     prompt nil nil nil 'query-replace-from-to-history
+                     (car (if regexp-flag regexp-search-ring search-ring)) 
t))))))
            (to))
       (if (and (zerop (length from)) query-replace-defaults)
          (cons (caar query-replace-defaults)



-- 
Alan Mackenzie (Nuremberg, Germany).