GNU bug report logs - #76313
New function `replace-region`

Previous Next

Full log

View this message in rfc822 format

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: phst <at> google.com, 76313 <at> debbugs.gnu.org, stefankangas <at> gmail.com, tsdh <at> gnu.org
Subject: bug#76313: New function `replace-region`
Date: Sat, 29 Mar 2025 17:58:55 -0400

>> +(defun replace-buffer-contents (source &optional max-secs max-costs)
>> +  "Replace accessible portion of current buffer with that of SOURCE.
>> +SOURCE can be a buffer or a string that names a buffer.
>> +Interactively, prompt for SOURCE.
>> +
>> +The replacement is performed using `replace-region-contents'
>> +which also describes the MAX-SECS and MAX-COSTS arguments and the
>> +return value."
>> +  (declare (obsolete replace-region-contents "31.1"))
>> +  (interactive "bSource buffer: ")
>> +  (replace-region-contents (point-min) (point-max)
>> +                           (if (stringp source) (get-buffer source) source)
>> +                           max-secs max-costs))
>
> Since this is an interactive function, shouldn't it test SOURCE to be
> of a valid type, instead of delegating to replace-region-contents?

I was being sloppy because it's obsolete anyway, but actually it's
simpler to always pass the arg to `get-buffer`, so you're quite right!

>> +Note: If the replacement is a string, it’ll usually be placed internally
>> +in a temporary buffer.  Therefore, if you already have the replacement
>> +in a buffer, it makes no sense to convert it to a string using
>> +‘buffer-substring’ or similar.
>
> This uses Unicode quotes in a doc string; please use ASCII characters
> instead.

Hmm... copy/paste from *Help*.  I think I'm going to turn off those
fancy quotes because it's not the first it bites me.

>> +      b = XBUFFER (AREF (source, 0));
>> +      min_b = XFIXNUM (AREF (source, 1));
>> +      size_b = XFIXNUM (insend) - min_b;
>> +      if (size_b < 0)
>> +	error ("Negative sized source range");
>
> Shouldn't we support here any order of SBEG and SEND, as we do in many
> other places?

Hmm... I must admit I'm not a big fan of this "feature", but OK,
I changed the code to use `validate_region` as elsewhere.

>> +      if (! (BUF_BEGV (b) <= min_b && min_b + size_b <= BUF_ZV (b)))
>> +	args_out_of_range_3 (AREF (source, 0), AREF (source, 0), AREF (source, 1));
>
> Should this error message show all 3 values, instead of showing one of
> them twice?  Or maybe show BEGV, ZV, and the offending position
> (either SBEG or SEND)?

Fixed, thanks.

>> +  bool b_empty = size_b == 0;
>> +  if (b && !BUFFER_LIVE_P (b))
>>      error ("Selecting deleted buffer");
>
> What if b is NULL? can we continue?

Yes: `b` is NULL here iff source is a string, in which case it's set
later to a locally-created temp buffer.

>> +  /* The rest of the code is not prepared to handle a string SOURCE.  */
>> +  if (!b)
>> +    {
>> +      Lisp_Object name
>> +	= Fgenerate_new_buffer_name (build_string (" *replace-workbuf*"), Qnil);
>> +      Lisp_Object workbuf = Fget_buffer_create (name, Qt);
>> +      b = XBUFFER (workbuf);
>> +      record_unwind_protect (kill_buffer, workbuf);
>> +      record_unwind_current_buffer ();
>> +      set_buffer_internal (b);
>> +      Fset_buffer_multibyte (STRING_MULTIBYTE (source) ? Qt : Qnil);
>
> coding.c:code_conversion_save does something similar; perhaps some of
> the buffer-local settings we use there could also be a good idea here?

Even better: I changed the code to call `code_conversion_save`!

> Please update the commentary to replace_range to describe this type of NEW.

Done, thanks, and pushed.


        Stefan

Previous Next

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