Subject: Re: bug#4981: C-l during query-replace

From: Dan Nicolaescu <[email-masked]>
Date: Sun, 29 Nov 2009 22:29:59 -0800 (PST)

Juri Linkov <[email-masked]> writes:

  > > I guess there are many other places that need replacing with the new
  > > command (e.g. `gnus-recenter').
  > 
  > I fixed `gnus-recenter' in gnus-sum.el.
  > 
  > > But with the patch I proposed in
  > > http://thread.gmane.org/gmane.emacs.devel/110349/focus=115915
  > > the name `recenter-top-bottom' makes no sense anymore.
  > > Maybe we should rename it to something more suitable
  > > before replacing `recenter' calls with the new name everywhere?
  > 
  > Installed.  Currently I have no opinion about renaming
  > `recenter-top-bottom' to something more reasonable.
  > 
  > >> C-l during query-replace should run `recenter-top-bottom', not
  > >> `recenter' for consistency with what C-l normally does nowadays.
  > 
  > I can't find a clean solution because in the case of query-replace,
  > `this-command' is always `query-replace'.
  > 
  > This patch kinda works (though it doesn't reset the cycling order),
  > but I don't like this.
  > 
  > Index: lisp/window.el
  > ===================================================================
  > RCS file: /sources/emacs/emacs/lisp/window.el,v
  > retrieving revision 1.190
  > diff -u -r1.190 window.el
  > --- lisp/window.el	29 Nov 2009 23:34:09 -0000	1.190
  > +++ lisp/window.el	29 Nov 2009 23:42:30 -0000
  > @@ -1654,7 +1654,8 @@
  >     (arg (recenter arg))			; Always respect ARG.
  >     (t
  >      (setq recenter-last-op
  > -	  (if (eq this-command last-command)
  > +	  (if (or (eq this-command last-command)
  > +		  (eq this-command 'query-replace))
  >  	      (car (or (cdr (member recenter-last-op recenter-positions))
  >  		       recenter-positions))
  >  	    (car recenter-positions)))
  > 
  > Index: lisp/replace.el
  > ===================================================================
  > RCS file: /sources/emacs/emacs/lisp/replace.el,v
  > retrieving revision 1.287
  > diff -u -r1.287 replace.el
  > --- lisp/replace.el	12 Nov 2009 06:55:43 -0000	1.287
  > +++ lisp/replace.el	29 Nov 2009 23:43:28 -0000
  > @@ -1785,7 +1788,9 @@
  >  			((eq def 'skip)
  >  			 (setq done t))
  >  			((eq def 'recenter)
  > -			 (recenter nil))
  > +			 (recenter-top-bottom))
  >  			((eq def 'edit)
  >  			 (let ((opos (point-marker)))
  >  			   (setq real-match-data (replace-match-data

Thanks for fixing this.  Are you sure that the new `recenter-positions'
is needed?  Given that there are 3 choices, it's easy to cycle through
them, so adding yet another defcustom that would be use by a very small
number of users does not seem justified (IMHO).