GNU bug report logs -
#56393
Actually fix the long lines display bug
Previous Next
Full log
View this message in rfc822 format
> Date: Mon, 18 Jul 2022 12:58:59 +0000
> From: Gregory Heytings <gregory <at> heytings.org>
> cc: Gerd Möllmann <gerd.moellmann <at> gmail.com>, larsi <at> gnus.org,
> 56393 <at> debbugs.gnu.org
>
> > This is isearch.el trying to establish whether the position of the match
> > is visible in the window. To do that, it starts from the current
> > window-start position (which happens to be 1), and simulates display of
> > the buffer text portion up until point or until the end of window,
> > whichever comes first, to determine whether point is beyond the end of
> > the window. Your change makes the search start from some much later
> > position instead, which could very well produce incorrect results:
> > pos-visible-in-window-p could decide that point _is_ visible, when it
> > really isn't. (It doesn't happen in this particular case because the
> > newline is far away -- at the very end of the buffer -- so it isn't
> > visible in both cases. But that's sheer luck.)
>
> Well, this happens only in buffers with long lines, and only when we are
> inside a long line
Is the last part really guaranteed? AFAIU, the detection of long
lines scans the entire buffer, so if there's a long line _anywhere_ in
the buffer, the narrowing is applied, even if point is in no-so-long
lines. Or am I missing something?
> so from my point of view it works as expected
"As expected" in what sense? Suppose we really are in a long line,
the Isearch match is really outside the window, but if we use
point-10000 as BEGV point seems to be _inside_ the window -- in this
case the feature implemented in isearch-update for slow terminals will
not do its thing, right?
> and moreover the risk is small.
Not sure this should pacify the fears.
> Would the following be better from your point of view?
>
> diff --git a/src/xdisp.c b/src/xdisp.c
> index d69d7440bc..572ad2b854 100644
> --- a/src/xdisp.c
> +++ b/src/xdisp.c
> @@ -8668,18 +8668,11 @@ get_visually_first_element (struct it *it)
> bool string_p = STRINGP (it->string) || it->s;
> ptrdiff_t eob = (string_p ? it->bidi_it.string.schars : ZV);
> ptrdiff_t bob;
> + ptrdiff_t obegv = BEGV;
>
> - SET_WITH_NARROWED_BEGV (it, bob, string_p ? 0 : BEGV);
> -
> - /* Reseat again when, as a consequence of the SET_WITH_NARROWED_BEGV
> - above, the iterator is before bob. */
> - if (!string_p && IT_CHARPOS (*it) < bob)
> - {
> - struct text_pos pos;
> - pos.charpos = bob;
> - pos.bytepos = CHAR_TO_BYTE (bob);
> - reseat (it, pos, true);
> - }
> + SET_WITH_NARROWED_BEGV (it, bob,
> + string_p ? 0 :
> + IT_BYTEPOS (*it) < BEGV ? obegv : BEGV);
I guess it's better, as it reduces the number of cases where the
problem could happen, at the price of making those cases slower.
> > More generally, if you look at redisplay_window, you will see that about
> > 2/3 of its code tries very hard to reuse the previous window-start
> > position, before it gives up and looks for a new starting position. So
> > in any situation where the previous window-start is far enough before
> > point, all that code will basically work with a buffer position that is
> > at risk of being before the "narrowed" BEGV. Thus, any code there which
> > tries stuff like start_display+move_it_to will risk hitting this kind of
> > problems -- either FETCH_BYTE will crash, or we risk producing the wrong
> > result because we force the code to jump to the "narrowed" BEGV before
> > doing anything, while its caller expects results relative to a different
> > position.
>
> I understand. But note that temporarily narrowing the buffer happens only
> at a few well-chosen places, which are situated rather low in the
> abstraction layers, so the effect on other parts of the code is nil.
I think I agree with everything except the "nil" part ;-)
> > I think this is because the display engine assumes that BEGV stays put
> > during the entire redisplay_window lifetime, i.e. that all of the
> > subroutines it calls see the same value of BEGV. This is no longer so
> > on the branch, and I wonder whether and how we should handle this new
> > situation to keep the display code stable and reliable.
> >
>
> I don't know.
Neither do I. Still thinking about it. I'd be interested to hear
Gerd's thoughts on this.
This bug report was last modified 3 years and 33 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.