--- Begin Message ---
|
Subject: |
25.0.50; query-replace: incorrect history when replacing the NUL character. |
|
Date: |
Fri, 29 May 2015 10:41:42 +0200 |
|
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.0.50 (gnu/linux) |
>From emacs -Q:
;; We do a query-replace to remove NUL characters and replace them by
;; an empty string:
M-% [query-replace]
C-q [quoted-insert]
0 <return> <return> [exit-minibuffer]
<return> [exit-minibuffer]
;; now we try to do it again, by relying on the history
M-% [query-replace]
<up> [previous-line-or-history-element]
<return> [exit-minibuffer]
;; At this point, emacs asks what to replace "two NUL chars" with,
;; instead of removing NULs like the previous query-replace did.
In GNU Emacs 25.0.50.1 (i686-pc-linux-gnu, X toolkit, Xaw scroll bars)
of 2015-05-07 on Aurora
Windowing system distributor `The X.Org Foundation', version 11.0.11501000
System Description: Ubuntu 14.04.2 LTS
I suggest the following changes :
>From 5eecb757d03a9f6986fd82fad1f2413ff3983726 Mon Sep 17 00:00:00 2001
From: Nicolas Richard <address@hidden>
Date: Fri, 29 May 2015 10:32:05 +0200
Subject: [PATCH 1/2] Avoid confusion in query-replace history when replacing
NUL chars.
* lisp/replace.el (query-replace--split-string): New function.
(query-replace-read-from): Rely on the 'separator' property
instead of searching for the NUL character.
---
lisp/replace.el | 52 ++++++++++++++++++++++++++++++----------------------
1 file changed, 30 insertions(+), 22 deletions(-)
diff --git a/lisp/replace.el b/lisp/replace.el
index 8e71615..0ab9b83 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -136,6 +136,16 @@ See `replace-regexp' and `query-replace-regexp-eval'.")
(defun query-replace-descr (string)
(mapconcat 'isearch-text-char-description string ""))
+(defun query-replace--split-string (string)
+ "Split string STRING at a character with property `separator'"
+ (let* ((length (length string))
+ (split-pos (text-property-any 0 length 'separator t string)))
+ (if (not split-pos)
+ 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)))))
+
(defun query-replace-read-from (prompt regexp-flag)
"Query and return the `from' argument of a query-replace operation.
The return value can also be a pair (FROM . TO) indicating that the user
@@ -174,32 +184,30 @@ wants to replace FROM with TO."
(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)))))
+ (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))
- (let* ((to (if (and (string-match separator from)
- (get-text-property (match-beginning 0) 'separator
from))
- (substring-no-properties from (match-end 0))))
- (from (if to (substring-no-properties from 0 (match-beginning 0))
- (substring-no-properties 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))))))))
+ (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.
--
1.9.1
>From 2daee12b2f1463a921a780f41ff00df670360b61 Mon Sep 17 00:00:00 2001
From: Nicolas Richard <address@hidden>
Date: Fri, 29 May 2015 10:33:35 +0200
Subject: [PATCH 2/2] Add test for previous commit
* test/automated/replace-tests.el: New file.
(query-replace--split-string-tests): Add test for previous commit.
---
test/automated/replace-tests.el | 35 +++++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
create mode 100644 test/automated/replace-tests.el
diff --git a/test/automated/replace-tests.el b/test/automated/replace-tests.el
new file mode 100644
index 0000000..f4e474b
--- /dev/null
+++ b/test/automated/replace-tests.el
@@ -0,0 +1,35 @@
+;;; replace-tests.el --- tests for replace.el.
+
+;; Copyright (C) 2015 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs. If not, see <http://www.gnu.org/licenses/>.
+
+;;; Code:
+
+(require 'ert)
+
+(ert-deftest query-replace--split-string-tests ()
+ (let ((sep (propertize "\0" 'separator t)))
+ (dolist (before '("" "b"))
+ (dolist (after '("" "a"))
+ (should (equal
+ (query-replace--split-string (concat before sep after))
+ (cons before after)))
+ (should (equal
+ (query-replace--split-string (concat before "\0" after))
+ (concat before "\0" after)))))))
+
+;;; replace-tests.el ends here
--
1.9.1
--- End Message ---
--- Begin Message ---
|
Subject: |
Re: bug#20690: 25.0.50; query-replace: incorrect history when replacing the NUL character. |
|
Date: |
Tue, 02 Jun 2015 11:30:13 +0200 |
|
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.0.50 (gnu/linux) |
Juri Linkov <address@hidden> writes:
>> +(defun query-replace--split-string (string)
>> + "Split string STRING at a character with property `separator'"
>> + (let* ((length (length string))
>> + (split-pos (text-property-any 0 length 'separator t string)))
>> + (if (not split-pos)
>> + string
> ======
> This used to be (substring-no-properties string)
Indeed, thanks for spotting this. I've corrected it and mentionned this
bug report.
pushed as
bc9d9bc7a8d56303595899cd66db67ef90d3a4cd and
c7695d0adb125a3817f3df015137287e801e457a
Nico.
--- End Message ---