Re: feature/fix-the-long-lines-display-bug 0699f80f85 1/2: Fix calculation of the vertical scroll bar's thumb

Re: feature/fix-the-long-lines-display-bug 0699f80f85 1/2: Fix calculation of the vertical scroll bar's thumb

[Po Lu]

Gregory Heytings <gregory@heytings.org> writes:

> branch: feature/fix-the-long-lines-display-bug
> commit 0699f80f851b1f9e2f7b0a22ddd688abb7bebde1
> Author: Eli Zaretskii <eliz@gnu.org>
> Commit: Gregory Heytings <gregory@heytings.org>
>
>     Fix calculation of the vertical scroll bar's thumb
>     
>     * src/xdisp.c (set_vertical_scroll_bar): Compute window's end
>     position "by hand" if w->window_end_pos cannot be relied upon.
> ---
>  src/xdisp.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/src/xdisp.c b/src/xdisp.c
> index 375158a520..ae6553d876 100644
> --- a/src/xdisp.c
> +++ b/src/xdisp.c
> @@ -18924,11 +18924,27 @@ set_vertical_scroll_bar (struct window *w)
>  	  && NILP (echo_area_buffer[0])))
>      {
>        struct buffer *buf = XBUFFER (w->contents);
> +      ptrdiff_t window_end_pos = w->window_end_pos;
> +
> +      /* If w->window_end_pos cannot be trusted, recompute it "the
> +	 hard way".  */
> +      if (!w->window_end_valid)
> +	{
> +	  struct it it;
> +	  struct text_pos start_pos;
> +
> +	  SET_TEXT_POS_FROM_MARKER (start_pos, w->start);
> +	  start_display (&it, w, start_pos);
> +	  move_it_to (&it, -1, it.last_visible_x, window_box_height (w), -1,
> +		      MOVE_TO_X | MOVE_TO_Y);
> +	  window_end_pos = BUF_Z (buf) - IT_CHARPOS (it);
> +	}
> +
>        whole = BUF_ZV (buf) - BUF_BEGV (buf);
>        start = marker_position (w->start) - BUF_BEGV (buf);
>        /* I don't think this is guaranteed to be right.  For the
>  	 moment, we'll pretend it is.  */
> -      end = BUF_Z (buf) - w->window_end_pos - BUF_BEGV (buf);
> +      end = BUF_Z (buf) - window_end_pos - BUF_BEGV (buf);
>  
>        if (end < start)
>  	end = start;

When making changes of this kind, you should test without toolkit scroll
bars, and with at least both the Motif (with
scroll-bar-adjust-thumb-position both on and off) and Xaw scroll bars to
make sure they still work correctly.  Testing on NS also helps, since
the scroll bar logic there is somewhat different.

The last time I tried to significantly change the scroll bar logic (to
operate on more realistic positions within a window), those specific
configurations bit me hard.

Re: feature/fix-the-long-lines-display-bug 0699f80f85 1/2: Fix calculation of the vertical scroll bar's thumb

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: Eli Zaretskii <eliz@gnu.org>
> Date: Tue, 19 Jul 2022 08:59:05 +0800
> 
> Gregory Heytings <gregory@heytings.org> writes:
> 
> > branch: feature/fix-the-long-lines-display-bug
> > commit 0699f80f851b1f9e2f7b0a22ddd688abb7bebde1
> > Author: Eli Zaretskii <eliz@gnu.org>
> > Commit: Gregory Heytings <gregory@heytings.org>
> >
> >     Fix calculation of the vertical scroll bar's thumb
> >     
> >     * src/xdisp.c (set_vertical_scroll_bar): Compute window's end
> >     position "by hand" if w->window_end_pos cannot be relied upon.
> > ---
> >  src/xdisp.c | 18 +++++++++++++++++-
> >  1 file changed, 17 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/xdisp.c b/src/xdisp.c
> > index 375158a520..ae6553d876 100644
> > --- a/src/xdisp.c
> > +++ b/src/xdisp.c
> > @@ -18924,11 +18924,27 @@ set_vertical_scroll_bar (struct window *w)
> >  	  && NILP (echo_area_buffer[0])))
> >      {
> >        struct buffer *buf = XBUFFER (w->contents);
> > +      ptrdiff_t window_end_pos = w->window_end_pos;
> > +
> > +      /* If w->window_end_pos cannot be trusted, recompute it "the
> > +	 hard way".  */
> > +      if (!w->window_end_valid)
> > +	{
> > +	  struct it it;
> > +	  struct text_pos start_pos;
> > +
> > +	  SET_TEXT_POS_FROM_MARKER (start_pos, w->start);
> > +	  start_display (&it, w, start_pos);
> > +	  move_it_to (&it, -1, it.last_visible_x, window_box_height (w), -1,
> > +		      MOVE_TO_X | MOVE_TO_Y);
> > +	  window_end_pos = BUF_Z (buf) - IT_CHARPOS (it);
> > +	}
> > +
> >        whole = BUF_ZV (buf) - BUF_BEGV (buf);
> >        start = marker_position (w->start) - BUF_BEGV (buf);
> >        /* I don't think this is guaranteed to be right.  For the
> >  	 moment, we'll pretend it is.  */
> > -      end = BUF_Z (buf) - w->window_end_pos - BUF_BEGV (buf);
> > +      end = BUF_Z (buf) - window_end_pos - BUF_BEGV (buf);
> >  
> >        if (end < start)
> >  	end = start;
> 
> When making changes of this kind, you should test without toolkit scroll
> bars, and with at least both the Motif (with
> scroll-bar-adjust-thumb-position both on and off) and Xaw scroll bars to
> make sure they still work correctly.  Testing on NS also helps, since
> the scroll bar logic there is somewhat different.
> 
> The last time I tried to significantly change the scroll bar logic (to
> operate on more realistic positions within a window), those specific
> configurations bit me hard.

I don't have any access to such systems, sorry.  If you see specific
problems with that change, please tell.  Otherwise, we will have to
rely on people reporting bugs.

Re: feature/fix-the-long-lines-display-bug 0699f80f85 1/2: Fix calculation of the vertical scroll bar's thumb

[Po Lu]

Eli Zaretskii <eliz@gnu.org> writes:

> I don't have any access to such systems, sorry.  If you see specific
> problems with that change, please tell.  Otherwise, we will have to
> rely on people reporting bugs.

Sure.  But this advice was aimed at Greogry, who seems to have access to
them.

Re: feature/fix-the-long-lines-display-bug 0699f80f85 1/2: Fix calculation of the vertical scroll bar's thumb

[Gregory Heytings]

>> I don't have any access to such systems, sorry.  If you see specific 
>> problems with that change, please tell.  Otherwise, we will have to 
>> rely on people reporting bugs.
>
> Sure.  But this advice was aimed at Greogry, who seems to have access to 
> them.
>

You may have seen that the commit author is Eli, I merely cherry-picked 
it.

Re: feature/fix-the-long-lines-display-bug 0699f80f85 1/2: Fix calculation of the vertical scroll bar's thumb

[Po Lu]

Gregory Heytings <gregory@heytings.org> writes:

> You may have seen that the commit author is Eli, I merely
> cherry-picked it.

Oh, okay.  I must've missed that.

Sorry for the noise.

Re: feature/fix-the-long-lines-display-bug 0699f80f85 1/2: Fix calculation of the vertical scroll bar's thumb

[Eli Zaretskii]

> Date: Tue, 19 Jul 2022 05:11:56 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org
> 
> 
> >> I don't have any access to such systems, sorry.  If you see specific 
> >> problems with that change, please tell.  Otherwise, we will have to 
> >> rely on people reporting bugs.
> >
> > Sure.  But this advice was aimed at Greogry, who seems to have access to 
> > them.
> >
> 
> You may have seen that the commit author is Eli, I merely cherry-picked 
> it.

Cherry-picked in what way?  I may be missing something, but that
change is not on master yet, it is only on the
fix-the-long-line-display-bug branch, where I committed it.  It will
be on master when we merge the branch.

Re: feature/fix-the-long-lines-display-bug 0699f80f85 1/2: Fix calculation of the vertical scroll bar's thumb

[Gregory Heytings]

>> You may have seen that the commit author is Eli, I merely cherry-picked 
>> it.
>
> Cherry-picked in what way?  I may be missing something, but that change 
> is not on master yet, it is only on the fix-the-long-line-display-bug 
> branch, where I committed it.  It will be on master when we merge the 
> branch.
>

Cherry-picked from the feature branch to the feature branch, because it 
had been reverted when you asked me to redesign the feature from scratch.

Re: feature/fix-the-long-lines-display-bug 0699f80f85 1/2: Fix calculation of the vertical scroll bar's thumb

[Eli Zaretskii]

> Date: Tue, 19 Jul 2022 12:53:25 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: luangruo@yahoo.com, emacs-devel@gnu.org
> 
> 
> >> You may have seen that the commit author is Eli, I merely cherry-picked 
> >> it.
> >
> > Cherry-picked in what way?  I may be missing something, but that change 
> > is not on master yet, it is only on the fix-the-long-line-display-bug 
> > branch, where I committed it.  It will be on master when we merge the 
> > branch.
> >
> 
> Cherry-picked from the feature branch to the feature branch, because it 
> had been reverted when you asked me to redesign the feature from scratch.

Ah, okay.  I didn't know it was reverted.