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: Eli Zaretskii
Subject: Re: move_it_vertically_backward question
Date: Sat, 18 Dec 2021 13:03:40 +0200

> From: Po Lu <luangruo@yahoo.com>
> Cc: emacs-devel@gnu.org
> Date: Sat, 18 Dec 2021 18:49:43 +0800
> 
> >> +    /* 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'?

Why do you need a flag variable at all?  Cannot 'doff' serve as that
flag?  Or, if not, why not test the argument -- it's just one NILP
test, after all?

>    /* Subtract height of header-line and tab-line which was counted
>       automatically by start_display.  */
> -  y = it.current_y + it.max_ascent + it.max_descent
> -    - WINDOW_TAB_LINE_HEIGHT (w) - WINDOW_HEADER_LINE_HEIGHT (w);
> +  if (!NILP (ignore_line_at_end))
> +    y = (it.current_y - WINDOW_TAB_LINE_HEIGHT (w)
> +      - WINDOW_HEADER_LINE_HEIGHT (w));
> +  else
> +    y = (it.current_y + it.max_ascent + it.max_descent
> +      - WINDOW_TAB_LINE_HEIGHT (w) - WINDOW_HEADER_LINE_HEIGHT (w));
> +
> +  if (calculating_overshoot)
> +    y += doff;

This is almost the same as the original code.

The problem that I wanted to point out is that ignore_line_at_end and
calculating_overshoot are related: one determines the value of the
other.  And the value of 'doff' is very similar to it.max_ascent +
it.max_descent.  So this code left me thinking for too long what
exactly is going on here, and why?  At first glance it even looked
like you are adding 'doff' in both cases, just indirectly.

It is this confusing aspect that I wanted you to fix.



reply via email to

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