GNU bug report logs - #49534
26.3; Isearch should support using filter predicates with empty search hits

Previous Next

Package: emacs;

Reported by: Drew Adams <drew.adams <at> oracle.com>

Date: Mon, 12 Jul 2021 14:35:02 UTC

Severity: normal

Tags: fixed

Found in version 26.3

Fixed in version 28.0.50

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

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 49534 in the body.
You can then email your comments to 49534 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#49534; Package emacs. (Mon, 12 Jul 2021 14:35:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Drew Adams <drew.adams <at> oracle.com>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Mon, 12 Jul 2021 14:35:02 GMT) Full text and rfc822 format available.

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" <bug-gnu-emacs <at> gnu.org>
Subject: 26.3; Isearch should support using filter predicates with empty
 search hits
Date: Mon, 12 Jul 2021 14:34:29 +0000
Consider regep-searching for just $.  No problem; search goes to every
line end.

Now consider wanting to use a filter predicate that allows search hits
only for lines that are at least 80 chars long, e.g.:

 (defun line>79-p (beg end)
   "Return non-nil if END is past column 79."
   (save-excursion
     (goto-char end)
     (> (current-column) 79)))

 (defun foo ()
   (interactive)
   (let ((isearch-filter-predicate  #'line>79-p))
     (isearch-forward 4)))

That won't work.  Isearch is unnecessarily restrictive because of this
test in `isearch-search', which if non-nil prevents invoking
`isearch-filter-predicate'.

 (= (match-beginning 0) (match-end 0))

I've fixed this in my own code (isearch+.el).  But there are now many
differences between vanilla isearch.el and my code, so it's better if
you fix the vanilla code for this bug.

The fix should be pretty straightforward, but if you have trouble let me
know.  You can anyway see my fix here:

  https://www.emacswiki.org/emacs/download/isearch%2b.el


In GNU Emacs 26.3 (build 1, x86_64-w64-mingw32)
 of 2019-08-29
Repository revision: 96dd0196c28bc36779584e47fffcca433c9309cd
Windowing system distributor `Microsoft Corp.', version 10.0.19042
Configured using:
 `configure --without-dbus --host=x86_64-w64-mingw32
 --without-compress-install 'CFLAGS=-O2 -static -g3''





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49534; Package emacs. (Tue, 13 Jul 2021 16:28:01 GMT) Full text and rfc822 format available.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: 49534 <at> debbugs.gnu.org, "Richard M. Stallman" <rms <at> gnu.org>
Subject: Re: bug#49534: 26.3; Isearch should support using filter predicates
 with empty search hits
Date: Tue, 13 Jul 2021 18:27:32 +0200
Drew Adams <drew.adams <at> oracle.com> writes:

> That won't work.  Isearch is unnecessarily restrictive because of this
> test in `isearch-search', which if non-nil prevents invoking
> `isearch-filter-predicate'.
>
>  (= (match-beginning 0) (match-end 0))

The code is:

	(while retry
	  (setq isearch-success
		(isearch-search-string isearch-string nil t))
	  ;; Clear RETRY unless the search predicate says
	  ;; to skip this search hit.
	  (if (or (not isearch-success)
		  (bobp) (eobp)
		  (= (match-beginning 0) (match-end 0))
		  (funcall isearch-filter-predicate
			   (match-beginning 0) (match-end 0)))
	      (setq retry nil)))

I've tried following the logic here, but I'm not sure why we never retry
if the match is zero length.  It looks like this was introduced here:

commit 86bfaffe409c6f7398bcf2144fa8d9f436bd4002
Author:     Richard M. Stallman <rms <at> gnu.org>
AuthorDate: Mon Feb 10 09:41:31 1997 +0000

    (isearch-search): Refuse to match invisible text.
    (isearch-range-invisible): New function.
    (search-invisible): New user option.

And, indeed, the default predicate doesn't match on invisible text...
but I'm not sure why it's also testing the length of the match.  (The
default predicate also checks this, so removing the test seems to
produce identical results by default.)

So I've added Richard to the CCs -- Richard, you don't happen to recall
the meaning of that apparently duplicated test in a patch that's only 24
years old?

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




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49534; Package emacs. (Tue, 13 Jul 2021 19:28:02 GMT) Full text and rfc822 format available.

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

From: Juri Linkov <juri <at> linkov.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 49534 <at> debbugs.gnu.org, "Richard M.
 Stallman" <rms <at> gnu.org>, Drew Adams <drew.adams <at> oracle.com>
Subject: Re: bug#49534: 26.3; Isearch should support using filter predicates
 with empty search hits
Date: Tue, 13 Jul 2021 22:25:36 +0300
tags 49534 fixed
close 49534 28.0.50
quit

>> That won't work.  Isearch is unnecessarily restrictive because of this
>> test in `isearch-search', which if non-nil prevents invoking
>> `isearch-filter-predicate'.
>>
>>  (= (match-beginning 0) (match-end 0))
>
> And, indeed, the default predicate doesn't match on invisible text...
> but I'm not sure why it's also testing the length of the match.  (The
> default predicate also checks this, so removing the test seems to
> produce identical results by default.)

I've tried to remove (= (match-beginning 0) (match-end 0))
and then tried the test case provided by Drew,
but it goes into an infinite loop.

So it requires advancing by 1 char - the same trick as it's used
in query-replace, etc.

Also I noticed that Drew's test case matches at 'eob'
that is wrong.  So I moved checks for bobp/eobp outside.

The third problem I noticed thanks to Drew's test case
is that lazy-highlighting incorrectly highlights empty matches.
Fixed as well.




Added tag(s) fixed. Request was from Juri Linkov <juri <at> linkov.net> to control <at> debbugs.gnu.org. (Tue, 13 Jul 2021 19:28:02 GMT) Full text and rfc822 format available.

bug marked as fixed in version 28.0.50, send any further explanations to 49534 <at> debbugs.gnu.org and Drew Adams <drew.adams <at> oracle.com> Request was from Juri Linkov <juri <at> linkov.net> to control <at> debbugs.gnu.org. (Tue, 13 Jul 2021 19:28:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49534; Package emacs. (Tue, 13 Jul 2021 20:22:01 GMT) Full text and rfc822 format available.

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

From: Juri Linkov <juri <at> linkov.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 49534 <at> debbugs.gnu.org
Subject: Re: bug#49534: 26.3; Isearch should support using filter predicates
 with empty search hits
Date: Tue, 13 Jul 2021 23:20:42 +0300
>>>  (= (match-beginning 0) (match-end 0))
>>
>> And, indeed, the default predicate doesn't match on invisible text...
>> but I'm not sure why it's also testing the length of the match.  (The
>> default predicate also checks this, so removing the test seems to
>> produce identical results by default.)
>
> I've tried to remove (= (match-beginning 0) (match-end 0))
> and then tried the test case provided by Drew,
> but it goes into an infinite loop.
>
> So it requires advancing by 1 char - the same trick as it's used
> in query-replace, etc.

Oh, I realized that

  (= (match-beginning 0) (match-end 0))

was moved below after

  (funcall isearch-filter-predicate
           (match-beginning 0) (match-end 0))

This means that if isearch-filter-predicate does own matching,
it will break later (= (match-beginning 0) (match-end 0)).

What would be better: to remember its result in a let-bound variable,
or to use save-match-data?  Probably, save-match-data:

diff --git a/lisp/isearch.el b/lisp/isearch.el
index 3337d9be68..9113e94c3b 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -3591,8 +3591,9 @@ isearch-search
 	  ;; Clear RETRY unless the search predicate says
 	  ;; to skip this search hit.
 	  (if (or (not isearch-success)
-		  (funcall isearch-filter-predicate
-			   (match-beginning 0) (match-end 0)))
+		  (save-match-data
+                    (funcall isearch-filter-predicate
+			     (match-beginning 0) (match-end 0))))
 	      (setq retry nil)
 	    ;; Advance point on empty matches before retrying
 	    (when (= (match-beginning 0) (match-end 0))




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49534; Package emacs. (Tue, 13 Jul 2021 20:42:02 GMT) Full text and rfc822 format available.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: 49534 <at> debbugs.gnu.org
Subject: Re: bug#49534: 26.3; Isearch should support using filter predicates
 with empty search hits
Date: Tue, 13 Jul 2021 22:41:40 +0200
Juri Linkov <juri <at> linkov.net> writes:

> What would be better: to remember its result in a let-bound variable,
> or to use save-match-data?  Probably, save-match-data:

save-match-data would be pretty natural here, I think.

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




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49534; Package emacs. (Tue, 13 Jul 2021 21:43:01 GMT) Full text and rfc822 format available.

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

From: Drew Adams <drew.adams <at> oracle.com>
To: Juri Linkov <juri <at> linkov.net>, Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: "49534 <at> debbugs.gnu.org" <49534 <at> debbugs.gnu.org>
Subject: RE: [External] : bug#49534: 26.3; Isearch should support using filter
 predicates with empty search hits
Date: Tue, 13 Jul 2021 21:42:00 +0000
>   (= (match-beginning 0) (match-end 0))
> 
> was moved below after
> 
>   (funcall isearch-filter-predicate
>            (match-beginning 0) (match-end 0))
> 
> This means that if isearch-filter-predicate does own matching,
> it will break later (= (match-beginning 0) (match-end 0)).
> 
> What would be better: to remember its result in a let-bound variable,
> or to use save-match-data?  Probably, save-match-data:

Yes, that's probably the right thing; good catch.
___

[On the other hand, a filter predicate can really
do more than just act as a predicate (it can
perform useful side effects).  There could
presumably be reasons we'd want to let it change
the match data.

No, I don't have any concrete use case in mind;
and yes, without some other changes, that could
mess up code that follows using the predicate.
I just wonder about further restricting the
predicate.  The point of this bug fix was to
_remove_ an unnecessary restriction, but now
we're adding another restriction. ;-)]




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49534; Package emacs. (Tue, 13 Jul 2021 21:45:02 GMT) Full text and rfc822 format available.

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

From: Drew Adams <drew.adams <at> oracle.com>
To: Juri Linkov <juri <at> linkov.net>, Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: "49534 <at> debbugs.gnu.org" <49534 <at> debbugs.gnu.org>,
 "Richard M. Stallman" <rms <at> gnu.org>
Subject: RE: [External] : Re: bug#49534: 26.3; Isearch should support using
 filter predicates with empty search hits
Date: Tue, 13 Jul 2021 21:44:41 +0000
> I've tried to remove (= (match-beginning 0) (match-end 0))
> and then tried the test case provided by Drew,
> but it goes into an infinite loop.
> 
> So it requires advancing by 1 char - the same trick as it's used
> in query-replace, etc.

Yes, my code does that.

> Also I noticed that Drew's test case matches at 'eob'
> that is wrong.  So I moved checks for bobp/eobp outside.

What do you mean by Drew's test case?

Do you mean the updated isearch+.el definition of `isearch-search', with the bug fix?  That is, do you mean this code?

 (when (or (not isearch-success)
           (isearchp-reached-limit-p) ; <==============
           (funcall isearch-filter-predicate
                    (match-beginning 0)
                    (match-end 0)))
   (setq retry  nil))

where

(defun isearchp-reached-limit-p ()
  "Return non-nil if at search-boundary limit in current search direction."
  (if isearch-forward
      (or (eobp) ; <==================
          (and isearchp-reg-end
               (> (point) isearchp-reg-end)))
      (or (bobp) ; <==================
          (and isearchp-reg-beg
               (< (point) isearchp-reg-beg)))))

Is that the use of `eobp' you're talking about?
(If not, what is?)  That `isearchp-reached-limit-p'
just replaces the original isearch.el code, which
just tested (or (bobp) (eobp)):

(or (not isearch-success)
    (bobp) (eobp) ; <=================
    (= (match-beginning 0) (match-end 0))
    (funcall isearch-filter-predicate
             (match-beginning 0) (match-end 0))) 

My code just uses the (original) region limits
instead of the buffer limits, when searching the
active region.

Your code (in master) removes that boundary test
altogether (no `bobp' or `eobp' test).  Why is
that the right thing?  Is it because the match
should be allowed to match up to `bobp' or `eobp'?
If so, why was that test in isearch.el in the
first place?

Actually, you do still test for reaching the
boundary, but only for an empty match and after
filter failure.  Why is that?

And why do you not need to back up a char after
the loop, if the match was empty the last time
around and the next time it fails?  It'll have
advanced a char; should it stay there instead
of backing up?  (Dunno, but I supposed not.)

Our code in those spots is slightly different.
I'd like to know why you did just as you did.

> The third problem I noticed thanks to Drew's test case
> is that lazy-highlighting incorrectly highlights empty matches.
> Fixed as well.

Yes, I left that code alone for the moment, but
your change is no doubt the right thing there.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49534; Package emacs. (Tue, 13 Jul 2021 22:33:02 GMT) Full text and rfc822 format available.

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

From: Juri Linkov <juri <at> linkov.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 49534 <at> debbugs.gnu.org
Subject: Re: bug#49534: 26.3; Isearch should support using filter predicates
 with empty search hits
Date: Wed, 14 Jul 2021 01:30:18 +0300
>> What would be better: to remember its result in a let-bound variable,
>> or to use save-match-data?  Probably, save-match-data:
>
> save-match-data would be pretty natural here, I think.

So now pushed as well.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49534; Package emacs. (Tue, 13 Jul 2021 22:33:02 GMT) Full text and rfc822 format available.

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

From: Juri Linkov <juri <at> linkov.net>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>,
 "49534 <at> debbugs.gnu.org" <49534 <at> debbugs.gnu.org>
Subject: Re: [External] : bug#49534: 26.3; Isearch should support using
 filter predicates with empty search hits
Date: Wed, 14 Jul 2021 01:30:36 +0300
>> What would be better: to remember its result in a let-bound variable,
>> or to use save-match-data?  Probably, save-match-data:
>
> Yes, that's probably the right thing; good catch.
> ___
>
> [On the other hand, a filter predicate can really
> do more than just act as a predicate (it can
> perform useful side effects).  There could
> presumably be reasons we'd want to let it change
> the match data.
>
> No, I don't have any concrete use case in mind;
> and yes, without some other changes, that could
> mess up code that follows using the predicate.
> I just wonder about further restricting the
> predicate.  The point of this bug fix was to
> _remove_ an unnecessary restriction, but now
> we're adding another restriction. ;-)]

If you want to mess with the match data,
you can use own isearch-search-fun-function.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49534; Package emacs. (Tue, 13 Jul 2021 22:33:03 GMT) Full text and rfc822 format available.

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

From: Juri Linkov <juri <at> linkov.net>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>,
 "49534 <at> debbugs.gnu.org" <49534 <at> debbugs.gnu.org>,
 "Richard M. Stallman" <rms <at> gnu.org>
Subject: Re: [External] : Re: bug#49534: 26.3; Isearch should support using
 filter predicates with empty search hits
Date: Wed, 14 Jul 2021 01:31:03 +0300
> What do you mean by Drew's test case?

I meant:

 (defun line>79-p (beg end)
   "Return non-nil if END is past column 79."
   (save-excursion
     (goto-char end)
     (> (current-column) 79)))

 (defun foo ()
   (interactive)
   (let ((isearch-filter-predicate  #'line>79-p))
     (isearch-forward 4)))

> Your code (in master) removes that boundary test
> altogether (no `bobp' or `eobp' test).  Why is
> that the right thing?  Is it because the match
> should be allowed to match up to `bobp' or `eobp'?
> If so, why was that test in isearch.el in the
> first place?

If you think that you found a problem, please provide
a test that exposes it.

> Actually, you do still test for reaching the
> boundary, but only for an empty match and after
> filter failure.  Why is that?

Please provide a test case if you think there is a problem.

> And why do you not need to back up a char after
> the loop, if the match was empty the last time
> around and the next time it fails?  It'll have
> advanced a char; should it stay there instead
> of backing up?  (Dunno, but I supposed not.)

Please provide more tests that confirm your doubts.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49534; Package emacs. (Tue, 13 Jul 2021 22:49:01 GMT) Full text and rfc822 format available.

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Juri Linkov <juri <at> linkov.net>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, 49534 <at> debbugs.gnu.org
Subject: Re: bug#49534: 26.3; Isearch should support using filter predicates
 with empty search hits
Date: Tue, 13 Jul 2021 18:48:39 -0400
> This means that if isearch-filter-predicate does own matching,
> it will break later (= (match-beginning 0) (match-end 0)).
> What would be better: to remember its result in a let-bound variable,
> or to use save-match-data?  Probably, save-match-data:

Remembering the result in a var might be more efficient.
Also if `isearch-filter-predicate` clobbers the match data we have more
problems (unless it tells us to skip this match).

But whichever choice we make I think the docstring of
`isearch-filter-predicate` should clarify what happens with the match
data (i.e. whether it needs to be careful not to clobber it or not).


        Stefan





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49534; Package emacs. (Tue, 13 Jul 2021 23:02:01 GMT) Full text and rfc822 format available.

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

From: Drew Adams <drew.adams <at> oracle.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>,
 "49534 <at> debbugs.gnu.org" <49534 <at> debbugs.gnu.org>
Subject: RE: [External] : bug#49534: 26.3; Isearch should support using filter
 predicates with empty search hits
Date: Tue, 13 Jul 2021 23:00:55 +0000
> > [On the other hand, a filter predicate can really
> > do more than just act as a predicate (it can
> > perform useful side effects).  There could
> > presumably be reasons we'd want to let it change
> > the match data.
> >
> > No, I don't have any concrete use case in mind;
> > and yes, without some other changes, that could
> > mess up code that follows using the predicate.
> > I just wonder about further restricting the
> > predicate.  The point of this bug fix was to
> > _remove_ an unnecessary restriction, but now
> > we're adding another restriction. ;-)]
> 
> If you want to mess with the match data,
> you can use own isearch-search-fun-function.

No, that doesn't speak to whether a filter
predicate should be able to do that.  The
question is not about how to accomplish this
or that; it's about what a filter predicate
should be able to do, as part of its intended
effect.

I'm OK with not letting it use changing match
data as a side effect.

But in general I'm in favor of it being free
to perform arbitrary actions (whether or not
it also acts effectively as a filter).
___

See "A Filter Predicate Can Perform Side Effects":

https://www.emacswiki.org/emacs/DynamicIsearchFiltering#AFilterPredicateCanPerformSideEffects




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49534; Package emacs. (Tue, 13 Jul 2021 23:05:02 GMT) Full text and rfc822 format available.

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

From: Drew Adams <drew.adams <at> oracle.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>,
 "49534 <at> debbugs.gnu.org" <49534 <at> debbugs.gnu.org>,
 "Richard M. Stallman" <rms <at> gnu.org>
Subject: RE: [External] : Re: bug#49534: 26.3; Isearch should support using
 filter predicates with empty search hits
Date: Tue, 13 Jul 2021 23:04:34 +0000
> > Your code (in master) removes that boundary test
> > altogether (no `bobp' or `eobp' test).  Why is
> > that the right thing?  Is it because the match
> > should be allowed to match up to `bobp' or `eobp'?
> > If so, why was that test in isearch.el in the
> > first place?
> 
> If you think that you found a problem, please provide
> a test that exposes it.

I'm asking why you removed that boundary test.
And peripherally, why it was there to begin with?

> > Actually, you do still test for reaching the
> > boundary, but only for an empty match and after
> > filter failure.  Why is that?
> 
> Please provide a test case if you think there is a problem.

I'm asking why you test for reaching the boundary,
and only for an empty match and after a filter
failure.

> > And why do you not need to back up a char after
> > the loop, if the match was empty the last time
> > around and the next time it fails?  It'll have
> > advanced a char; should it stay there instead
> > of backing up?  (Dunno, but I supposed not.)
> 
> Please provide more tests that confirm your doubts.

I'm asking why you don't need to back up a char
after having advanced a char in that case.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49534; Package emacs. (Tue, 13 Jul 2021 23:23:01 GMT) Full text and rfc822 format available.

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

From: Juri Linkov <juri <at> linkov.net>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, 49534 <at> debbugs.gnu.org
Subject: Re: bug#49534: 26.3; Isearch should support using filter predicates
 with empty search hits
Date: Wed, 14 Jul 2021 02:21:54 +0300
>> This means that if isearch-filter-predicate does own matching,
>> it will break later (= (match-beginning 0) (match-end 0)).
>> What would be better: to remember its result in a let-bound variable,
>> or to use save-match-data?  Probably, save-match-data:
>
> Remembering the result in a var might be more efficient.
> Also if `isearch-filter-predicate` clobbers the match data we have more
> problems (unless it tells us to skip this match).
>
> But whichever choice we make I think the docstring of
> `isearch-filter-predicate` should clarify what happens with the match
> data (i.e. whether it needs to be careful not to clobber it or not).

Then there is no need to remember the result in a var either
since `isearch-filter-predicate` might want intentionally
force forward-char as if the original match was empty.

So the last change fixed a non-problem and now is reverted.
Also I clarified the docstring of `isearch-filter-predicate`.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Wed, 11 Aug 2021 11:24:07 GMT) Full text and rfc822 format available.

bug unarchived. Request was from Drew Adams <drew.adams <at> oracle.com> to control <at> debbugs.gnu.org. (Wed, 16 Feb 2022 04:17:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49534; Package emacs. (Wed, 16 Feb 2022 04:21:01 GMT) Full text and rfc822 format available.

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

From: Drew Adams <drew.adams <at> oracle.com>
To: Juri Linkov <juri <at> linkov.net>, "49534 <at> debbugs.gnu.org"
 <49534 <at> debbugs.gnu.org>
Subject: Re: bug#49534: 26.3; Isearch should support using filter predicates
 with empty search hits
Date: Wed, 16 Feb 2022 04:20:40 +0000
(Resending after unarchiving.)

Juri, could you please answer the questions
I asked you here?  Thx.

> > > Your code (in master) removes that boundary test
> > > altogether (no `bobp' or `eobp' test).  Why is
> > > that the right thing?  Is it because the match
> > > should be allowed to match up to `bobp' or `eobp'?
> > > If so, why was that test in isearch.el in the
> > > first place?
> >
> > If you think that you found a problem, please provide
> > a test that exposes it.
> 
> I'm asking why you removed that boundary test.
> And peripherally, why it was there to begin with?

ping.  I'd really like to know.

> > > Actually, you do still test for reaching the
> > > boundary, but only for an empty match and after
> > > filter failure.  Why is that?
> >
> > Please provide a test case if you think there is a problem.
> 
> I'm asking why you test for reaching the boundary,
> and only for an empty match and after a filter
> failure.

ping.  I'd really like to know.
 
> > > And why do you not need to back up a char after
> > > the loop, if the match was empty the last time
> > > around and the next time it fails?  It'll have
> > > advanced a char; should it stay there instead
> > > of backing up?  (Dunno, but I supposed not.)
> >
> > Please provide more tests that confirm your doubts.
> 
> I'm asking why you don't need to back up a char
> after having advanced a char in that case.

ping.  I'd really like to know.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49534; Package emacs. (Wed, 16 Feb 2022 18:45:03 GMT) Full text and rfc822 format available.

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

From: Juri Linkov <juri <at> linkov.net>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: "49534 <at> debbugs.gnu.org" <49534 <at> debbugs.gnu.org>
Subject: Re: bug#49534: 26.3; Isearch should support using filter predicates
 with empty search hits
Date: Wed, 16 Feb 2022 20:20:30 +0200
>> > > Your code (in master) removes that boundary test
>> > > altogether (no `bobp' or `eobp' test).  Why is
>> > > that the right thing?  Is it because the match
>> > > should be allowed to match up to `bobp' or `eobp'?
>> > > If so, why was that test in isearch.el in the
>> > > first place?
>> >
>> > If you think that you found a problem, please provide
>> > a test that exposes it.
>>
>> I'm asking why you removed that boundary test.
>> And peripherally, why it was there to begin with?
>
> ping.  I'd really like to know.
>
>> > > Actually, you do still test for reaching the
>> > > boundary, but only for an empty match and after
>> > > filter failure.  Why is that?
>> >
>> > Please provide a test case if you think there is a problem.
>>
>> I'm asking why you test for reaching the boundary,
>> and only for an empty match and after a filter
>> failure.
>
> ping.  I'd really like to know.
>
>> > > And why do you not need to back up a char after
>> > > the loop, if the match was empty the last time
>> > > around and the next time it fails?  It'll have
>> > > advanced a char; should it stay there instead
>> > > of backing up?  (Dunno, but I supposed not.)
>> >
>> > Please provide more tests that confirm your doubts.
>>
>> I'm asking why you don't need to back up a char
>> after having advanced a char in that case.
>
> ping.  I'd really like to know.

All these changes fixed the test case that you presented in the bug report.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49534; Package emacs. (Wed, 16 Feb 2022 18:55:01 GMT) Full text and rfc822 format available.

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

From: Drew Adams <drew.adams <at> oracle.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: "49534 <at> debbugs.gnu.org" <49534 <at> debbugs.gnu.org>
Subject: RE: [External] : Re: bug#49534: 26.3; Isearch should support using
 filter predicates with empty search hits
Date: Wed, 16 Feb 2022 18:54:49 +0000
> All these changes fixed the test case that you
> presented in the bug report.

Yes, I understand that.  You've said that.  
But that doesn't help me understand the code.

Could you please answer the specific questions
I asked about the code?  That will help me
understand what's going on.

I'm in no way arguing that your fix is in some
way incorrect or insufficient.  I'm just trying
to understand the code here.

I'd appreciate this; thx.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49534; Package emacs. (Wed, 16 Feb 2022 19:10:02 GMT) Full text and rfc822 format available.

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

From: Juri Linkov <juri <at> linkov.net>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: "49534 <at> debbugs.gnu.org" <49534 <at> debbugs.gnu.org>
Subject: Re: [External] : Re: bug#49534: 26.3; Isearch should support using
 filter predicates with empty search hits
Date: Wed, 16 Feb 2022 21:09:05 +0200
>> All these changes fixed the test case that you
>> presented in the bug report.
>
> Yes, I understand that.  You've said that.  
> But that doesn't help me understand the code.
>
> Could you please answer the specific questions
> I asked about the code?  That will help me
> understand what's going on.
>
> I'm in no way arguing that your fix is in some
> way incorrect or insufficient.  I'm just trying
> to understand the code here.
>
> I'd appreciate this; thx.

If I designed a new function from scratch then I could explain all
decisions made while implementing it.  But I only fixed a few places
in the existing function just to pass test cases that you reported.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49534; Package emacs. (Wed, 16 Feb 2022 19:18:02 GMT) Full text and rfc822 format available.

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

From: Drew Adams <drew.adams <at> oracle.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: "49534 <at> debbugs.gnu.org" <49534 <at> debbugs.gnu.org>
Subject: RE: [External] : Re: bug#49534: 26.3; Isearch should support using
 filter predicates with empty search hits
Date: Wed, 16 Feb 2022 19:17:11 +0000
> > I'm in no way arguing that your fix is in some
> > way incorrect or insufficient.  I'm just trying
> > to understand the code here.  I'd appreciate this; thx.
> 
> If I designed a new function from scratch then I could explain all
> decisions made while implementing it.  But I only fixed a few places
> in the existing function just to pass test cases that you reported.

Understood.  But you must at least understand why
you made the changes you made.  That's what I'm
asking about - those particular changes.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49534; Package emacs. (Wed, 16 Feb 2022 19:28:01 GMT) Full text and rfc822 format available.

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

From: Juri Linkov <juri <at> linkov.net>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: "49534 <at> debbugs.gnu.org" <49534 <at> debbugs.gnu.org>
Subject: Re: [External] : Re: bug#49534: 26.3; Isearch should support using
 filter predicates with empty search hits
Date: Wed, 16 Feb 2022 21:25:53 +0200
>> > I'm in no way arguing that your fix is in some
>> > way incorrect or insufficient.  I'm just trying
>> > to understand the code here.  I'd appreciate this; thx.
>>
>> If I designed a new function from scratch then I could explain all
>> decisions made while implementing it.  But I only fixed a few places
>> in the existing function just to pass test cases that you reported.
>
> Understood.  But you must at least understand why
> you made the changes you made.  That's what I'm
> asking about - those particular changes.

I made these changes because you reported a bug
and provided test cases.  So these changes correspond
to your test cases one to one.




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

This bug report was last modified 2 years and 34 days ago.

Previous Next


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