GNU bug report logs - #49842
re-builder restriction to region (lisp/emacs-lisp/re-builder)

Previous Next

Package: emacs;

Reported by: Karthik Chikmagalur <karthikchikmagalur <at> gmail.com>

Date: Tue, 3 Aug 2021 04:04:02 UTC

Severity: wishlist

Tags: moreinfo

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

Bug is archived. No further changes may be made.

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

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

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#49842; Package emacs. (Tue, 03 Aug 2021 04:04:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Karthik Chikmagalur <karthikchikmagalur <at> gmail.com>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Tue, 03 Aug 2021 04:04:02 GMT) Full text and rfc822 format available.

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

From: Karthik Chikmagalur <karthikchikmagalur <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: re-builder restriction to region (lisp/emacs-lisp/re-builder)
Date: Mon, 02 Aug 2021 21:03:31 -0700
[Message part 1 (text/plain, inline)]
Hello,

The function "reb-update-overlays" (in lisp/emacs-lisp/re-builder.el)
displays matches in the whole of reb-target-buffer starting at
(point-min) even when the region is active. In keeping with the behavior
of commands like query-replace, replace-regexp and query-replace-regexp,
I modified it so that when the region is active re-builder only displays
matches in the active region. It's behavior is unchanged when the region is
inactive.

I think this change makes sense without disrupting the expectations of
re-builder users.

More generally, I think re-builder needs some work:

1. When the user quits re-builder, the point in reb-target-buffer is not
restored correctly.

2. Without an active region, there should be an option to match forward
or backward from reb-target-buffer's point instead of always matching
from (point-min), with the ability to customize the default behavior.

3. re-builder's overlay system (or some modification of it) can be used
by query-replace-regexp to show matches as the user types.

I am waiting to sign the copyright papers from FSF (which I have
applied for) before sending in these larger patches.

Commit log entry:

* lisp/emacs-lisp/re-builder.el (reb-update-overlays): Restrict
  re-builder matches to region when the region is active.
  
Karthik Chikmagalur
[re-builder.patch (text/x-patch, inline)]
diff -u emacs-src/lisp/emacs-lisp/re-builder.el emacs-src/lisp/emacs-lisp/re-builder-new.el
--- /lisp/emacs-lisp/re-builder.el	2021-08-02 20:47:39.226669281 -0700
+++ /lisp/emacs-lisp/re-builder-new.el	2021-08-02 20:37:27.442020958 -0700
@@ -642,16 +642,19 @@
 	 (submatches 0)
 	 firstmatch
          here
+         start end
          firstmatch-after-here)
     (with-current-buffer reb-target-buffer
         (setq here
               (if reb-target-window
                   (with-selected-window reb-target-window (window-point))
-                (point)))
+                (point))
+              start (if (region-active-p) (region-beginning) (point-min))
+              end   (if (region-active-p) (region-end) (point-max)))
       (reb-delete-overlays)
-      (goto-char (point-min))
+      (goto-char start)
       (while (and (not (eobp))
-		  (re-search-forward re (point-max) t)
+		  (re-search-forward re end t)
 		  (or (not reb-auto-match-limit)
 		      (< matches reb-auto-match-limit)))
 	(when (and (= 0 (length (match-string 0)))

Diff finished.  Mon Aug  2 20:48:01 2021

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49842; Package emacs. (Wed, 04 Aug 2021 01:30:02 GMT) Full text and rfc822 format available.

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

From: Karthik Chikmagalur <karthikchikmagalur <at> gmail.com>
To: 49842 <at> debbugs.gnu.org
Subject: re-builder restriction to region
Date: Tue, 03 Aug 2021 18:28:47 -0700
Hello,

Please disregard this patch. With further testing this does not behave as expected.

re-builder moves the point around in reb-target-buffer with each search, so (region-beginning) has a different meaning every time reb-update-overlays is called. As a result, this fails when, for example, we type in a regexp (in the re-builder buffer), then delete it and type in a new regexp. The new regexp is matched from the location of the first match of the old regexp instead of from the beginning of the region as originally specified by the user.

IIRC, the right way to restrict this to the region would be to define a variable to hold the bounds of the region as specified by the user before starting re-builder and re-search-forward inside those bounds. Perhaps a closure that's tied to the specific re-builder session can be used to avoid issues with the value of this variable when running multiple simultaneous re-builder sessions.

Karthik




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49842; Package emacs. (Wed, 04 Aug 2021 07:30:02 GMT) Full text and rfc822 format available.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Karthik Chikmagalur <karthikchikmagalur <at> gmail.com>
Cc: 49842 <at> debbugs.gnu.org
Subject: Re: bug#49842: re-builder restriction to region
 (lisp/emacs-lisp/re-builder)
Date: Wed, 04 Aug 2021 09:29:44 +0200
Karthik Chikmagalur <karthikchikmagalur <at> gmail.com> writes:

> IIRC, the right way to restrict this to the region would be to define
> a variable to hold the bounds of the region as specified by the user
> before starting re-builder and re-search-forward inside those
> bounds. Perhaps a closure that's tied to the specific re-builder
> session can be used to avoid issues with the value of this variable
> when running multiple simultaneous re-builder sessions.

I'm not very familiar with re-builder.el myself, but I think that sounds
like a promising approach.

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




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49842; Package emacs. (Wed, 04 Aug 2021 21:00:02 GMT) Full text and rfc822 format available.

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

From: Karthik Chikmagalur <karthikchikmagalur <at> gmail.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 49842 <at> debbugs.gnu.org
Subject: Re: bug#49842: re-builder restriction to region
 (lisp/emacs-lisp/re-builder)
Date: Wed, 04 Aug 2021 13:59:26 -0700
I'm working on a few improvements to re-builder including the above, and need to store some state information (bounds of region, etc) that persists until the re-builder session is closed. I see two ways to do this:

1. With a buffer-local variable in reb-target-buffer, with the assumption that only one re-builder session can be run per buffer.

2. With a lexical closure in the re-builder update code.

I'm not very familiar with elisp best practices, is there a reason to prefer one over the other beyond the direct effect on the re-builder code logic/complexity?

Karthik

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

> Karthik Chikmagalur <karthikchikmagalur <at> gmail.com> writes:
>
>> IIRC, the right way to restrict this to the region would be to define
>> a variable to hold the bounds of the region as specified by the user
>> before starting re-builder and re-search-forward inside those
>> bounds. Perhaps a closure that's tied to the specific re-builder
>> session can be used to avoid issues with the value of this variable
>> when running multiple simultaneous re-builder sessions.
>
> I'm not very familiar with re-builder.el myself, but I think that sounds
> like a promising approach.
>
> -- 
> (domestic pets only, the antidote for overdose, milk.)
>    bloggy blog: http://lars.ingebrigtsen.no




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49842; Package emacs. (Thu, 05 Aug 2021 10:59:02 GMT) Full text and rfc822 format available.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Karthik Chikmagalur <karthikchikmagalur <at> gmail.com>
Cc: 49842 <at> debbugs.gnu.org
Subject: Re: bug#49842: re-builder restriction to region
 (lisp/emacs-lisp/re-builder)
Date: Thu, 05 Aug 2021 12:57:58 +0200
Karthik Chikmagalur <karthikchikmagalur <at> gmail.com> writes:

> I'm working on a few improvements to re-builder including the above,
> and need to store some state information (bounds of region, etc) that
> persists until the re-builder session is closed. I see two ways to do
> this:
>
> 1. With a buffer-local variable in reb-target-buffer, with the
> assumption that only one re-builder session can be run per buffer.
>
> 2. With a lexical closure in the re-builder update code.
>
> I'm not very familiar with elisp best practices, is there a reason to
> prefer one over the other beyond the direct effect on the re-builder
> code logic/complexity?

Traditionally, Emacs Lisp didn't have lexical binding, so people usually
stashed the data in buffer-local values.  Now that all of the in-tree
code uses lexical binding, I think using closures usually gives more
readable and maintainable code.  But it's up to the person implementing
it what they prefer, or what makes most sense to them in that particular
case, really.  (Sometimes using buffer-local values is much more
convenient, and sometimes closures are.)

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




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49842; Package emacs. (Mon, 22 Aug 2022 11:03:02 GMT) Full text and rfc822 format available.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Karthik Chikmagalur <karthikchikmagalur <at> gmail.com>
Cc: 49842 <at> debbugs.gnu.org
Subject: Re: bug#49842: re-builder restriction to region
 (lisp/emacs-lisp/re-builder)
Date: Mon, 22 Aug 2022 13:01:51 +0200
Karthik Chikmagalur <karthikchikmagalur <at> gmail.com> writes:

> I'm working on a few improvements to re-builder including the above,
> and need to store some state information (bounds of region, etc) that
> persists until the re-builder session is closed.

This was a year ago -- did you make any further progress here?




Added tag(s) moreinfo. Request was from Lars Ingebrigtsen <larsi <at> gnus.org> to control <at> debbugs.gnu.org. (Mon, 22 Aug 2022 11:03:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49842; Package emacs. (Mon, 19 Sep 2022 19:23:01 GMT) Full text and rfc822 format available.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Karthik Chikmagalur <karthikchikmagalur <at> gmail.com>
Cc: 49842 <at> debbugs.gnu.org
Subject: Re: bug#49842: re-builder restriction to region
 (lisp/emacs-lisp/re-builder)
Date: Mon, 19 Sep 2022 21:22:38 +0200
Lars Ingebrigtsen <larsi <at> gnus.org> writes:

>> I'm working on a few improvements to re-builder including the above,
>> and need to store some state information (bounds of region, etc) that
>> persists until the re-builder session is closed.
>
> This was a year ago -- did you make any further progress here?

And this was a month ago, so I'm guessing there isn't going to be any
further progress in this bug report, so I'm closing it.  If further
progress can be made, please respond to the debbugs address and we'll
reopen.




bug closed, send any further explanations to 49842 <at> debbugs.gnu.org and Karthik Chikmagalur <karthikchikmagalur <at> gmail.com> Request was from Lars Ingebrigtsen <larsi <at> gnus.org> to control <at> debbugs.gnu.org. (Mon, 19 Sep 2022 19:23:02 GMT) Full text and rfc822 format available.

bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Tue, 18 Oct 2022 11:24:08 GMT) Full text and rfc822 format available.

This bug report was last modified 1 year and 162 days ago.

Previous Next


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