GNU bug report logs - #71525
30.0.50; Spin in delete-region/interval_deletion_adjustment

Previous Next

Full log

Message #47 received at 71525 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: jporterbugs <at> gmail.com, steven <at> stebalien.com, 71525 <at> debbugs.gnu.org
Subject: Re: bug#71525: 30.0.50; Spin in
 delete-region/interval_deletion_adjustment Spin in
 delete-region/interval_deletion_adjustment)
Date: Fri, 14 Jun 2024 19:51:44 +0300

On 14/06/2024 10:13, Eli Zaretskii wrote:
> 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! I've pushed the amended patch to master, please see how you like 
the result.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.