emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: move_it_vertically_backward question


From: Po Lu
Subject: Re: move_it_vertically_backward question
Date: Sat, 18 Dec 2021 18:49:43 +0800
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.60 (gnu/linux)

Eli Zaretskii <eliz@gnu.org> writes:

> I think it would be a lot easier to understand if we use the
> description I proposed.  Lisp programmers do not necessarily know what
> are ascent and descent, and what they have to do with this function's
> operation.

Thanks, I renamed it to `ignore-line-at-end'.

> Please for the next iteration show them as a single patch.  One of
> them seems to depend on the other, and it's hard to review the changes
> piecemeal.

Agreed.  To make things easier, let's work on the `no-ascents-descents'
argument first, and only afterwards on the extension to TO.

> However, I don't think I see where that additional argument causes the
> function to stop at the visual left edge of a screen line containing
> TO.  What did I miss?

My understanding is that if we don't add the current ascent and descent
of the iterator to the returned height, the effect is the same.

> Don't give up trying!  No one is born with these abilities, they are
> acquired by writing documentation and learning from that.

Thanks.

>> @@ -10969,8 +10969,19 @@ window_text_pixel_size (Lisp_Object window, 
>> Lisp_Object from, Lisp_Object to, Li
>>        if (IT_CHARPOS (it) == end)
>>      {
>>        x += it.pixel_width;
>> -      it.max_ascent = max (it.max_ascent, it.ascent);
>> -      it.max_descent = max (it.max_descent, it.descent);
>> +
>> +      /* DTRT if no_ascents_descents is t.  */
>> +      if (no_ascents_descents)
>> +        {
>> +          saw_display_prop_at_end_p = true;

> This bool value seems to be a misnomer: we don't really have evidence
> that we found a display property at end of text.

How about `calculating_overshoot'?

> Also, you should use NILP in the if-clause, as no_ascents_descents is
> a Lisp object.

Thanks, good catch.

> This is quite confusing: you actually add the same value, but in two
> different ways and with 2 different (but subtly equivalent)
> conditions.  It would be good to make this less confusing.

Thanks, I tried to do that.  How's the attached patch?

>> @@ -11053,26 +11085,32 @@ window_text_pixel_size (Lisp_Object window, 
>> Lisp_Object from, Lisp_Object to, Li
>>  
>>    bidi_unshelve_cache (itdata, false);
>>  
>> -  return Fcons (make_fixnum (x - start_x), make_fixnum (y));
>> +  return (!movement
>> +      ? Fcons (make_fixnum (x - start_x), make_fixnum (y))
>> +      : list3i (x - start_x, y, start));
>
> Why not use list2i instead of Fcons there?

`window-text-pixel-size' returns by default the measurements as a pair,
not a list.

> Do these changes solve the performance problem in
> pixel-scroll-precision-mode?

Yes, they do.

Thanks.

Attachment: 0001-Add-new-argument-ignore-line-at-end-to-window-text-p.patch
Description: Text Data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]