GNU bug report logs - #45607
27.1; compiled replace-string breaks repeat-complex-command

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 45607 in the body.
You can then email your comments to 45607 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

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

From: Allen Li <darkfeline <at> felesatra.moe>
To: bug-gnu-emacs <at> gnu.org
Subject: 27.1; compiled replace-string breaks repeat-complex-command
Date: Sat, 02 Jan 2021 01:04:30 -0800

Interactive commands that act on the region are handled specially such
that when repeated with `repeat-complex-command`, the repeated command
uses the current region rather than the region used for the previous
invocation of the command.

`replace-string` does not respect this; it uses the previous region when
repeated with `repeat-complex-command`.

Note that loading `replace-string` from source (rather than byte
compiled) fixes this problem.  So it's probably a problem with byte
compiled commands.

I swear I filed a bug for this a long time ago, and I can't remember if
it's a regression or it hasn't landed in a release yet.  I can't find
the original bug.

In GNU Emacs 27.1 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.22, cairo version 1.17.3)
 of 2020-08-28 built on juergen
Windowing system distributor 'The X.Org Foundation', version 11.0.12010000
System Description: Arch Linux

Configured features:
XPM JPEG TIFF GIF PNG RSVG CAIRO SOUND GPM DBUS GSETTINGS GLIB NOTIFY
INOTIFY ACL GNUTLS LIBXML2 FREETYPE HARFBUZZ M17N_FLT LIBOTF ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM MODULES THREADS LIBSYSTEMD JSON
PDUMPER LCMS2 GMP

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Allen Li <darkfeline <at> felesatra.moe>
Cc: 45607 <at> debbugs.gnu.org
Subject: Re: bug#45607: 27.1; compiled replace-string breaks
 repeat-complex-command
Date: Tue, 07 Jun 2022 14:38:56 +0200

Allen Li <darkfeline <at> felesatra.moe> writes:

> Interactive commands that act on the region are handled specially such
> that when repeated with `repeat-complex-command`, the repeated command
> uses the current region rather than the region used for the previous
> invocation of the command.
>
> `replace-string` does not respect this; it uses the previous region when
> repeated with `repeat-complex-command`.
>
> Note that loading `replace-string` from source (rather than byte
> compiled) fixes this problem.  So it's probably a problem with byte
> compiled commands.

(I'm going through old bug reports that unfortunately weren't resolved
at the time.)

I can reproduce this problem in Emacs 29?

In any case, it's because `replace-string' specifies the start/end
position in the `interactive' spec (as it should), so it lands in
`command-history', and `repeat-complex-command' just executes that.
Other commands, like `flush-lines', have pass in nil as start/end, and
then computes the start/end in the body of the function.

So this can be fixed by rewriting `replace-string' to do the same...
but surely there's a lot of commands out there that say:

  (interactive
[...]
     (list 
	   (if (use-region-p) (region-beginning))

And all of these would have the same problem.  (interactive "r") does
not, because in that case:

(defun foo (start end)
  (interactive "r")
  (message "%s %s" start end))

The following ends up there in the history:

 (foo (region-beginning) (region-end))

Does anybody know of a more general solution to this?

The reason replace-string works when it's not compiled is the because
then this ends up in command-history:

(replace-string "buffer" "foo" nil (if (use-region-p) (region-beginning)) (if (use-region-p) (region-end)) nil nil)

For some reason.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Juri Linkov <juri <at> linkov.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 45607 <at> debbugs.gnu.org, Allen Li <darkfeline <at> felesatra.moe>
Subject: Re: bug#45607: 27.1; compiled replace-string breaks
 repeat-complex-command
Date: Tue, 07 Jun 2022 21:40:58 +0300

> Does anybody know of a more general solution to this?

This feature is broken by design as I explained in
https://debbugs.gnu.org/45617#17

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: larsi <at> gnus.org, 45607 <at> debbugs.gnu.org, darkfeline <at> felesatra.moe
Subject: Re: bug#45607: 27.1;
 compiled replace-string breaks repeat-complex-command
Date: Tue, 07 Jun 2022 21:58:05 +0300

> Cc: 45607 <at> debbugs.gnu.org, Allen Li <darkfeline <at> felesatra.moe>
> From: Juri Linkov <juri <at> linkov.net>
> Date: Tue, 07 Jun 2022 21:40:58 +0300
> 
> > Does anybody know of a more general solution to this?
> 
> This feature is broken by design as I explained in
> https://debbugs.gnu.org/45617#17

It isn't broken, you just expect it to do some magic that it never
meant to do.

As in many other cases, the perfect is the enemy of the good here.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: 45607 <at> debbugs.gnu.org, Allen Li <darkfeline <at> felesatra.moe>
Subject: Re: bug#45607: 27.1; compiled replace-string breaks
 repeat-complex-command
Date: Wed, 08 Jun 2022 14:05:33 +0200

Juri Linkov <juri <at> linkov.net> writes:

>> Does anybody know of a more general solution to this?
>
> This feature is broken by design as I explained in
> https://debbugs.gnu.org/45617#17

Hm...  Is there any way forward here?  Could we institute some special
form to be used in interactive specs that will be recorded in
command-history in a useful manner?  That is, code that today is:

	   (if (use-region-p) (region-beginning))
	   (if (use-region-p) (region-end))

could be something like

          (interactive-region-beginning)
          (interactive-region-end)

and whatever updates command-history would reify those as is instead of
their return values?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Allen Li <darkfeline <at> felesatra.moe>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, 45607 <at> debbugs.gnu.org,
 Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#45607: 27.1;
 compiled replace-string breaks repeat-complex-command
Date: Thu, 9 Jun 2022 01:39:01 -0700

[Message part 1 (text/plain, inline)]

On Tue, Jun 7, 2022 at 11:58 AM Eli Zaretskii <eliz <at> gnu.org> wrote:

> > Cc: 45607 <at> debbugs.gnu.org, Allen Li <darkfeline <at> felesatra.moe>
> > From: Juri Linkov <juri <at> linkov.net>
> > Date: Tue, 07 Jun 2022 21:40:58 +0300
> >
> > > Does anybody know of a more general solution to this?
> >
> > This feature is broken by design as I explained in
> > https://debbugs.gnu.org/45617#17
>
> It isn't broken, you just expect it to do some magic that it never
> meant to do.
>
> As in many other cases, the perfect is the enemy of the good here.
>

I think it's reasonable to consider the different behavior of evaled vs
compiled to be a bug.  Which one is correct can be debated, but the fact
that they're different is a bug.  Would you disagree?

[Message part 2 (text/html, inline)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Allen Li <darkfeline <at> felesatra.moe>
Cc: larsi <at> gnus.org, 45607 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#45607: 27.1;
 compiled replace-string breaks repeat-complex-command
Date: Thu, 09 Jun 2022 12:23:02 +0300

> From: Allen Li <darkfeline <at> felesatra.moe>
> Date: Thu, 9 Jun 2022 01:39:01 -0700
> Cc: Juri Linkov <juri <at> linkov.net>, Lars Ingebrigtsen <larsi <at> gnus.org>, 45607 <at> debbugs.gnu.org
> 
> 
> [1:text/plain Show]
> 
> 
> [2:text/html Hide Save:noname (944B)]
> 
> On Tue, Jun 7, 2022 at 11:58 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
> 
>  > Cc: 45607 <at> debbugs.gnu.org, Allen Li <darkfeline <at> felesatra.moe>
>  > From: Juri Linkov <juri <at> linkov.net>
>  > Date: Tue, 07 Jun 2022 21:40:58 +0300
>  > 
>  > > Does anybody know of a more general solution to this?
>  > 
>  > This feature is broken by design as I explained in
>  > https://debbugs.gnu.org/45617#17
> 
>  It isn't broken, you just expect it to do some magic that it never
>  meant to do.
> 
>  As in many other cases, the perfect is the enemy of the good here.
> 
> I think it's reasonable to consider the different behavior of evaled vs compiled to be a bug.  Which one is
> correct can be debated, but the fact that they're different is a bug.  Would you disagree? 

That's not what I alluded to, not at all.  I was talking about
repeat-complex-command itself and its alleged "broken" state.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: 45607 <at> debbugs.gnu.org, Allen Li <darkfeline <at> felesatra.moe>
Subject: Re: bug#45607: 27.1; compiled replace-string breaks
 repeat-complex-command
Date: Thu, 09 Jun 2022 20:52:57 +0200

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> could be something like
>
>           (interactive-region-beginning)
>           (interactive-region-end)
>
> and whatever updates command-history would reify those as is instead of
> their return values?

Sometimes it's helpful to actually look at the code.  All this magic
comes from:

/* If the list of args INPUT was produced with an explicit call to
   `list', look for elements that were computed with
   (region-beginning) or (region-end), and put those expressions into
   VALUES instead of the present values.

   This function doesn't return a value because it modifies elements
   of VALUES to do its job.  */

static void
fix_command (Lisp_Object input, Lisp_Object values)
{
  /* FIXME: Instead of this ugly hack, we should provide a way for an
     interactive spec to return an expression/function that will re-build the
     args without user intervention.  */
  if (CONSP (input))


And what this does is to try to hack its way through the lisp code in an
interactive spec like

      /* Skip through certain special forms.  */
      while (EQ (car, Qlet) || EQ (car, Qletx)
	     || EQ (car, Qsave_excursion)
	     || EQ (car, Qprogn))

looking for `region-beginning' and friends.  But we now byte-compile the
interactive specs, so all this fails spectacularly.

So we need a brand new way to specify which options are
`region-beginning' etc.  Perhaps with a declare form?  (That translates
into symbol properties, I guess.)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: 45607 <at> debbugs.gnu.org, Allen Li <darkfeline <at> felesatra.moe>
Subject: Re: bug#45607: 27.1; compiled replace-string breaks
 repeat-complex-command
Date: Thu, 09 Jun 2022 20:56:12 +0200

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> So we need a brand new way to specify which options are
> `region-beginning' etc.  Perhaps with a declare form?  (That translates
> into symbol properties, I guess.)

I.e.,

(defun replace-string (from-string to-string &optional delimited start end backward region-noncontiguous-p)
...
  (declare (arg start (if (use-region-p) (region-beginning)))
           (arg end (if (use-region-p) (region-end))))

and fix_command would pick them up from the symbol plist and use those
forms instead of the value for these arguments.

This could be generally useful if we have other things like this that we
want to have reified in a particular way in the command history.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Drew Adams <drew.adams <at> oracle.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>, Juri Linkov <juri <at> linkov.net>
Cc: "45607 <at> debbugs.gnu.org" <45607 <at> debbugs.gnu.org>,
 Allen Li <darkfeline <at> felesatra.moe>
Subject: RE: [External] : bug#45607: 27.1; compiled replace-string breaks
 repeat-complex-command
Date: Thu, 9 Jun 2022 20:51:29 +0000

I thank you for trying to fix this.  And
what you've said so far make sense to me.
This problem has long bugged me.

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

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Allen Li <darkfeline <at> felesatra.moe>, 45607 <at> debbugs.gnu.org,
 Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#45607: 27.1; compiled replace-string breaks
 repeat-complex-command
Date: Tue, 05 Jul 2022 16:41:23 +0200

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> I.e.,
>
> (defun replace-string (from-string to-string &optional delimited start end backward region-noncontiguous-p)
> ...
>   (declare (arg start (if (use-region-p) (region-beginning)))
>            (arg end (if (use-region-p) (region-end))))
>
> and fix_command would pick them up from the symbol plist and use those
> forms instead of the value for these arguments.

If we do that, it would be impossible to explicitly specify START and
END values that are different from an active region from ELisp code. If
the region is active, those arguments would always just be ignored.

We would substitute one ugly corner case with another one, but would
have added more semantic complexity.

We only have a problem for `repeat-complex-command' usage, right?  Then
the effect of a new `declare' spec should better be limited to the value
added to `command-history'.

Michael.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: Allen Li <darkfeline <at> felesatra.moe>, 45607 <at> debbugs.gnu.org,
 Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#45607: 27.1; compiled replace-string breaks
 repeat-complex-command
Date: Tue, 05 Jul 2022 18:45:37 +0200

Michael Heerdegen <michael_heerdegen <at> web.de> writes:

>> (defun replace-string (from-string to-string &optional delimited start end backward region-noncontiguous-p)
>> ...
>>   (declare (arg start (if (use-region-p) (region-beginning)))
>>            (arg end (if (use-region-p) (region-end))))
>>
>> and fix_command would pick them up from the symbol plist and use those
>> forms instead of the value for these arguments.

[...]

> We only have a problem for `repeat-complex-command' usage, right?  Then
> the effect of a new `declare' spec should better be limited to the value
> added to `command-history'.

Isn't that all that fix_command does?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Allen Li <darkfeline <at> felesatra.moe>, 45607 <at> debbugs.gnu.org,
 Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#45607: 27.1; compiled replace-string breaks
 repeat-complex-command
Date: Tue, 05 Jul 2022 20:47:33 +0200

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> > We only have a problem for `repeat-complex-command' usage, right?  Then
> > the effect of a new `declare' spec should better be limited to the value
> > added to `command-history'.
>
> Isn't that all that fix_command does?

Please ignore my comment if it does, I haven't checked.

Michael.

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

From: Juri Linkov <juri <at> linkov.net>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, 45607 <at> debbugs.gnu.org,
 Allen Li <darkfeline <at> felesatra.moe>
Subject: Re: bug#45607: 27.1; compiled replace-string breaks
 repeat-complex-command
Date: Wed, 06 Jul 2022 10:53:13 +0300

>> (defun replace-string (from-string to-string &optional delimited start end backward region-noncontiguous-p)
>> ...
>>   (declare (arg start (if (use-region-p) (region-beginning)))
>>            (arg end (if (use-region-p) (region-end))))
>>
>> and fix_command would pick them up from the symbol plist and use those
>> forms instead of the value for these arguments.
>
> If we do that, it would be impossible to explicitly specify START and
> END values that are different from an active region from ELisp code. If
> the region is active, those arguments would always just be ignored.

Indeed, some users might want to have numbers for START and END values
to repeat the command exactly on the same previous region, but other users
might want to repeat the command on a newly selected region with
(region-beginning)/(region-end) in the command history.

OTOH, keeping numbers in the history breaks replace-string for
rectangular regions, because the command history will contain
inconsistent numbers: some numbers from the previous replacement,
and other numbers from the new rectangular region.  For example:

  (replace-string "buffer" "foo" nil 1 2 nil '((3 . 4) (5 . 6)))

where 1 2 are the old region boundaries, and '((3 . 4) (5 . 6))
is a new rectangular region boundaries.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 45607 <at> debbugs.gnu.org,
 Allen Li <darkfeline <at> felesatra.moe>
Subject: Re: bug#45607: 27.1; compiled replace-string breaks
 repeat-complex-command
Date: Wed, 06 Jul 2022 13:35:28 +0200

Juri Linkov <juri <at> linkov.net> writes:

>>> (defun replace-string (from-string to-string &optional delimited start end backward region-noncontiguous-p)
>>> ...
>>>   (declare (arg start (if (use-region-p) (region-beginning)))
>>>            (arg end (if (use-region-p) (region-end))))

[...]

> Indeed, some users might want to have numbers for START and END values
> to repeat the command exactly on the same previous region, but other users
> might want to repeat the command on a newly selected region with
> (region-beginning)/(region-end) in the command history.

Have a look at fix_command -- it tries to parse code in an interactive
spec to find instances of

  preserved_fns = pure_list (intern_c_string ("region-beginning"),
			     intern_c_string ("region-end"),
			     intern_c_string ("point"),
			     intern_c_string ("mark"));

in the code.  (Which doesn't work now, of course, since the spec is
byte-compiled.)  My `declare' suggestion would just make this work
again, and fix a regression.  That is, this isn't new functionality.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Juri Linkov <juri <at> linkov.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 45607 <at> debbugs.gnu.org,
 Allen Li <darkfeline <at> felesatra.moe>
Subject: Re: bug#45607: 27.1; compiled replace-string breaks
 repeat-complex-command
Date: Wed, 06 Jul 2022 21:39:11 +0300

>>>> (defun replace-string (from-string to-string &optional delimited start end backward region-noncontiguous-p)
>>>> ...
>>>>   (declare (arg start (if (use-region-p) (region-beginning)))
>>>>            (arg end (if (use-region-p) (region-end))))
>
> My `declare' suggestion would just make this work again, and fix
> a regression.  That is, this isn't new functionality.

Does `declare' put some property on the command's symbol?
Then if a user doesn't want this fix_command thing, it's
easy to customize and remove a special property from the symbol
of a command like `replace-string'.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 45607 <at> debbugs.gnu.org,
 Allen Li <darkfeline <at> felesatra.moe>
Subject: Re: bug#45607: 27.1; compiled replace-string breaks
 repeat-complex-command
Date: Thu, 07 Jul 2022 10:02:35 +0200

Juri Linkov <juri <at> linkov.net> writes:

> Does `declare' put some property on the command's symbol?
> Then if a user doesn't want this fix_command thing, it's
> easy to customize and remove a special property from the symbol
> of a command like `replace-string'.

`declare' forms can do basically anything, but, yes, in this case, I
think a symbol property would make the most sense.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 45607 <at> debbugs.gnu.org,
 Allen Li <darkfeline <at> felesatra.moe>
Subject: Re: bug#45607: 27.1; compiled replace-string breaks
 repeat-complex-command
Date: Mon, 08 Aug 2022 15:53:30 +0200

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> `declare' forms can do basically anything, but, yes, in this case, I
> think a symbol property would make the most sense.

I've now fixed the replace-string problem in Emacs 29.

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

From: Juri Linkov <juri <at> linkov.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 45607 <at> debbugs.gnu.org,
 Allen Li <darkfeline <at> felesatra.moe>
Subject: Re: bug#45607: 27.1; compiled replace-string breaks
 repeat-complex-command
Date: Mon, 08 Aug 2022 20:07:31 +0300

>> `declare' forms can do basically anything, but, yes, in this case, I
>> think a symbol property would make the most sense.
>
> I've now fixed the replace-string problem in Emacs 29.

Should the same interactive-args now be added to other
replacement commands?

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 45607 <at> debbugs.gnu.org,
 Allen Li <darkfeline <at> felesatra.moe>
Subject: Re: bug#45607: 27.1; compiled replace-string breaks
 repeat-complex-command
Date: Tue, 09 Aug 2022 17:00:58 +0200

Juri Linkov <juri <at> linkov.net> writes:

>> I've now fixed the replace-string problem in Emacs 29.
>
> Should the same interactive-args now be added to other
> replacement commands?

It should be added to all commands that work on the region like this,
yes.  But I wondered whether we should make some trivial helper
functions first like

(defun use-region-beginning ()
  "Return the start of the region if `use-region-p'."
  (and (use-region-p) (region-beginning)))

and the same for -end to avoid having to repeat that code phrase so many
places.

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

From: Juri Linkov <juri <at> linkov.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 45607 <at> debbugs.gnu.org,
 Allen Li <darkfeline <at> felesatra.moe>
Subject: Re: bug#45607: 27.1; compiled replace-string breaks
 repeat-complex-command
Date: Tue, 09 Aug 2022 21:41:24 +0300

> It should be added to all commands that work on the region like this,
> yes.  But I wondered whether we should make some trivial helper
> functions first like
>
> (defun use-region-beginning ()
>   "Return the start of the region if `use-region-p'."
>   (and (use-region-p) (region-beginning)))
>
> and the same for -end to avoid having to repeat that code phrase so many
> places.

Indeed, this will help to make the history items shorter:

  (replace-string "a" "b" nil (if (use-region-p) (region-beginning)) (if (use-region-p) (region-end)))
  ->
  (replace-string "a" "b" nil (use-region-beginning) (use-region-end))

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: michael_heerdegen <at> web.de, larsi <at> gnus.org, 45607 <at> debbugs.gnu.org,
 darkfeline <at> felesatra.moe
Subject: Re: bug#45607: 27.1;
 compiled replace-string breaks repeat-complex-command
Date: Tue, 09 Aug 2022 21:48:18 +0300

> Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 45607 <at> debbugs.gnu.org,
>  Allen Li <darkfeline <at> felesatra.moe>
> From: Juri Linkov <juri <at> linkov.net>
> Date: Tue, 09 Aug 2022 21:41:24 +0300
> 
> > It should be added to all commands that work on the region like this,
> > yes.  But I wondered whether we should make some trivial helper
> > functions first like
> >
> > (defun use-region-beginning ()
> >   "Return the start of the region if `use-region-p'."
> >   (and (use-region-p) (region-beginning)))
> >
> > and the same for -end to avoid having to repeat that code phrase so many
> > places.
> 
> Indeed, this will help to make the history items shorter:
> 
>   (replace-string "a" "b" nil (if (use-region-p) (region-beginning)) (if (use-region-p) (region-end)))
>   ->
>   (replace-string "a" "b" nil (use-region-beginning) (use-region-end))

Bonus points for calling use-region-p just once, not twice.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 45607 <at> debbugs.gnu.org,
 Allen Li <darkfeline <at> felesatra.moe>
Subject: Re: bug#45607: 27.1; compiled replace-string breaks
 repeat-complex-command
Date: Tue, 09 Aug 2022 21:14:20 +0200

Juri Linkov <juri <at> linkov.net> writes:

> Indeed, this will help to make the history items shorter:
>
>   (replace-string "a" "b" nil (if (use-region-p) (region-beginning))
> (if (use-region-p) (region-end)))
>   ->
>   (replace-string "a" "b" nil (use-region-beginning) (use-region-end))

OK, so I've now done this.  So somebody™ should go through the code base
and adjust the code and add the new `declare' forms.  😀

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: michael_heerdegen <at> web.de, darkfeline <at> felesatra.moe, 45607 <at> debbugs.gnu.org,
 Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#45607: 27.1; compiled replace-string breaks
 repeat-complex-command
Date: Tue, 09 Aug 2022 21:15:51 +0200

Eli Zaretskii <eliz <at> gnu.org> writes:

> Bonus points for calling use-region-p just once, not twice.

I don't really see a convenient way to do that in these cases -- the
only ones I could think of (adding a new function to return both
start/end, and then splice the results in into the `interactive' specs
etc) would lead to obfuscated code.

(And it's not like this is performance critical anyway.)

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: michael_heerdegen <at> web.de, darkfeline <at> felesatra.moe, 45607 <at> debbugs.gnu.org,
 juri <at> linkov.net
Subject: Re: bug#45607: 27.1; compiled replace-string breaks
 repeat-complex-command
Date: Tue, 09 Aug 2022 22:25:44 +0300

> From: Lars Ingebrigtsen <larsi <at> gnus.org>
> Cc: Juri Linkov <juri <at> linkov.net>,  michael_heerdegen <at> web.de,
>   45607 <at> debbugs.gnu.org,  darkfeline <at> felesatra.moe
> Date: Tue, 09 Aug 2022 21:15:51 +0200
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> > Bonus points for calling use-region-p just once, not twice.
> 
> I don't really see a convenient way to do that in these cases -- the
> only ones I could think of (adding a new function to return both
> start/end, and then splice the results in into the `interactive' specs
> etc) would lead to obfuscated code.

What about cl-destructuring-bind and its ilk?

> (And it's not like this is performance critical anyway.)

It's IMO inelegant to make the same test twice in a row.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: michael_heerdegen <at> web.de, darkfeline <at> felesatra.moe, 45607 <at> debbugs.gnu.org,
 juri <at> linkov.net
Subject: Re: bug#45607: 27.1; compiled replace-string breaks
 repeat-complex-command
Date: Tue, 09 Aug 2022 21:30:01 +0200

Eli Zaretskii <eliz <at> gnu.org> writes:

> What about cl-destructuring-bind and its ilk?

That'd possible, of course, but awkward.

>> (And it's not like this is performance critical anyway.)
>
> It's IMO inelegant to make the same test twice in a row.

Yup.

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

From: Juri Linkov <juri <at> linkov.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 45607 <at> debbugs.gnu.org,
 Allen Li <darkfeline <at> felesatra.moe>
Subject: Re: bug#45607: 27.1; compiled replace-string breaks
 repeat-complex-command
Date: Tue, 09 Aug 2022 22:30:54 +0300

>>   (replace-string "a" "b" nil (if (use-region-p) (region-beginning))
>> (if (use-region-p) (region-end)))
>>   ->
>>   (replace-string "a" "b" nil (use-region-beginning) (use-region-end))
>
> OK, so I've now done this.  So somebody™ should go through the code base
> and adjust the code and add the new `declare' forms.  😀

Interesting, there are not too many uses of this pattern,
and most of them are related to replacement commands.
Ok, I could replace them with adding interactive-args.

lisp/isearch.el
2392:      (if (use-region-p) (region-beginning))
2393:      (if (use-region-p) (region-end))
lisp/replace.el
464: 	   (if (use-region-p) (region-beginning))
465: 	   (if (use-region-p) (region-end))
558: 	   (if (use-region-p) (region-beginning))
559: 	   (if (use-region-p) (region-end))
606: 	   (if (use-region-p) (region-beginning))
607: 	   (if (use-region-p) (region-end))
761: 	   (if (use-region-p) (region-beginning))
762: 	   (if (use-region-p) (region-end))
lisp/textmodes/paragraphs.el
518:                      (if (use-region-p) (region-beginning))
519:                      (if (use-region-p) (region-end))))
lisp/vc/log-view.el
581:    (list (if (use-region-p) (region-beginning) (point))
582:          (if (use-region-p) (region-end) (point))))
596:    (list (if (use-region-p) (region-beginning) (point))
597:          (if (use-region-p) (region-end) (point))))

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 45607 <at> debbugs.gnu.org,
 Allen Li <darkfeline <at> felesatra.moe>
Subject: Re: bug#45607: 27.1; compiled replace-string breaks
 repeat-complex-command
Date: Fri, 12 Aug 2022 15:01:47 +0200

Juri Linkov <juri <at> linkov.net> writes:

> Interesting, there are not too many uses of this pattern,
> and most of them are related to replacement commands.
> Ok, I could replace them with adding interactive-args.
>
> lisp/isearch.el
> 2392:      (if (use-region-p) (region-beginning))
> 2393:      (if (use-region-p) (region-end))
> lisp/replace.el
> 464: 	   (if (use-region-p) (region-beginning))
> 465: 	   (if (use-region-p) (region-end))
> 558: 	   (if (use-region-p) (region-beginning))
> 559: 	   (if (use-region-p) (region-end))
> 606: 	   (if (use-region-p) (region-beginning))
> 607: 	   (if (use-region-p) (region-end))
> 761: 	   (if (use-region-p) (region-beginning))
> 762: 	   (if (use-region-p) (region-end))
> lisp/textmodes/paragraphs.el
> 518:                      (if (use-region-p) (region-beginning))
> 519:                      (if (use-region-p) (region-end))))
> lisp/vc/log-view.el
> 581:    (list (if (use-region-p) (region-beginning) (point))
> 582:          (if (use-region-p) (region-end) (point))))
> 596:    (list (if (use-region-p) (region-beginning) (point))
> 597:          (if (use-region-p) (region-end) (point))))

That's fewer than I'd have guessed -- but I guess that quite a few
commands stash the logic down into the function body instead of putting
it into the `interactive' spec.  Commands like `duplicate-dwim', for
instance, could be pretty easily fixed in that way, for instance.

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

From: Juri Linkov <juri <at> linkov.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 45607 <at> debbugs.gnu.org,
 Allen Li <darkfeline <at> felesatra.moe>
Subject: Re: bug#45607: 27.1; compiled replace-string breaks
 repeat-complex-command
Date: Fri, 12 Aug 2022 20:39:32 +0300

>> 2392:      (if (use-region-p) (region-beginning))
>> 2393:      (if (use-region-p) (region-end))
>
> That's fewer than I'd have guessed -- but I guess that quite a few
> commands stash the logic down into the function body instead of putting
> it into the `interactive' spec.  Commands like `duplicate-dwim', for
> instance, could be pretty easily fixed in that way, for instance.

OTOH, `duplicate-dwim' is optimized to call `use-region-p' only once:

  (cond
   ((use-region-p)
    (let* ((beg (region-beginning))
           (end (region-end))

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 45607 <at> debbugs.gnu.org,
 Allen Li <darkfeline <at> felesatra.moe>
Subject: Re: bug#45607: 27.1; compiled replace-string breaks
 repeat-complex-command
Date: Sat, 13 Aug 2022 13:46:39 +0200

Juri Linkov <juri <at> linkov.net> writes:

> OTOH, `duplicate-dwim' is optimized to call `use-region-p' only once:
>
>   (cond
>    ((use-region-p)
>     (let* ((beg (region-beginning))
>            (end (region-end))

I don't think that makes much difference when it comes to interactive
specs:

(benchmark-run 1000000 (use-region-p))
=> (0.038645288 0 0.0)

If you're calling `duplicate-dwim' interactively more than a million
times a second, you'll get a slowdown of a couple hundredths of a
second.

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

From: Juri Linkov <juri <at> linkov.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 45607 <at> debbugs.gnu.org,
 Allen Li <darkfeline <at> felesatra.moe>
Subject: Re: bug#45607: 27.1; compiled replace-string breaks
 repeat-complex-command
Date: Sat, 13 Aug 2022 22:30:50 +0300

>> OTOH, `duplicate-dwim' is optimized to call `use-region-p' only once:
>>
>>   (cond
>>    ((use-region-p)
>>     (let* ((beg (region-beginning))
>>            (end (region-end))
>
> I don't think that makes much difference when it comes to interactive
> specs:
>
> (benchmark-run 1000000 (use-region-p))
> => (0.038645288 0 0.0)
>
> If you're calling `duplicate-dwim' interactively more than a million
> times a second, you'll get a slowdown of a couple hundredths of a
> second.

But still `use-region-p' is used to start a new logical branch of `cond'.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 45607 <at> debbugs.gnu.org,
 Allen Li <darkfeline <at> felesatra.moe>
Subject: Re: bug#45607: 27.1; compiled replace-string breaks
 repeat-complex-command
Date: Mon, 15 Aug 2022 08:37:02 +0200

Juri Linkov <juri <at> linkov.net> writes:

> But still `use-region-p' is used to start a new logical branch of `cond'.

The suggestion was to put that logic into the interactive spec and bind
to new parameters beg/end (like other commands do).  The `cond' would
then be adjusted to react to beg/end.

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

From: Juri Linkov <juri <at> linkov.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 45607 <at> debbugs.gnu.org,
 Allen Li <darkfeline <at> felesatra.moe>
Subject: Re: bug#45607: 27.1; compiled replace-string breaks
 repeat-complex-command
Date: Sun, 04 Sep 2022 19:57:01 +0300

>>>   (replace-string "a" "b" nil (if (use-region-p) (region-beginning))
>>> (if (use-region-p) (region-end)))
>>>   ->
>>>   (replace-string "a" "b" nil (use-region-beginning) (use-region-end))
>>
>> OK, so I've now done this.  So somebody™ should go through the code base
>> and adjust the code and add the new `declare' forms.  😀
>
> Interesting, there are not too many uses of this pattern,
> and most of them are related to replacement commands.
> Ok, I could replace them with adding interactive-args.

Done, with adding also 'use-region-noncontiguous-p'.

Previous Next

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