emacs-devel
[Top][All Lists]
Advanced

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

Re: Line wrap reconsidered


From: Eli Zaretskii
Subject: Re: Line wrap reconsidered
Date: Tue, 26 May 2020 17:54:11 +0300

> From: Yuan Fu <address@hidden>
> Date: Mon, 25 May 2020 19:32:33 -0400
> Cc: emacs-devel <address@hidden>
> 
> +static int char_can_wrap(struct it *it)
> +{
> +  Lisp_Object charpos = make_fixnum (IT_STRING_CHARPOS (*it));
> +  Lisp_Object tail = Fget_text_property (charpos, Qcan_wrap, Qnil);

Regardless of the bug you say you fixed in the meantime, the above is
wrong: it only considers the text property on buffer text, but not on
display strings and overlay strings.  Those strings can also need to
be wrapped on display.

> +  for (; CONSP (tail); tail = XCDR (tail))
> +    {
> +      register Lisp_Object tem = XCAR (tail);
> +      if (EQ (Qcan_wrap, tem))
> +        {
> +          Lisp_Object val = XCDR (tail);
> +          if (NILP (val))
> +            { return 0; }
> +          else if (EQ (Qbefore_only, val))
> +            { return 1; }
> +          else if (EQ (Qafter_only, val))
> +            { return 2; }
> +          else if (EQ (Qt, val))
> +            { return 3; }
> +          else
> +            { return 0; }
> +        }

Why is the value of the text property a cons cell?  Why not a simple
symbol?

> -           if (IT_DISPLAYING_WHITESPACE (it))
> +              /* If this character displays a whitespace or the text
> +                 property says you can wrap after it, the next one can
> +                 wrap.  */
> +           if (IT_DISPLAYING_WHITESPACE (it) || IT_CAN_WRAP_AFTER (it))

It is cleaner to define a new macro that handles both conditions, and
use it instead of IT_DISPLAYING_WHITESPACE.



reply via email to

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