GNU bug report logs -
#56682
Fix the long lines font locking related slowdowns
Previous Next
Full log
View this message in rfc822 format
Thanks for your detailed review.
>> + struct Lisp_Marker *begv
>> + = labeled_restrictions_get_bound (buf, true, true);
>> + struct Lisp_Marker *zv
>> + = labeled_restrictions_get_bound (buf, false, true);
>
> Why the strange design of having a function return a pointer to a
> 'struct Lisp_Marker'? why not return the marker itself instead? (I
> realize that this was so in the code we already have, but I still don't
> understand why you did it that way, and prefer that function to return a
> marker instead.)
>
Good question. You mean that it would have been better to return a
Lisp_Object, right? I don't recall exactly, I think it was because in the
calls to SET_BUF_BEGV_BOTH/SET_BUF_ZV_BOTH (which are the only places
where the return value of labeled_restrictions_get_bound are used) one can
use the pointer to a struct Lisp_Marker immediately, whereas a call to
XMARKER would have been necessary if a Lisp_Object had been used.
>> record_unwind_protect (save_restriction_restore, save_restriction_save ());
>> + labeled_restrictions_remove_in_current_buffer ();
>
> Why are we removing the restrictions as part of write-region?
>
We are removing them temporarily, just before the Fwiden, and they are
restored by save_restriction_restore.
>> record_unwind_protect (save_restriction_restore,
>> save_restriction_save ());
>> + labeled_restrictions_remove_in_current_buffer ();
>> Fwiden ();
>
> And why here?
>
For the same reason: calls to Fwiden which are preceded by a
"record_unwind_protect (save_restriction_restore, save_restriction_save
());" are meant to temporarily widen the buffer, and restore the
restrictions upon returning from the function. So we temporarily remove
labeled restrictions as well (and they are restored by
save_restriction_restore, too).
>> record_unwind_protect (save_restriction_restore, save_restriction_save ());
>> + labeled_restrictions_remove_in_current_buffer ();
>
> And here?
>
For the same reason again ;-)
>> record_unwind_protect (save_restriction_restore, save_restriction_save ());
>> + labeled_restrictions_remove_in_current_buffer ();
>> Fwiden ();
>> val = display_count_lines (start_byte, limit_byte, count, byte_pos_ptr);
>
> Why do we remove the restrictions here?
>
... and again ;-)
>> + The corresponding function 'get_medium_narrowing_zv' (and
>> + 'medium_narrowing_zv' field in 'struct it') is not used to set the
>> + end limit of a the restriction, which is again unnecessary, but to
> ^^^^^
> Typo.
>
Good catch, thanks!
>> +static ptrdiff_t
>> +get_nearby_bol_pos (ptrdiff_t pos)
>> +{
>> + ptrdiff_t start, pos_bytepos, cur, next, found, bol = BEGV - 1;
>> + int dist;
>> + for (dist = 500; dist <= 500000; dist *= 10)
>> + {
>> + pos_bytepos = pos == BEGV ? BEGV_BYTE : CHAR_TO_BYTE (pos);
>> + start = pos - dist < BEGV ? BEGV : pos - dist;
>> + for (cur = start; cur < pos; cur = next)
>> + {
>> + next = find_newline1 (cur, CHAR_TO_BYTE (cur),
>> + pos, pos_bytepos,
>> + 1, &found, NULL, false);
>> + if (found)
>> + bol = next;
>> + else
>> + break;
>> + }
>> + if (bol >= BEGV || start == BEGV)
>> + return bol;
>> + else
>> + pos = pos - dist < BEGV ? BEGV : pos - dist;
>> + }
>> + return bol;
>> +}
>
> This function should have a commentary describing what it does.
>
Okay, I'll add that.
>
> Is it okay for this function to return a position > POS, its input?
>
Unless I misunderstood something, it cannot, because find_newline1 is
called with end = pos and end_byte = pos_bytepos.
This bug report was last modified 2 years and 8 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.