27.1; compiled replace-string breaks repeat-complex-command

DoneSubmitted by Allen Li.
Details
6 participants
  • Allen Li
  • Drew Adams
  • Eli Zaretskii
  • Juri Linkov
  • Lars Ingebrigtsen
  • Michael Heerdegen
Owner
unassigned
Severity
normal
A
A
Allen Li wrote on 2 Jan 2021 10:04
(address . bug-gnu-emacs@gnu.org)
80o8i7676p.fsf@felesatra.moe
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
L
L
Lars Ingebrigtsen wrote on 7 Jun 14:38 +0200
(name . Allen Li)(address . darkfeline@felesatra.moe)(address . 45607@debbugs.gnu.org)
87czfkir6n.fsf@gnus.org
Allen Li <darkfeline@felesatra.moe> writes:

Toggle quote (12 lines)
> 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.)
L
L
Lars Ingebrigtsen wrote on 7 Jun 14:39 +0200
control message for bug #45607
(address . control@debbugs.gnu.org)
87bkv4ir6h.fsf@gnus.org
tags 45607 + moreinfo
quit
J
J
Juri Linkov wrote on 7 Jun 20:40 +0200
Re: bug#45607: 27.1; compiled replace-string breaks repeat-complex-command
(name . Lars Ingebrigtsen)(address . larsi@gnus.org)
86r1402un1.fsf@mail.linkov.net
Toggle quote (2 lines)
> Does anybody know of a more general solution to this?

This feature is broken by design as I explained in
E
E
Eli Zaretskii wrote on 7 Jun 20:58 +0200
(name . Juri Linkov)(address . juri@linkov.net)
83pmjk5miq.fsf@gnu.org
Toggle quote (9 lines)
> Cc: 45607@debbugs.gnu.org, Allen Li <darkfeline@felesatra.moe>
> From: Juri Linkov <juri@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.
L
L
Lars Ingebrigtsen wrote on 8 Jun 14:05 +0200
(name . Juri Linkov)(address . juri@linkov.net)
87y1y79x82.fsf@gnus.org
Juri Linkov <juri@linkov.net> writes:

Toggle quote (5 lines)
>> 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.)
A
A
Allen Li wrote on 9 Jun 10:39 +0200
(name . Eli Zaretskii)(address . eliz@gnu.org)
CADbSrJy9QUJZNFGAij1d3Cjbp0muqn3Nf38T=6yK+NnJbXgGng@mail.gmail.com
On Tue, Jun 7, 2022 at 11:58 AM Eli Zaretskii <eliz@gnu.org> wrote:

Toggle quote (15 lines)
> > Cc: 45607@debbugs.gnu.org, Allen Li <darkfeline@felesatra.moe>
> > From: Juri Linkov <juri@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?
Attachment: file
E
E
Eli Zaretskii wrote on 9 Jun 11:23 +0200
(name . Allen Li)(address . darkfeline@felesatra.moe)
834k0u42dl.fsf@gnu.org
Toggle quote (29 lines)
> From: Allen Li <darkfeline@felesatra.moe>
> Date: Thu, 9 Jun 2022 01:39:01 -0700
> Cc: Juri Linkov <juri@linkov.net>, Lars Ingebrigtsen <larsi@gnus.org>, 45607@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@gnu.org> wrote:
>
> > Cc: 45607@debbugs.gnu.org, Allen Li <darkfeline@felesatra.moe>
> > From: Juri Linkov <juri@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.
L
L
Lars Ingebrigtsen wrote on 9 Jun 20:52 +0200
(name . Juri Linkov)(address . juri@linkov.net)
87v8t9y8hi.fsf@gnus.org
Lars Ingebrigtsen <larsi@gnus.org> writes:

Toggle quote (8 lines)
> 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.)
L
L
Lars Ingebrigtsen wrote on 9 Jun 20:56 +0200
(name . Juri Linkov)(address . juri@linkov.net)
87r13xy8c3.fsf@gnus.org
Lars Ingebrigtsen <larsi@gnus.org> writes:

Toggle quote (4 lines)
> 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.)
D
D
Drew Adams wrote on 9 Jun 22:51 +0200
RE: [External] : bug#45607: 27.1; compiled replace-string breaks repeat-complex-command
SJ0PR10MB5488DDCF5BF252716BEF8B20F3A79@SJ0PR10MB5488.namprd10.prod.outlook.com
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.
M
M
Michael Heerdegen wrote on 5 Jul 16:41 +0200
Re: bug#45607: 27.1; compiled replace-string breaks repeat-complex-command
(name . Lars Ingebrigtsen)(address . larsi@gnus.org)
871quzaagc.fsf@web.de
Lars Ingebrigtsen <larsi@gnus.org> writes:

Toggle quote (10 lines)
> 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.
L
L
Lars Ingebrigtsen wrote on 5 Jul 18:45 +0200
(name . Michael Heerdegen)(address . michael_heerdegen@web.de)
87fsjf1pam.fsf@gnus.org
Michael Heerdegen <michael_heerdegen@web.de> writes:

Toggle quote (8 lines)
>> (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.

[...]

Toggle quote (4 lines)
> 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.)
M
M
Michael Heerdegen wrote on 5 Jul 20:47 +0200
(name . Lars Ingebrigtsen)(address . larsi@gnus.org)
87sfnfs8fu.fsf@web.de
Lars Ingebrigtsen <larsi@gnus.org> writes:

Toggle quote (6 lines)
> > 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.
J
J
Juri Linkov wrote on 6 Jul 09:53 +0200
(name . Michael Heerdegen)(address . michael_heerdegen@web.de)
861quywusu.fsf@mail.linkov.net
Toggle quote (12 lines)
>> (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.
L
L
Lars Ingebrigtsen wrote on 6 Jul 13:35 +0200
(name . Juri Linkov)(address . juri@linkov.net)
8735felbi7.fsf@gnus.org
Juri Linkov <juri@linkov.net> writes:

Toggle quote (5 lines)
>>> (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))))

[...]

Toggle quote (5 lines)
> 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.)
J
J
Juri Linkov wrote on 6 Jul 20:39 +0200
(name . Lars Ingebrigtsen)(address . larsi@gnus.org)
864jzursq8.fsf@mail.linkov.net
Toggle quote (8 lines)
>>>> (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'.
L
L
Lars Ingebrigtsen wrote on 7 Jul 10:02 +0200
(name . Juri Linkov)(address . juri@linkov.net)
87zghlgxk4.fsf@gnus.org
Juri Linkov <juri@linkov.net> writes:

Toggle quote (5 lines)
> 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.)
L
L
Lars Ingebrigtsen wrote on 8 Aug 15:53 +0200
(name . Juri Linkov)(address . juri@linkov.net)
87bksuzvs5.fsf@gnus.org
Lars Ingebrigtsen <larsi@gnus.org> writes:

Toggle quote (3 lines)
> `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.
L
L
Lars Ingebrigtsen wrote on 8 Aug 15:53 +0200
control message for bug #45607
(address . control@debbugs.gnu.org)
87a68ezvrx.fsf@gnus.org
close 45607 29.1
quit
J
J
Juri Linkov wrote on 8 Aug 19:07 +0200
Re: bug#45607: 27.1; compiled replace-string breaks repeat-complex-command
(name . Lars Ingebrigtsen)(address . larsi@gnus.org)
86iln264vg.fsf@mail.linkov.net
Toggle quote (5 lines)
>> `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?
L
L
Lars Ingebrigtsen wrote on 9 Aug 17:00 +0200
(name . Juri Linkov)(address . juri@linkov.net)
871qtpzck5.fsf@gnus.org
Juri Linkov <juri@linkov.net> writes:

Toggle quote (5 lines)
>> 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.
J
J
Juri Linkov wrote on 9 Aug 20:41 +0200
(name . Lars Ingebrigtsen)(address . larsi@gnus.org)
868rnxqmy3.fsf@mail.linkov.net
Toggle quote (11 lines)
> 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))
E
E
Eli Zaretskii wrote on 9 Aug 20:48 +0200
(name . Juri Linkov)(address . juri@linkov.net)
83wnbhs171.fsf@gnu.org
Toggle quote (22 lines)
> Cc: Michael Heerdegen <michael_heerdegen@web.de>, 45607@debbugs.gnu.org,
> Allen Li <darkfeline@felesatra.moe>
> From: Juri Linkov <juri@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.
L
L
Lars Ingebrigtsen wrote on 9 Aug 21:14 +0200
(name . Juri Linkov)(address . juri@linkov.net)
87r11ptek3.fsf@gnus.org
Juri Linkov <juri@linkov.net> writes:

Toggle quote (7 lines)
> 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. 😀
L
L
Lars Ingebrigtsen wrote on 9 Aug 21:15 +0200
(name . Eli Zaretskii)(address . eliz@gnu.org)
87mtcdtehk.fsf@gnus.org
Eli Zaretskii <eliz@gnu.org> writes:

Toggle quote (2 lines)
> 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.)
E
E
Eli Zaretskii wrote on 9 Aug 21:25 +0200
(name . Lars Ingebrigtsen)(address . larsi@gnus.org)
83pmh9rzgn.fsf@gnu.org
Toggle quote (14 lines)
> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Juri Linkov <juri@linkov.net>, michael_heerdegen@web.de,
> 45607@debbugs.gnu.org, darkfeline@felesatra.moe
> Date: Tue, 09 Aug 2022 21:15:51 +0200
>
> Eli Zaretskii <eliz@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?

Toggle quote (2 lines)
> (And it's not like this is performance critical anyway.)

It's IMO inelegant to make the same test twice in a row.
L
L
Lars Ingebrigtsen wrote on 9 Aug 21:30 +0200
(name . Eli Zaretskii)(address . eliz@gnu.org)
87edxptdty.fsf@gnus.org
Eli Zaretskii <eliz@gnu.org> writes:

Toggle quote (2 lines)
> What about cl-destructuring-bind and its ilk?

That'd possible, of course, but awkward.

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

Yup.
J
J
Juri Linkov wrote on 9 Aug 21:30 +0200
(name . Lars Ingebrigtsen)(address . larsi@gnus.org)
86tu6lnrip.fsf@mail.linkov.net
Toggle quote (8 lines)
>> (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))))
L
L
Lars Ingebrigtsen wrote 6 days ago
(name . Juri Linkov)(address . juri@linkov.net)
87h72hr4xw.fsf@gnus.org
Juri Linkov <juri@linkov.net> writes:

Toggle quote (25 lines)
> 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.
J
J
Juri Linkov wrote 5 days ago
(name . Lars Ingebrigtsen)(address . larsi@gnus.org)
86o7wpjr8r.fsf@mail.linkov.net
Toggle quote (8 lines)
>> 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))
L
L
Lars Ingebrigtsen wrote 5 days ago
(name . Juri Linkov)(address . juri@linkov.net)
87tu6gjrhc.fsf@gnus.org
Juri Linkov <juri@linkov.net> writes:

Toggle quote (7 lines)
> 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.
J
J
Juri Linkov wrote 4 days ago
(name . Lars Ingebrigtsen)(address . larsi@gnus.org)
86bksn53dx.fsf@mail.linkov.net
Toggle quote (17 lines)
>> 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'.
L
L
Lars Ingebrigtsen wrote 3 days ago
(name . Juri Linkov)(address . juri@linkov.net)
87y1vqc8s1.fsf@gnus.org
Juri Linkov <juri@linkov.net> writes:

Toggle quote (2 lines)
> 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.
?