From: Nicolas Richard <youngfrog@members.fsf.org>
To: 20690@debbugs.gnu.org
Subject: bug#20690: 25.0.50; query-replace: incorrect history when replacing the NUL character.
Date: Fri, 29 May 2015 10:41:42 +0200 [thread overview]
Message-ID: <86d21jzsft.fsf@members.fsf.org> (raw)
[-- Attachment #1: Type: text/plain, Size: 739 bytes --]
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 :
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Avoid-confusion-in-query-replace-history-when-replac.patch --]
[-- Type: text/x-diff, Size: 4261 bytes --]
From 5eecb757d03a9f6986fd82fad1f2413ff3983726 Mon Sep 17 00:00:00 2001
From: Nicolas Richard <youngfrog@members.fsf.org>
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
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Add-test-for-previous-commit.patch --]
[-- Type: text/x-diff, Size: 1997 bytes --]
From 2daee12b2f1463a921a780f41ff00df670360b61 Mon Sep 17 00:00:00 2001
From: Nicolas Richard <youngfrog@members.fsf.org>
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
next reply other threads:[~2015-05-29 8:41 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-05-29 8:41 Nicolas Richard [this message]
2015-06-01 20:39 ` bug#20690: 25.0.50; query-replace: incorrect history when replacing the NUL character Juri Linkov
2015-06-02 9:30 ` Nicolas Richard
2015-06-22 22:59 ` Juri Linkov
2015-06-23 5:11 ` Nicolas Richard
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=86d21jzsft.fsf@members.fsf.org \
--to=youngfrog@members.fsf.org \
--cc=20690@debbugs.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).