* bug#4981: C-l during query-replace @ 2009-11-20 0:16 ` Dan Nicolaescu 2009-11-20 9:29 ` Juri Linkov 2009-11-30 16:30 ` bug#4981: marked as done (C-l during query-replace) Emacs bug Tracking System 0 siblings, 2 replies; 12+ messages in thread From: Dan Nicolaescu @ 2009-11-20 0:16 UTC (permalink / raw) To: bug-gnu-emacs C-l during query-replace should run `recenter-top-bottom', not `recenter' for consistency with what C-l normally does nowadays. ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#4981: C-l during query-replace 2009-11-20 0:16 ` bug#4981: C-l during query-replace Dan Nicolaescu @ 2009-11-20 9:29 ` Juri Linkov 2009-11-29 23:44 ` Juri Linkov 2009-11-30 16:30 ` bug#4981: marked as done (C-l during query-replace) Emacs bug Tracking System 1 sibling, 1 reply; 12+ messages in thread From: Juri Linkov @ 2009-11-20 9:29 UTC (permalink / raw) To: Dan Nicolaescu; +Cc: 4981 > C-l during query-replace should run `recenter-top-bottom', not > `recenter' for consistency with what C-l normally does nowadays. I guess there are many other places that need replacing with the new command (e.g. `gnus-recenter'). 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? -- Juri Linkov http://www.jurta.org/emacs/ ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#4981: C-l during query-replace 2009-11-20 9:29 ` Juri Linkov @ 2009-11-29 23:44 ` Juri Linkov 2009-11-30 1:28 ` Stefan Monnier 2009-11-30 6:29 ` Dan Nicolaescu 0 siblings, 2 replies; 12+ messages in thread From: Juri Linkov @ 2009-11-29 23:44 UTC (permalink / raw) To: Dan Nicolaescu; +Cc: 4981 > 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 -- Juri Linkov http://www.jurta.org/emacs/ ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#4981: C-l during query-replace 2009-11-29 23:44 ` Juri Linkov @ 2009-11-30 1:28 ` Stefan Monnier 2009-11-30 6:29 ` Dan Nicolaescu 1 sibling, 0 replies; 12+ messages in thread From: Stefan Monnier @ 2009-11-30 1:28 UTC (permalink / raw) To: Juri Linkov; +Cc: 4981, Dan Nicolaescu > I can't find a clean solution because in the case of query-replace, > `this-command' is always `query-replace'. Why not: > 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)) > + (let ((this-command 'recenter-top-bottom)) > + (recenter-top-bottom))) > ((eq def 'edit) > (let ((opos (point-marker))) > (setq real-match-data (replace-match-data -- Stefan ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#4981: C-l during query-replace 2009-11-29 23:44 ` Juri Linkov 2009-11-30 1:28 ` Stefan Monnier @ 2009-11-30 6:29 ` Dan Nicolaescu 2009-11-30 11:12 ` Juanma Barranquero 2009-11-30 14:13 ` Stefan Monnier 1 sibling, 2 replies; 12+ messages in thread From: Dan Nicolaescu @ 2009-11-30 6:29 UTC (permalink / raw) To: Juri Linkov; +Cc: 4981 Juri Linkov <juri@jurta.org> 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). ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#4981: C-l during query-replace 2009-11-30 6:29 ` Dan Nicolaescu @ 2009-11-30 11:12 ` Juanma Barranquero 2009-11-30 12:04 ` Juri Linkov 2009-11-30 14:13 ` Stefan Monnier 1 sibling, 1 reply; 12+ messages in thread From: Juanma Barranquero @ 2009-11-30 11:12 UTC (permalink / raw) To: Dan Nicolaescu; +Cc: 4981 On Mon, Nov 30, 2009 at 07:29, Dan Nicolaescu <dann@ics.uci.edu> wrote: > Are you sure that the new `recenter-positions' > is needed? Given that there are 3 choices, it's easy to cycle through > them With Juri's `recenter-positions', there are more than 3 choices. Juanma ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#4981: C-l during query-replace 2009-11-30 11:12 ` Juanma Barranquero @ 2009-11-30 12:04 ` Juri Linkov 0 siblings, 0 replies; 12+ messages in thread From: Juri Linkov @ 2009-11-30 12:04 UTC (permalink / raw) To: Juanma Barranquero; +Cc: 4981, Dan Nicolaescu >> Are you sure that the new `recenter-positions' >> is needed? Given that there are 3 choices, it's easy to cycle through >> them > > With Juri's `recenter-positions', there are more than 3 choices. Or what is more important, also *less* than 3 choices :-) E.g. when two positions is enough, I'd like to use for `recenter-positions' the value `(0.15 top)'. -- Juri Linkov http://www.jurta.org/emacs/ ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#4981: C-l during query-replace 2009-11-30 6:29 ` Dan Nicolaescu 2009-11-30 11:12 ` Juanma Barranquero @ 2009-11-30 14:13 ` Stefan Monnier 2009-11-30 16:07 ` Juri Linkov 1 sibling, 1 reply; 12+ messages in thread From: Stefan Monnier @ 2009-11-30 14:13 UTC (permalink / raw) To: Dan Nicolaescu; +Cc: 4981 > 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). I agree that it's overengineering. This patch is only acceptable if (to compensate) it unifies the two duplicate code paths of move-to-window-line-top-bottom and recenter-top-bottom. Stefan ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#4981: C-l during query-replace 2009-11-30 14:13 ` Stefan Monnier @ 2009-11-30 16:07 ` Juri Linkov 2009-11-30 17:24 ` Drew Adams 2009-11-30 20:55 ` Stefan Monnier 0 siblings, 2 replies; 12+ messages in thread From: Juri Linkov @ 2009-11-30 16:07 UTC (permalink / raw) To: Stefan Monnier; +Cc: 4981, Dan Nicolaescu >> 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). > > I agree that it's overengineering. I think what is overengineering is adding recenter-top-bottom in the first place. It imposes the arbitrary fixed cycling order on users with no hope to customize such fundamental feature as recentering. `recenter-positions' mitigates this problem in the true Emacs way as the *customizable* editor. Please also note that even a minor feature `next-error' allows a similar customization with `next-error-recenter'. Perhaps we should try to merge them, or at least provide a new option in `next-error-recenter' to use the first value of `recenter-positions' as the primary position the user prefers to put point after recentering. > This patch is only acceptable if (to compensate) it unifies the two > duplicate code paths of move-to-window-line-top-bottom and > recenter-top-bottom. Do you mean we should merge move-to-window-line-top-bottom and recenter-top-bottom into one function? -- Juri Linkov http://www.jurta.org/emacs/ ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#4981: C-l during query-replace 2009-11-30 16:07 ` Juri Linkov @ 2009-11-30 17:24 ` Drew Adams 2009-11-30 20:55 ` Stefan Monnier 1 sibling, 0 replies; 12+ messages in thread From: Drew Adams @ 2009-11-30 17:24 UTC (permalink / raw) To: 'Juri Linkov', 4981, 'Stefan Monnier' Cc: 'Dan Nicolaescu' > >> 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). > > > > I agree that it's overengineering. > > I think what is overengineering is adding recenter-top-bottom > in the first place. It imposes the arbitrary fixed cycling order > on users with no hope to customize such fundamental feature as > recentering. `recenter-positions' mitigates this problem in the true > Emacs way as the *customizable* editor. As the one originally responsible for `recenter-top-bottom', let me chime in. ;-) 1. Just as with `recenter', a `recenter-top-bottom' user can always provide a prefix arg to get the exact behavior wanted. It imposes nothing more than `recenter' imposed. 2. The fact that we seem to be extending the use of this to other areas indicates that it has proved to be an improvement wrt `recenter'. 3. I have no objection to user's being able, via an option, to add more cycle points and define their positions. That's not overengineering, IMO. I think the default should be what `recenter-top-bottom' defined: 3 cycle points, top, center, bottom. - Drew ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#4981: C-l during query-replace 2009-11-30 16:07 ` Juri Linkov 2009-11-30 17:24 ` Drew Adams @ 2009-11-30 20:55 ` Stefan Monnier 1 sibling, 0 replies; 12+ messages in thread From: Stefan Monnier @ 2009-11-30 20:55 UTC (permalink / raw) To: Juri Linkov; +Cc: 4981, Dan Nicolaescu > Do you mean we should merge move-to-window-line-top-bottom and > recenter-top-bottom into one function? No, but the core of both (which is identical) should be moved to a third function, used by the two commands. So as to remove the code duplication. Stefan ^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#4981: marked as done (C-l during query-replace) 2009-11-20 0:16 ` bug#4981: C-l during query-replace Dan Nicolaescu 2009-11-20 9:29 ` Juri Linkov @ 2009-11-30 16:30 ` Emacs bug Tracking System 1 sibling, 0 replies; 12+ messages in thread From: Emacs bug Tracking System @ 2009-11-30 16:30 UTC (permalink / raw) To: Juri Linkov [-- Attachment #1: Type: text/plain, Size: 845 bytes --] Your message dated Mon, 30 Nov 2009 18:06:33 +0200 with message-id <87skbwt9xq.fsf@mail.jurta.org> and subject line Re: bug#4981: C-l during query-replace has caused the Emacs bug report #4981, regarding C-l during query-replace to be marked as done. This means that you claim that the problem has been dealt with. If this is not the case it is now your responsibility to reopen the bug report if necessary, and/or fix the problem forthwith. (NB: If you are a system administrator and have no idea what this message is talking about, this may indicate a serious mail system misconfiguration somewhere. Please contact owner@emacsbugs.donarmstrong.com immediately.) -- 4981: http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=4981 Emacs Bug Tracking System Contact owner@emacsbugs.donarmstrong.com with problems [-- Attachment #2: Type: message/rfc822, Size: 2707 bytes --] From: Dan Nicolaescu <dann@ics.uci.edu> To: bug-gnu-emacs <bug-gnu-emacs@gnu.org> Subject: C-l during query-replace Date: Thu, 19 Nov 2009 16:16:59 -0800 (PST) Message-ID: <200911200016.nAK0Gxf5005670@godzilla.ics.uci.edu> C-l during query-replace should run `recenter-top-bottom', not `recenter' for consistency with what C-l normally does nowadays. [-- Attachment #3: Type: message/rfc822, Size: 2400 bytes --] From: Juri Linkov <juri@jurta.org> To: Stefan Monnier <monnier@iro.umontreal.ca> Cc: 4981-done@emacsbugs.donarmstrong.com, Dan Nicolaescu <dann@ics.uci.edu> Subject: Re: bug#4981: C-l during query-replace Date: Mon, 30 Nov 2009 18:06:33 +0200 Message-ID: <87skbwt9xq.fsf@mail.jurta.org> > Why not: > >> 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)) >> + (let ((this-command 'recenter-top-bottom)) >> + (recenter-top-bottom))) >> ((eq def 'edit) >> (let ((opos (point-marker))) >> (setq real-match-data (replace-match-data Thanks, this helped. Fixed with more changes to keep the cycling order. -- Juri Linkov http://www.jurta.org/emacs/ ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2009-11-30 20:55 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- [not found] <87skbwt9xq.fsf@mail.jurta.org> 2009-11-20 0:16 ` bug#4981: C-l during query-replace Dan Nicolaescu 2009-11-20 9:29 ` Juri Linkov 2009-11-29 23:44 ` Juri Linkov 2009-11-30 1:28 ` Stefan Monnier 2009-11-30 6:29 ` Dan Nicolaescu 2009-11-30 11:12 ` Juanma Barranquero 2009-11-30 12:04 ` Juri Linkov 2009-11-30 14:13 ` Stefan Monnier 2009-11-30 16:07 ` Juri Linkov 2009-11-30 17:24 ` Drew Adams 2009-11-30 20:55 ` Stefan Monnier 2009-11-30 16:30 ` bug#4981: marked as done (C-l during query-replace) Emacs bug Tracking System
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.