GNU bug report logs - #27897
25.1; Add REGION-NONCONTIGUOUS-P arg to other replace.el commands

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 27897 in the body.
You can then email your comments to 27897 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: Drew Adams <drew.adams <at> oracle.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 25.1; Add REGION-NONCONTIGUOUS-P arg to other replace.el commands
Date: Mon, 31 Jul 2017 21:37:23 -0700 (PDT)

Subject line says it all.

The patch is trivial - just do the same thing to commands such as
`replace-string', `replace-regexp', `query-replace-regexp-eval', and
`map-query-replace-regexp' as you did to `query-replace' and
`query-replace-regexp': add REGION-NONCONTIGUOUS-P arg to the
interactive spec the same way, and pass it to `perform-replace' the same
way.

(Shouldn't this have been done in the first place?)


In GNU Emacs 25.1.1 (x86_64-w64-mingw32)
 of 2016-09-17 built on LAPHROAIG
Windowing system distributor 'Microsoft Corp.', version 6.1.7601
Configured using:
 'configure --without-dbus --without-compress-install CFLAGS=-static'

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

From: Drew Adams <drew.adams <at> oracle.com>
To: 27897 <at> debbugs.gnu.org
Subject: RE: bug#27897: 25.1; Add REGION-NONCONTIGUOUS-P arg to other
 replace.el commands
Date: Tue, 17 Apr 2018 14:53:58 -0700 (PDT)

ping.

Do you need an explicit patch for this?  Would that make a
difference in getting this done?

> The patch is trivial - just do the same thing to commands such as
> `replace-string', `replace-regexp', `query-replace-regexp-eval', and
> `map-query-replace-regexp' as you did to `query-replace' and
> `query-replace-regexp': add REGION-NONCONTIGUOUS-P arg to the
> interactive spec the same way, and pass it to `perform-replace' the same
> way.
> 
> (Shouldn't this have been done in the first place?)

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

From: Drew Adams <drew.adams <at> oracle.com>
To: 27897 <at> debbugs.gnu.org
Subject: RE: bug#27897: [PATCH] 25.1; Add REGION-NONCONTIGUOUS-P arg to other
 replace.el commands
Date: Wed, 18 Apr 2018 14:05:28 -0700 (PDT)

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

Attached is a patch for this.

(I updated doc strings for arg REGION-NONCONTIGUOUS-P.
See also bug #31207, for other doc-string changes that
should be made.)

> ping.
> 
> Do you need an explicit patch for this?  Would that make a
> difference in getting this done?
> 
> > The patch is trivial - just do the same thing to commands such as
> > `replace-string', `replace-regexp', `query-replace-regexp-eval', and
> > `map-query-replace-regexp' as you did to `query-replace' and
> > `query-replace-regexp': add REGION-NONCONTIGUOUS-P arg to the
> > interactive spec the same way, and pass it to `perform-replace' the
> same
> > way.
> >
> > (Shouldn't this have been done in the first place?)

[replace-2018-04-18.patch (application/octet-stream, attachment)]

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

From: Juri Linkov <juri <at> linkov.net>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: 27897 <at> debbugs.gnu.org
Subject: Re: bug#27897: [PATCH] 25.1;
 Add REGION-NONCONTIGUOUS-P arg to other replace.el commands
Date: Thu, 19 Apr 2018 00:41:20 +0300

> -(defun query-replace-regexp-eval (regexp to-expr &optional delimited start end)
> +(defun query-replace-regexp-eval (regexp to-expr &optional delimited start end region-noncontiguous-p)
> -(defun map-query-replace-regexp (regexp to-strings &optional n start end)
> +(defun map-query-replace-regexp (regexp to-strings &optional n start end region-noncontiguous-p)

But why not to add the arg backward like in all other commands?

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

From: Drew Adams <drew.adams <at> oracle.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 27897 <at> debbugs.gnu.org
Subject: RE: bug#27897: [PATCH] 25.1; Add REGION-NONCONTIGUOUS-P arg to other
 replace.el commands
Date: Wed, 18 Apr 2018 16:25:54 -0700 (PDT)

> > -(defun query-replace-regexp-eval (regexp to-expr &optional delimited
> start end)
> > +(defun query-replace-regexp-eval (regexp to-expr &optional delimited
> start end region-noncontiguous-p)
> > -(defun map-query-replace-regexp (regexp to-strings &optional n start
> end)
> > +(defun map-query-replace-regexp (regexp to-strings &optional n start
> end region-noncontiguous-p)
> 
> But why not to add the arg backward like in all other commands?

PLEASE DO, here and elsewhere where it is missing, wherever
it makes sense.

I'm having enough trouble just getting REGION-NONCONTIGUOUS-P
added. ;-)

[And getting arguments described in doc strings (bug #31207).
I was afraid that someone might even complain that I added
some rudimentary mention of other args in this patch.  But
I tried to sneak that in anyway. ;-)]

And some of the commands I added REGION-NONCONTIGUOUS-P to
already had BACKWARD, while others did not.  Should we
assume that whoever did that did it on purpose and wisely?
Dunno.  My purpose here was not to fix missing BACKWARD,
but if you want to think about that and take care of it,
please do - that would be great, along with applying my
patch for REGION-NONCONTIGUOUS-P.

I have the impression that someone added some stuff here
and there without bothering to think more about it and
doing it more systematically.  But maybe not; maybe there
are good reasons why they did what they did.  For
REGION-NONCONTIGUOUS-P, at least, it seems clear to me
that it is useful everywhere I added it.

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

From: Juri Linkov <juri <at> linkov.net>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: 27897-done <at> debbugs.gnu.org
Subject: Re: bug#27897: [PATCH] 25.1;
 Add REGION-NONCONTIGUOUS-P arg to other replace.el commands
Date: Thu, 19 Apr 2018 22:36:53 +0300

> And some of the commands I added REGION-NONCONTIGUOUS-P to
> already had BACKWARD, while others did not.  Should we
> assume that whoever did that did it on purpose and wisely?

query-replace-regexp-eval has no BACKWARD arg because
query-replace-regexp-eval is obsolete and should be removed.
map-query-replace-regexp has no BACKWARD arg because
map-query-replace-regexp has special logic for its prefix arg.

> Dunno.  My purpose here was not to fix missing BACKWARD,
> but if you want to think about that and take care of it,
> please do - that would be great, along with applying my
> patch for REGION-NONCONTIGUOUS-P.

Thanks, your patch pushed to master as 75a32f4.

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

From: Drew Adams <drew.adams <at> oracle.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 27897-done <at> debbugs.gnu.org
Subject: RE: bug#27897: [PATCH] 25.1; Add REGION-NONCONTIGUOUS-P arg to other
 replace.el commands
Date: Thu, 19 Apr 2018 12:48:00 -0700 (PDT)

> Thanks, your patch pushed to master as 75a32f4.

Thank you.

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

From: Drew Adams <drew.adams <at> oracle.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 27897-done <at> debbugs.gnu.org
Subject: RE: bug#27897: [PATCH] 25.1; Add REGION-NONCONTIGUOUS-P arg to other
 replace.el commands
Date: Thu, 19 Apr 2018 13:12:13 -0700 (PDT)

> Thanks, your patch pushed to master as 75a32f4.

BTW, what does that mean, in terms of something I can test.
Does it mean, for example, (> emacs-major-version 26)?

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

From: Juri Linkov <juri <at> linkov.net>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: 27897 <at> debbugs.gnu.org
Subject: Re: bug#27897: [PATCH] 25.1;
 Add REGION-NONCONTIGUOUS-P arg to other replace.el commands
Date: Thu, 19 Apr 2018 23:27:59 +0300

fixed 27897 27.0.50
thanks

>> Thanks, your patch pushed to master as 75a32f4.
>
> BTW, what does that mean, in terms of something I can test.
> Does it mean, for example, (> emacs-major-version 26)?

Thanks for the reminder, now marked as fixed in 27.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: 27897 <at> debbugs.gnu.org, drew.adams <at> oracle.com
Subject: Re: bug#27897: [PATCH] 25.1;
 Add REGION-NONCONTIGUOUS-P arg to other replace.el commands
Date: Fri, 20 Apr 2018 10:31:14 +0300

> From: Juri Linkov <juri <at> linkov.net>
> Date: Thu, 19 Apr 2018 22:36:53 +0300
> Cc: 27897-done <at> debbugs.gnu.org
> 
> Thanks, your patch pushed to master as 75a32f4.

I think this needs to be reflected ion NEWS, and perhaps also in the
manuals.

Thanks.

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

From: Juri Linkov <juri <at> linkov.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 27897 <at> debbugs.gnu.org, drew.adams <at> oracle.com
Subject: Re: bug#27897: [PATCH] 25.1;
 Add REGION-NONCONTIGUOUS-P arg to other replace.el commands
Date: Sat, 21 Apr 2018 22:59:11 +0300

>> Thanks, your patch pushed to master as 75a32f4.
>
> I think this needs to be reflected ion NEWS, and perhaps also in the
> manuals.

query-replace-regexp-eval and map-query-replace-regexp are obscure commands
that don't need to be documented and mentioned in NEWS.  As for the commands
replace-string and replace-regexp, their other arguments also are
not documented.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: 27897 <at> debbugs.gnu.org, drew.adams <at> oracle.com
Subject: Re: bug#27897: [PATCH] 25.1;
 Add REGION-NONCONTIGUOUS-P arg to other replace.el commands
Date: Sun, 22 Apr 2018 05:36:15 +0300

> From: Juri Linkov <juri <at> linkov.net>
> Cc: 27897 <at> debbugs.gnu.org,  drew.adams <at> oracle.com
> Date: Sat, 21 Apr 2018 22:59:11 +0300
> 
> >> Thanks, your patch pushed to master as 75a32f4.
> >
> > I think this needs to be reflected ion NEWS, and perhaps also in the
> > manuals.
> 
> query-replace-regexp-eval and map-query-replace-regexp are obscure commands
> that don't need to be documented and mentioned in NEWS.  As for the commands
> replace-string and replace-regexp, their other arguments also are
> not documented.

The user-visible changes should be in NEWS.  If you think the manuals
don't need updates on their behalf, mark the NEWS entry with "---".

Thanks.

Previous Next

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