* bug#51776: master 9741924: In insert_file_contents, always set windows' point markers. [not found] ` <20211112184452.6C899209C6@vcs0.savannah.gnu.org> @ 2021-11-13 0:27 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors 2021-11-13 12:29 ` Alan Mackenzie 0 siblings, 1 reply; 2+ messages in thread From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-11-13 0:27 UTC (permalink / raw) To: Alan Mackenzie; +Cc: 51776 Hi Alan, Alan Mackenzie [2021-11-12 13:44:52] wrote: > In insert_file_contents, always set windows' point markers. > This fixes bug #51776. > > * src/fileio.c (restore_window_points): Restore a w->mpoint even > when that marker originally pointed into the unchanged area near > BOB or EOB. This prevents that window's point being moved a long > way from its starting place due to the removal of the central part > of the buffer by insert_file_contents. Hmm... my understanding of the code says that if `oldppos <= same_at_beg` then your change is harmless but unnecessary because the marker is still at `oldpos` anyway. But when `oldppos > same_at_beg` your change is harmful because the marker has properly preserved its exact position in the text whereas your code will arbitrarily move it to `oldpos` (which is even potentially past Z if the revert shortened the buffer). So maybe the better fix is to just change XFIXNUM (oldpos) < same_at_end into XFIXNUM (oldpos) <= same_at_end Or am I missing something? [ Not sure what we should do when `oldpos == same_at_beg == same_at_end`, OTOH. I suspect staying at `same_at_beg` might be the better choice in that case. ] Stefan > src/fileio.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/src/fileio.c b/src/fileio.c > index 3c13d3f..a7b1649 100644 > --- a/src/fileio.c > +++ b/src/fileio.c > @@ -3827,6 +3827,7 @@ restore_window_points (Lisp_Object window_markers, ptrdiff_t inserted, > Lisp_Object car = XCAR (window_markers); > Lisp_Object marker = XCAR (car); > Lisp_Object oldpos = XCDR (car); > + ptrdiff_t newpos; > if (MARKERP (marker) && FIXNUMP (oldpos) > && XFIXNUM (oldpos) > same_at_start > && XFIXNUM (oldpos) < same_at_end) > @@ -3834,10 +3835,12 @@ restore_window_points (Lisp_Object window_markers, ptrdiff_t inserted, > ptrdiff_t oldsize = same_at_end - same_at_start; > ptrdiff_t newsize = inserted; > double growth = newsize / (double)oldsize; > - ptrdiff_t newpos > - = same_at_start + growth * (XFIXNUM (oldpos) - same_at_start); > - Fset_marker (marker, make_fixnum (newpos), Qnil); > + newpos = same_at_start > + + growth * (XFIXNUM (oldpos) - same_at_start); > } > + else > + newpos = XFIXNUM (oldpos); > + Fset_marker (marker, make_fixnum (newpos), Qnil); > } > } > ^ permalink raw reply [flat|nested] 2+ messages in thread
* bug#51776: master 9741924: In insert_file_contents, always set windows' point markers. 2021-11-13 0:27 ` bug#51776: master 9741924: In insert_file_contents, always set windows' point markers Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-11-13 12:29 ` Alan Mackenzie 0 siblings, 0 replies; 2+ messages in thread From: Alan Mackenzie @ 2021-11-13 12:29 UTC (permalink / raw) To: Stefan Monnier; +Cc: 51776 On Fri, Nov 12, 2021 at 19:27:29 -0500, Stefan Monnier wrote: > Hi Alan, > Alan Mackenzie [2021-11-12 13:44:52] wrote: > > In insert_file_contents, always set windows' point markers. > > This fixes bug #51776. > > * src/fileio.c (restore_window_points): Restore a w->mpoint even > > when that marker originally pointed into the unchanged area near > > BOB or EOB. This prevents that window's point being moved a long > > way from its starting place due to the removal of the central part > > of the buffer by insert_file_contents. > Hmm... my understanding of the code says that if `oldppos <= same_at_beg` then > your change is harmless but unnecessary because the marker > is still at `oldpos` anyway. > But when `oldppos > same_at_beg` your change is harmful because the > marker has properly preserved its exact position in the text whereas > your code will arbitrarily move it to `oldpos` (which is even > potentially past Z if the revert shortened the buffer). Er, yes, you're right. Thanks! > So maybe the better fix is to just change > XFIXNUM (oldpos) < same_at_end > into > XFIXNUM (oldpos) <= same_at_end Indeed. > Or am I missing something? > [ Not sure what we should do when `oldpos == same_at_beg == same_at_end`, OTOH. > I suspect staying at `same_at_beg` might be the better choice in that > case. ] That case is harmless, because it just means that after the reversion, point stays where it was (before the re-inserted middle) rather than where it was (after the re-inserted middle). I'll correct this patch. > Stefan > > src/fileio.c | 9 ++++++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/src/fileio.c b/src/fileio.c > > index 3c13d3f..a7b1649 100644 > > --- a/src/fileio.c > > +++ b/src/fileio.c > > @@ -3827,6 +3827,7 @@ restore_window_points (Lisp_Object window_markers, ptrdiff_t inserted, > > Lisp_Object car = XCAR (window_markers); > > Lisp_Object marker = XCAR (car); > > Lisp_Object oldpos = XCDR (car); > > + ptrdiff_t newpos; > > if (MARKERP (marker) && FIXNUMP (oldpos) > > && XFIXNUM (oldpos) > same_at_start > > && XFIXNUM (oldpos) < same_at_end) > > @@ -3834,10 +3835,12 @@ restore_window_points (Lisp_Object window_markers, ptrdiff_t inserted, > > ptrdiff_t oldsize = same_at_end - same_at_start; > > ptrdiff_t newsize = inserted; > > double growth = newsize / (double)oldsize; > > - ptrdiff_t newpos > > - = same_at_start + growth * (XFIXNUM (oldpos) - same_at_start); > > - Fset_marker (marker, make_fixnum (newpos), Qnil); > > + newpos = same_at_start > > + + growth * (XFIXNUM (oldpos) - same_at_start); > > } > > + else > > + newpos = XFIXNUM (oldpos); > > + Fset_marker (marker, make_fixnum (newpos), Qnil); > > } > > } > > -- Alan Mackenzie (Nuremberg, Germany). ^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-11-13 12:29 UTC | newest] Thread overview: 2+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- [not found] <20211112184451.21516.85178@vcs0.savannah.gnu.org> [not found] ` <20211112184452.6C899209C6@vcs0.savannah.gnu.org> 2021-11-13 0:27 ` bug#51776: master 9741924: In insert_file_contents, always set windows' point markers Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors 2021-11-13 12:29 ` Alan Mackenzie
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).