From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Nicolas Richard Newsgroups: gmane.emacs.bugs Subject: bug#20690: 25.0.50; query-replace: incorrect history when replacing the NUL character. Date: Fri, 29 May 2015 10:41:42 +0200 Message-ID: <86d21jzsft.fsf@members.fsf.org> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Trace: ger.gmane.org 1432889313 25473 80.91.229.3 (29 May 2015 08:48:33 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Fri, 29 May 2015 08:48:33 +0000 (UTC) To: 20690@debbugs.gnu.org Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Fri May 29 10:48:22 2015 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1YyFxd-0008A3-L2 for geb-bug-gnu-emacs@m.gmane.org; Fri, 29 May 2015 10:48:13 +0200 Original-Received: from localhost ([::1]:34407 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YyFxc-00057p-TC for geb-bug-gnu-emacs@m.gmane.org; Fri, 29 May 2015 04:48:12 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:51568) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YyFxY-00056p-0x for bug-gnu-emacs@gnu.org; Fri, 29 May 2015 04:48:09 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YyFxS-00055v-Vp for bug-gnu-emacs@gnu.org; Fri, 29 May 2015 04:48:07 -0400 Original-Received: from debbugs.gnu.org ([140.186.70.43]:50083) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YyFxS-00055o-SB for bug-gnu-emacs@gnu.org; Fri, 29 May 2015 04:48:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.80) (envelope-from ) id 1YyFxS-0003kA-Bn for bug-gnu-emacs@gnu.org; Fri, 29 May 2015 04:48:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Nicolas Richard Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 29 May 2015 08:48:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: report 20690 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: X-Debbugs-Original-To: bug-gnu-emacs@gnu.org Original-Received: via spool by submit@debbugs.gnu.org id=B.143288924114337 (code B ref -1); Fri, 29 May 2015 08:48:02 +0000 Original-Received: (at submit) by debbugs.gnu.org; 29 May 2015 08:47:21 +0000 Original-Received: from localhost ([127.0.0.1]:60058 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1YyFwl-0003jA-Ry for submit@debbugs.gnu.org; Fri, 29 May 2015 04:47:20 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:37864) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1YyFwk-0003iy-7K for submit@debbugs.gnu.org; Fri, 29 May 2015 04:47:19 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YyFwd-0004xO-D9 for submit@debbugs.gnu.org; Fri, 29 May 2015 04:47:12 -0400 Original-Received: from lists.gnu.org ([2001:4830:134:3::11]:42571) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YyFwd-0004xK-AH for submit@debbugs.gnu.org; Fri, 29 May 2015 04:47:11 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:51272) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YyFwa-0004m9-Qh for bug-gnu-emacs@gnu.org; Fri, 29 May 2015 04:47:11 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YyFwV-0004sS-Hh for bug-gnu-emacs@gnu.org; Fri, 29 May 2015 04:47:08 -0400 Original-Received: from mxin.ulb.ac.be ([164.15.128.112]:40558) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YyFwV-0004ri-4Y for bug-gnu-emacs@gnu.org; Fri, 29 May 2015 04:47:03 -0400 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: AqYEABwnaFWkD4XH/2dsb2JhbABcg2TGUAKCEAEBAQEBAYELhCIBgRcUAwECCjQBBDshiC2gCrQfkDgehCcFhmuLeIxihkAwiB6DIoNZI2GDGTyCeAEBAQ Original-Received: from pno-math-199.ulb.ac.be (HELO Aurora) ([164.15.133.199]) by smtp.ulb.ac.be with ESMTP; 29 May 2015 10:41:42 +0200 User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.0.50 (gnu/linux) X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 140.186.70.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:103297 Archived-At: --=-=-= Content-Type: text/plain >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 [exit-minibuffer] [exit-minibuffer] ;; now we try to do it again, by relying on the history M-% [query-replace] [previous-line-or-history-element] [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 : --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=0001-Avoid-confusion-in-query-replace-history-when-replac.patch >From 5eecb757d03a9f6986fd82fad1f2413ff3983726 Mon Sep 17 00:00:00 2001 From: Nicolas Richard 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 --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=0002-Add-test-for-previous-commit.patch >From 2daee12b2f1463a921a780f41ff00df670360b61 Mon Sep 17 00:00:00 2001 From: Nicolas Richard 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 . + +;;; 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 --=-=-=--