emacs-devel
[Top][All Lists]
Advanced

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

Re: Line wrap reconsidered


From: Yuan Fu
Subject: Re: Line wrap reconsidered
Date: Sun, 19 Jul 2020 12:16:32 -0400


> On Jul 19, 2020, at 10:52 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
>> From: Yuan Fu <casouri@gmail.com>
>> Date: Sat, 18 Jul 2020 13:14:15 -0400
>> Cc: Lars Ingebrigtsen <larsi@gnus.org>,
>> emacs-devel@gnu.org
>> 
>>> A minor stylistic nit: I'd prefer the if - elseif clauses to yield the
>>> relevant character, and then apply CHAR_HAS_CATEGORY only once to that
>>> character at the end.  (It is generally better to have only one return
>>> point from a function, especially when the function is short.  If
>>> nothing else, it makes debugging easier.)
>> 
>> I changed the it, do you code below this is ok?
>> 
>>  if (ch == 0)
>>    return false;
>>  else
>>    return CHAR_HAS_CATEGORY(ch, cat);
> 
> Yes.  Or any of the variants shown by Stefan.

Cool.

> 
>>>>      if (it->line_wrap == WORD_WRAP && it->area == TEXT_AREA)
>>>>        {
>>>> -        if (IT_DISPLAYING_WHITESPACE (it))
>>>> -          may_wrap = true;
>>>> -        else if (may_wrap)
>>>> +              /* Can we wrap here? */
>>>> +        if (may_wrap && char_can_wrap_before (it))
>>> 
>>> Likewise here.
>> 
>> 
>> In both can_wrap_before and can_wrap_after, I have a short circuit for the 
>> case when cjk_word_wrap is nil:
>> 
>>  if (!Vcjk_word_wrap)
>>    return IT_DISPLAYING_WHITESPACE (it);
>> 
>> That should guarantee the old behavior when cjk_word_wrap is nil, if that’s 
>> what you are asking about.
> 
> I've seen that, but what bothers me is not this.  It's the fact that
> the old code didn't test may_wrap, whereas the new code does.
> 

I see. I changed the code a bit and added some explanation in the commit 
message. Hopefully that will convince you that the new logic is equivalent to 
the old one when cjk-word-wrap is nil.

>>>> +  DEFVAR_BOOL("cjk-word-wrap", Vcjk_word_wrap,
>>>> +    doc: /*  Non-nil means wrap after CJK chracters.
>>> 
>>> This is unclear.  Does it mean after _any_ CJK character, or just
>>> after some?  And if the latter, which ones?
>> 
>> I added more detail and hopefully they are clearer now.
> 
> Looks much better, thanks.


BTW, any ideas for alternatives for cjk-word-wrap? Maybe extended-word-wrap?

Yuan




reply via email to

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