GNU bug report logs - #13579
24.3.50; query-replace ressurrects previous minibuffer contents

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 13579 in the body.
You can then email your comments to 13579 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: <rcopley <at> gmail.com>
To: "bug-gnu-emacs <at> gnu.org" <bug-gnu-emacs <at> gnu.org>
Subject: 24.3.50;
	 query-replace ressurrects previous minibuffer contents
Date: Mon, 28 Jan 2013 21:17:10 +0000

[Message part 1 (text/plain, inline)]

From "emacs -Q":
  M-x set-variable <return>
  enable-recursive-minibuffers <return>
  t <return> 
  M-x switch-to-butter
  C-a M-%
  t <return> 
  f <return>
  !
  ;; wait two seconds


Expected (in minibuffer): "swifch-to-buffer".
Actual (in minibutter): "sef-variable".




In GNU Emacs 24.3.50.1 (i386-mingw-nt6.2.9200)
 of 2013-01-28 on MACHINE
Bzr revision: 111617 monnier <at> iro.umontreal.ca-20130128200035-l5y6lyn7rerbmfjn
Windowing system distributor `Microsoft Corp.', version 6.2.9200
Configured using:
 `configure --with-gcc (4.7) --cflags -I c:/gnuwin32/include --ldflags
 -L c:/gnuwin32/lib'


Important settings:
  value of $LANG: ENG
  locale-coding-system: cp1252
  default enable-multibyte-characters: t


Major mode: Lisp Interaction


Minor modes in effect:
  tooltip-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t


Recent input:
M-x s e t - v a r <return> e n a b l e - r e c u r 
s i v e <return> t <return> M-x s w i t c h - b u t 
t e r C-a M-% t <return> f <return> ! C-g M-x r e p 
o r t - e m a c s - b u g <return>


Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Replaced 1 occurrence
Quit


Load-path shadows:
None found.


Features:
(shadow sort gnus-util mail-extr emacsbug message format-spec rfc822 mml
mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev
gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util
mail-prsvr mail-utils cus-edit easymenu wid-edit cus-start cus-load
help-fns time-date tooltip ediff-hook vc-hooks lisp-float-type mwheel
dos-w32 ls-lisp w32-common-fns disp-table w32-win w32-vars tool-bar dnd
fontset image regexp-opt fringe tabulated-list newcomment lisp-mode
register page menu-bar rfn-eshadow timer select scroll-bar mouse
jit-lock font-lock syntax facemenu font-core frame cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese case-table epa-hook jka-cmpr-hook help simple abbrev
minibuffer button faces cus-face macroexp files text-properties overlay
sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote make-network-process w32notify w32
multi-tty emacs)

[Message part 2 (text/html, inline)]

Message #8 received at 13579 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> jurta.org>
To: <rcopley <at> gmail.com>
Cc: 13579 <at> debbugs.gnu.org
Subject: Re: bug#13579: 24.3.50;
	query-replace ressurrects previous minibuffer contents
Date: Wed, 30 Jan 2013 08:03:30 +0200

> From "emacs -Q":
>   M-x set-variable <return>
>   enable-recursive-minibuffers <return>
>   t <return>
>   M-x switch-to-butter
>   C-a M-%
>   t <return>
>   f <return>
>   !
>   ;; wait two seconds
>
> Expected (in minibuffer): "swifch-to-buffer".
> Actual (in minibutter): "sef-variable".

Thanks for the report.  Initially `isearch-forward' is nil, so M-% tries
to search backward going through the minibuffer history (thus it uses the
global value of `isearch-forward' in `minibuffer-history-isearch-search').

It is evident now from your report that the search space for replacements
should be narrowed to the strict set of search functions defined in isearch.el.

This is a regression, so I propose to install the following patch to the
emacs-24 release branch:

=== modified file 'lisp/replace.el'
--- lisp/replace.el	2013-01-01 09:11:05 +0000
+++ lisp/replace.el	2013-01-30 06:03:10 +0000
@@ -1831,7 +1831,7 @@ (defun perform-replace (from-string repl
 		     replace-regexp-lax-whitespace)
 		    (isearch-case-fold-search case-fold-search)
 		    (isearch-forward t))
-		(isearch-search-fun))))
+		(isearch-search-fun-default))))
          (search-string from-string)
          (real-match-data nil)       ; The match data for the current match.
          (next-replacement nil)

Message #11 received at 13579 <at> debbugs.gnu.org (full text, mbox):

From: "Drew Adams" <drew.adams <at> oracle.com>
To: "'Juri Linkov'" <juri <at> jurta.org>, <rcopley <at> gmail.com>
Cc: 13579 <at> debbugs.gnu.org
Subject: RE: bug#13579: 24.3.50;
	query-replace ressurrects previous minibuffer contents
Date: Wed, 30 Jan 2013 06:00:13 -0800

>  M-x switch-to-butter

Thank you!  That one made my day.

Message #14 received at 13579 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> jurta.org>
To: <rcopley <at> gmail.com>
Cc: 13579 <at> debbugs.gnu.org
Subject: Re: bug#13579: 24.3.50;
	query-replace ressurrects previous minibuffer contents
Date: Thu, 31 Jan 2013 02:07:04 +0200

> It is evident now from your report that the search space for replacements
> should be narrowed to the strict set of search functions defined in isearch.el.

If someone wants to perform replacements through the minibuffer history,
this can be implemented as a new separate feature like multi-buffer
replacements (where going to the previous history element is like
going to the previous buffer in a sequence.)

> This is a regression, so I propose to install the following patch to the
> emacs-24 release branch:

`isearch-search-fun-default' prevents the search from going through
the history, but it fixes only part of the problem.  Another part of
the problem is that it tried to go backwards through the history
using the global nil value of `isearch-forward' and ignoring the
let-binding t of `isearch-forward' in `perform-replace'.

That's because `isearch-forward' is not lexically bound when the
lambda in `minibuffer-history-isearch-search' is evaluated.
Now `isearch-search-fun-default' should not use that lambda,
but there is another lambda in `isearch-search-fun-default'
used for `isearch-word' mode.  It causes another regression
when using delimited replacements in the minibuffer - the let-bindings
of isearch variables in `perform-replace' such as binding
`isearch-word' to `delimited-flag' are no more effective
at the moment when the lambda in `isearch-search-fun-default'
is evaluated.

This regression can be fixed by the following patch that will let-bind
isearch-related variables at the moment when the search function
is called:

=== modified file 'lisp/replace.el'
--- lisp/replace.el	2013-01-01 09:11:05 +0000
+++ lisp/replace.el	2013-01-31 00:04:17 +0000
@@ -1823,15 +1823,17 @@ (defun perform-replace (from-string repl
 	  (or (if regexp-flag
 		  replace-re-search-function
 		replace-search-function)
-	      (let ((isearch-regexp regexp-flag)
-		    (isearch-word delimited-flag)
-		    (isearch-lax-whitespace
-		     replace-lax-whitespace)
-		    (isearch-regexp-lax-whitespace
-		     replace-regexp-lax-whitespace)
-		    (isearch-case-fold-search case-fold-search)
-		    (isearch-forward t))
-		(isearch-search-fun))))
+	      (lambda (string &optional bound noerror count)
+		(let ((isearch-regexp regexp-flag)
+		      (isearch-word delimited-flag)
+		      (isearch-lax-whitespace
+		       replace-lax-whitespace)
+		      (isearch-regexp-lax-whitespace
+		       replace-regexp-lax-whitespace)
+		      (isearch-case-fold-search case-fold-search)
+		      (isearch-nonincremental t)
+		      (isearch-forward t))
+		  (funcall (isearch-search-fun-default) string bound noerror count)))))
          (search-string from-string)
          (real-match-data nil)       ; The match data for the current match.
          (next-replacement nil)

Message #17 received at 13579 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Juri Linkov <juri <at> jurta.org>
Cc: rcopley <at> gmail.com, 13579 <at> debbugs.gnu.org
Subject: Re: bug#13579: 24.3.50;
	query-replace ressurrects previous minibuffer contents
Date: Wed, 30 Jan 2013 19:32:37 -0500

> This regression can be fixed by the following patch that will let-bind
> isearch-related variables at the moment when the search function
> is called:

A better fix is to say that isearch-search-fun-function should return
a function that does not pay attention to isearch-* variables.


        Stefan

Message #20 received at 13579 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> jurta.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: rcopley <at> gmail.com, 13579 <at> debbugs.gnu.org
Subject: Re: bug#13579: 24.3.50;
	query-replace ressurrects previous minibuffer contents
Date: Fri, 01 Feb 2013 02:17:33 +0200

> A better fix is to say that isearch-search-fun-function should return
> a function that does not pay attention to isearch-* variables.

It is possible to construct a lambda for `isearch-word' in
`isearch-search-fun-default' that won't use isearch-* variables.
Rewriting more complex functions like `minibuffer-history-isearch-search'
would be more tricky.  And may be some external packages use global values too.
Also there are some variables that still will use the global values like
using `search-whitespace-regexp' in `search-forward-lax-whitespace'
(currently this shouldn't cause a problem since replace.el doesn't use
a replace-specific counterpart option for `search-whitespace-regexp';
introducing an option `replace-whitespace-regexp' might break this).
I'll try to do this in trunk.

Regarding fixing the regression in the emacs-24 branch, I propose a patch
that let-binds isearch-* variables in the same place where `search-function'
is funcalled.  I believe this is a more reliable and less error-prone fix.

=== modified file 'lisp/replace.el'
--- lisp/replace.el	2013-01-01 09:11:05 +0000
+++ lisp/replace.el	2013-02-01 00:15:31 +0000
@@ -1819,19 +1819,6 @@ (defun perform-replace (from-string repl
 	    case-fold-search))
          (nocasify (not (and case-replace case-fold-search)))
          (literal (or (not regexp-flag) (eq regexp-flag 'literal)))
-         (search-function
-	  (or (if regexp-flag
-		  replace-re-search-function
-		replace-search-function)
-	      (let ((isearch-regexp regexp-flag)
-		    (isearch-word delimited-flag)
-		    (isearch-lax-whitespace
-		     replace-lax-whitespace)
-		    (isearch-regexp-lax-whitespace
-		     replace-regexp-lax-whitespace)
-		    (isearch-case-fold-search case-fold-search)
-		    (isearch-forward t))
-		(isearch-search-fun))))
          (search-string from-string)
          (real-match-data nil)       ; The match data for the current match.
          (next-replacement nil)
@@ -1894,6 +1881,26 @@ (defun perform-replace (from-string repl
 	;; Loop finding occurrences that perhaps should be replaced.
 	(while (and keep-going
 		    (not (or (eobp) (and limit (>= (point) limit))))
+		    ;; Let-bind global isearch-* variables to values used
+		    ;; to search the next replacement.  These let-bindings
+		    ;; should be effective both at the time of calling
+		    ;; `isearch-search-fun-default' and also at the
+		    ;; time of funcalling `search-function'.
+		    (let* ((isearch-regexp regexp-flag)
+			   (isearch-word delimited-flag)
+			   (isearch-lax-whitespace
+			    replace-lax-whitespace)
+			   (isearch-regexp-lax-whitespace
+			    replace-regexp-lax-whitespace)
+			   (isearch-case-fold-search case-fold-search)
+			   (isearch-adjusted nil)
+			   (isearch-nonincremental t)
+			   (isearch-forward t)
+			   (search-function
+			    (or (if regexp-flag
+				    replace-re-search-function
+				  replace-search-function)
+				(isearch-search-fun-default))))
 		    ;; Use the next match if it is already known;
 		    ;; otherwise, search for a match after moving forward
 		    ;; one char if progress is required.
@@ -1926,7 +1933,7 @@ (defun perform-replace (from-string repl
 				       (replace-match-data
 					t real-match-data)
 				     (goto-char opoint)
-				     nil))))))
+				       nil)))))))
 
 	  ;; Record whether the match is nonempty, to avoid an infinite loop
 	  ;; repeatedly matching the same empty string.

Message #23 received at 13579 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Juri Linkov <juri <at> jurta.org>
Cc: rcopley <at> gmail.com, 13579 <at> debbugs.gnu.org
Subject: Re: bug#13579: 24.3.50;
	query-replace ressurrects previous minibuffer contents
Date: Fri, 01 Feb 2013 09:15:17 -0500

> I'll try to do this in trunk.

Thanks.

> Regarding fixing the regression in the emacs-24 branch, I propose a patch
> that let-binds isearch-* variables in the same place where `search-function'
> is funcalled.  I believe this is a more reliable and less error-prone fix.

That looks fine, yes, thank you,


        Stefan

Message #28 received at 13579-done <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> jurta.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: rcopley <at> gmail.com, 13579-done <at> debbugs.gnu.org
Subject: Re: bug#13579: 24.3.50;
	query-replace ressurrects previous minibuffer contents
Date: Sat, 02 Feb 2013 01:39:10 +0200

Version: 24.2.92

> That looks fine, yes, thank you,

Fixed in emacs-24 branch.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.