* 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).