From: Eli Zaretskii <eliz@gnu.org>
To: Dmitry Gutov <dmitry@gutov.dev>
Cc: jporterbugs@gmail.com, steven@stebalien.com, 71525@debbugs.gnu.org
Subject: bug#71525: 30.0.50; Spin in delete-region/interval_deletion_adjustment Spin in delete-region/interval_deletion_adjustment)
Date: Fri, 14 Jun 2024 10:13:30 +0300 [thread overview]
Message-ID: <86v82cxahx.fsf@gnu.org> (raw)
In-Reply-To: <f8c96939-5a91-418f-83ad-0d7de40b115f@gutov.dev> (message from Dmitry Gutov on Fri, 14 Jun 2024 02:41:15 +0300)
> Date: Fri, 14 Jun 2024 02:41:15 +0300
> From: Dmitry Gutov <dmitry@gutov.dev>
> Cc: jporterbugs@gmail.com, 71525@debbugs.gnu.org
>
> On 14/06/2024 00:47, Dmitry Gutov wrote:
> > The thing is, decode_coding_c_string already calls
> > adjust_markers_for_insert through
> >
> > decode_coding_object->decode_coding->produce_chars->insert_from_gap
> >
> > And the extra call moves the markers too far.
> >
> > Unfortunately, it's called with BEFORE_MARKERS=nil, and the above call
> > chain makes it difficult to pass through the extra argument.
>
> We can do it through the coding_system structure, though.
>
> See attached. It fixes the freezes in my testing.
>
> It seems like the least invasive possible fix, but better suggestions
> welcome.
This is the correct approach, IMO. But see some minor comments below.
> @@ -7814,7 +7815,7 @@ encode_coding (struct coding_system *coding)
> } while (coding->consumed_char < coding->src_chars);
>
> if (BUFFERP (coding->dst_object) && coding->produced_char > 0)
> - insert_from_gap (coding->produced_char, coding->produced, 0);
> + insert_from_gap (coding->produced_char, coding->produced, 0, coding->dst_before_markers);
Here (and elsewhere in the patch) too-long lines should be broken in
two.
> + /* True to insert before markers in the DST_OBJECT buffer. */
> + bool_bf dst_before_markers : 1;
I'd call this 'insert_before_markers' instead. Please also add a
detailed comment here explaining what this flag is for and where and
why used.
> /* Insert a sequence of NCHARS chars which occupy NBYTES bytes
> starting at GAP_END_ADDR - NBYTES (if text_at_gap_tail) and at
> - GPT_ADDR (if not text_at_gap_tail). */
> + GPT_ADDR (if not text_at_gap_tail).
> +
> + If BEFORE_MARKERS is true, insert before markers. */
This commentary should also mention process.c as the single caller
using this facility in unconventional ways.
> extern void insert_from_gap_1 (ptrdiff_t, ptrdiff_t, bool text_at_gap_tail);
> -extern void insert_from_gap (ptrdiff_t, ptrdiff_t, bool text_at_gap_tail);
> +extern void insert_from_gap (ptrdiff_t, ptrdiff_t, bool text_at_gap_tail,
> + bool before_markers);
We don't have names of arguments in prototypes, only their types.
(Yes, it means the original prototype, and the ones around it, were
also wrong.)
> --- a/src/process.c
> +++ b/src/process.c
> @@ -6415,6 +6415,7 @@ read_and_insert_process_output (struct Lisp_Process *p, char *buf,
> specpdl_ref count1 = SPECPDL_INDEX ();
>
> XSETBUFFER (curbuf, current_buffer);
> + process_coding->dst_before_markers = true;
Please also add a comment here explaining why this is done.
Thanks.
next prev parent reply other threads:[~2024-06-14 7:13 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-12 19:20 bug#71525: 30.0.50; Spin in delete-region/interval_deletion_adjustment Steven Allen via Bug reports for GNU Emacs, the Swiss army knife of text editors
[not found] ` <handler.71525.B.171822007123849.ack@debbugs.gnu.org>
2024-06-13 5:14 ` bug#71525: Acknowledgement (30.0.50; Spin in delete-region/interval_deletion_adjustment) Steven Allen via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-06-13 5:52 ` bug#71525: 30.0.50; Spin in delete-region/interval_deletion_adjustment " Eli Zaretskii
2024-06-13 15:32 ` Steven Allen via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-06-13 15:52 ` Eli Zaretskii
2024-06-13 16:00 ` Steven Allen via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-06-13 16:06 ` Steven Allen via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-06-13 16:31 ` Eli Zaretskii
2024-06-13 21:47 ` Dmitry Gutov
2024-06-13 23:41 ` Dmitry Gutov
2024-06-14 7:13 ` Eli Zaretskii [this message]
2024-06-14 16:51 ` Dmitry Gutov
2024-06-14 18:14 ` Steven Allen via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-06-14 18:17 ` Dmitry Gutov
2024-06-14 18:46 ` Eli Zaretskii
2024-06-14 19:13 ` Dmitry Gutov
2024-06-13 19:18 ` Jim Porter
2024-06-13 19:42 ` Steven Allen via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-06-13 5:49 ` bug#71525: 30.0.50; Spin in delete-region/interval_deletion_adjustment Eli Zaretskii
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
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=86v82cxahx.fsf@gnu.org \
--to=eliz@gnu.org \
--cc=71525@debbugs.gnu.org \
--cc=dmitry@gutov.dev \
--cc=jporterbugs@gmail.com \
--cc=steven@stebalien.com \
/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 public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).