From: Gregory Heytings <gregory@heytings.org>
To: Eli Zaretskii <eliz@gnu.org>
Cc: 56682@debbugs.gnu.org, monnier@iro.umontreal.ca
Subject: bug#56682: feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing.
Date: Fri, 05 May 2023 21:29:16 +0000 [thread overview]
Message-ID: <c6e8867fed87ebbb6e87@heytings.org> (raw)
In-Reply-To: <834joqiyuq.fsf@gnu.org>
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.
next prev parent reply other threads:[~2023-05-05 21:29 UTC|newest]
Thread overview: 102+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <166939872890.18950.12581667269687468681@vcs2.savannah.gnu.org>
[not found] ` <20221125175209.51166C004B6@vcs2.savannah.gnu.org>
2022-12-30 16:22 ` feature/improved-locked-narrowing 9dee6df39c: Reworked locked narrowing Stefan Monnier
2022-12-30 16:38 ` bug#56682: " Gregory Heytings
2022-12-30 16:41 ` Gregory Heytings
2022-12-30 17:01 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-12-30 17:25 ` Gregory Heytings
2022-12-30 18:51 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-01-12 9:34 ` Eli Zaretskii
2023-01-14 21:38 ` Gregory Heytings
2023-01-26 7:29 ` Eli Zaretskii
2023-01-28 15:11 ` Gregory Heytings
2023-01-28 15:36 ` Eli Zaretskii
2023-01-30 9:00 ` Gregory Heytings
2023-01-30 13:07 ` Eli Zaretskii
2023-01-30 15:03 ` Gregory Heytings
2023-01-30 17:11 ` Eli Zaretskii
2023-01-30 17:24 ` Juri Linkov
2023-01-30 17:52 ` Eli Zaretskii
2023-01-30 17:56 ` Juri Linkov
2023-01-30 18:05 ` Eli Zaretskii
2023-01-30 18:56 ` Dmitry Gutov
2023-01-30 19:02 ` Eli Zaretskii
2023-01-30 21:07 ` Dmitry Gutov
2023-01-30 21:49 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-01-30 23:29 ` Dmitry Gutov
2023-01-31 12:14 ` Eli Zaretskii
2023-01-31 15:58 ` Dmitry Gutov
2023-01-31 15:17 ` Gregory Heytings
2023-01-31 16:03 ` Dmitry Gutov
2023-01-31 15:14 ` Gregory Heytings
2023-01-31 16:25 ` Dmitry Gutov
2023-01-31 21:46 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-01-31 22:25 ` Dmitry Gutov
2023-02-01 18:55 ` Eli Zaretskii
2023-02-01 20:46 ` dick
2023-02-01 22:42 ` Gregory Heytings
2023-02-02 6:43 ` Eli Zaretskii
2023-02-03 0:20 ` Gregory Heytings
2023-02-03 7:39 ` Eli Zaretskii
2023-02-03 22:12 ` Gregory Heytings
2023-02-04 6:32 ` Eli Zaretskii
2023-02-09 1:57 ` Gregory Heytings
2023-02-09 7:01 ` Eli Zaretskii
2023-02-09 10:33 ` Gregory Heytings
2023-02-09 14:26 ` Eli Zaretskii
2023-02-09 14:39 ` Gregory Heytings
2023-02-09 15:46 ` Eli Zaretskii
2023-02-09 16:11 ` Gregory Heytings
2023-02-09 17:02 ` Eli Zaretskii
2023-02-09 17:44 ` Juri Linkov
2023-02-09 20:47 ` Gregory Heytings
2023-02-09 22:46 ` Drew Adams
2023-02-09 23:06 ` Drew Adams
2023-02-13 18:11 ` Eli Zaretskii
2023-02-10 7:44 ` Eli Zaretskii
2023-02-10 23:05 ` Gregory Heytings
2023-02-09 17:31 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-02-09 17:43 ` Eli Zaretskii
2023-02-09 17:57 ` Juri Linkov
2023-02-10 16:46 ` Andrea Corallo
2023-02-11 7:18 ` Eli Zaretskii
2023-02-13 11:00 ` Gregory Heytings
2023-02-13 18:10 ` Eli Zaretskii
2023-02-14 10:30 ` Gregory Heytings
2023-02-14 14:37 ` Eli Zaretskii
2023-02-14 14:59 ` Gregory Heytings
2023-02-14 16:55 ` Eli Zaretskii
2023-02-14 22:50 ` Gregory Heytings
2023-02-15 12:36 ` Eli Zaretskii
2023-02-15 13:37 ` Gregory Heytings
2023-02-15 14:10 ` Eli Zaretskii
2023-02-15 14:37 ` Gregory Heytings
2023-02-18 23:12 ` Gregory Heytings
2023-02-19 6:29 ` Eli Zaretskii
[not found] ` <a9b3c867-aa6a-2979-a83-dd700e985c9@heytings.org>
2023-03-29 14:52 ` Gregory Heytings
2023-04-01 0:27 ` Gregory Heytings
2023-04-01 5:42 ` Eli Zaretskii
2023-04-01 9:04 ` Gregory Heytings
2023-04-01 11:11 ` Eli Zaretskii
2023-04-01 14:26 ` Gregory Heytings
2023-04-01 15:09 ` Eli Zaretskii
2023-04-01 15:41 ` Gregory Heytings
2023-04-01 16:21 ` Eli Zaretskii
2023-04-01 17:01 ` Gregory Heytings
2023-04-01 17:12 ` Eli Zaretskii
2023-04-01 21:56 ` Gregory Heytings
2023-04-02 5:16 ` Eli Zaretskii
2023-04-04 2:55 ` Richard Stallman
2023-04-04 10:50 ` Eli Zaretskii
[not found] ` <ccfcc63b8da74932424b@heytings.org>
2023-05-04 5:31 ` Eli Zaretskii
2023-05-04 15:45 ` Gregory Heytings
2023-05-05 15:26 ` Eli Zaretskii
2023-05-05 21:29 ` Gregory Heytings [this message]
2023-05-06 6:26 ` Eli Zaretskii
2023-05-09 21:48 ` Gregory Heytings
2023-05-10 14:00 ` Eli Zaretskii
2023-05-12 11:12 ` Eli Zaretskii
2023-05-12 12:50 ` Gregory Heytings
2023-05-12 22:18 ` Gregory Heytings
2023-05-13 6:41 ` Eli Zaretskii
2023-01-30 14:46 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-01-30 15:05 ` Gregory Heytings
2023-01-30 15:08 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c6e8867fed87ebbb6e87@heytings.org \
--to=gregory@heytings.org \
--cc=56682@debbugs.gnu.org \
--cc=eliz@gnu.org \
--cc=monnier@iro.umontreal.ca \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.