bug#40808: 27.0.91; inaccuracy in isearch-lazy-count

bug#40808: 27.0.91; inaccuracy in isearch-lazy-count

[Abdullah Asad]

When using isearch with isearch-lazy-count in org mode with heading
collapsed isearch gives wrong number of matches (both total & current).

Reproducing it is simple just enable isearch-lazy-count and perform
isearch in org buffer with collapsed heading.

In GNU Emacs 27.0.91 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.17)
of 2020-04-23 built on notArch
Windowing system distributor 'The X.Org Foundation

bug#40808: 27.0.91; inaccuracy in isearch-lazy-count

[Juri Linkov]

> When using isearch with isearch-lazy-count in org mode with heading
> collapsed isearch gives wrong number of matches (both total & current).
>
> Reproducing it is simple just enable isearch-lazy-count and perform
> isearch in org buffer with collapsed heading.

Thanks for bringing up this question, it will help to fix this feature
for the upcoming release.  I tend to agree that isearch-lazy-count
should show the number of *all* matches in the buffer, even the matches that
are invisible, and that can be opened when visited by isearch navigation.

Eli, do you agree with this patch for emacs-27.  It counts all matches, even
invisible.  And also it fixes an old bug existed in previous Emacs versions
where lazy-highlight didn't update lazy-highlighting when a hidden outline
was automatically opened at the end of the file (I noticed this bug only now).

So the condition ‘(eq search-invisible 'open)’ fixes an old bug, and
the condition ‘isearch-lazy-count’ fixes the new feature added in emacs-27:

diff --git a/lisp/isearch.el b/lisp/isearch.el
index e13a4dda83..ed1097c5ea 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -319,7 +319,7 @@ isearch-lazy-count
   "Show match numbers in the search prompt.
 When both this option and `isearch-lazy-highlight' are non-nil,
 show the current match number and the total number of matches
-in the buffer (or its restriction)."
+in the buffer (or its restriction), including all hidden matches."
   :type 'boolean
   :group 'lazy-count
   :group 'isearch
@@ -3869,7 +3871,8 @@ isearch-lazy-highlight-search
 	    (isearch-regexp-lax-whitespace
 	     isearch-lazy-highlight-regexp-lax-whitespace)
 	    (isearch-forward isearch-lazy-highlight-forward)
-	    (search-invisible nil)	; don't match invisible text
+            ;; don't match invisible text unless it can open or counting matches
+            (search-invisible (or (eq search-invisible 'open) isearch-lazy-count))
 	    (retry t)
 	    (success nil))
 	;; Use a loop like in `isearch-search'.

bug#40808: 27.0.91; inaccuracy in isearch-lazy-count

[Abdullah Asad]

> Eli, do you agree with this patch for emacs-27.
So, what is decided. Is it getting patched for emacs-27?

bug#40808: 27.0.91; inaccuracy in isearch-lazy-count

[Eli Zaretskii]

> From: Abdullah Asad <abdullah@net-c.com>
> Date: Wed, 29 Apr 2020 00:16:39 +0530
> Cc: 40808@debbugs.gnu.org
> 
> > Eli, do you agree with this patch for emacs-27.
> So, what is decided. Is it getting patched for emacs-27?

Yes, it's fine with me.  Thanks.

bug#40808: 27.0.91; inaccuracy in isearch-lazy-count

[Juri Linkov]

[-- Attachment #1: Type: text/plain, Size: 761 bytes --]

>> So, what is decided. Is it getting patched for emacs-27?
>
> Yes, it's fine with me.  Thanks.

Now pushed to emacs-27.

But there is still one corner case I'm worried about:
isearch-lazy-count still counts matches that can't be opened
and can't be visited, such as hidden links in org-mode.

To not count them we need to bind 'search-invisible' to 'open'
in isearch-lazy-highlight-search, but the problem is that
isearch-filter-predicate and isearch-range-invisible will
temporarily open them, whereas we need just to check
whether they can be opened.

So in the following patch I added a new variable isearch-check-overlays
that could instruct isearch-range-invisible to not open overlays
when it's non-nil that means we need only to check them, not open:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: isearch-check-overlays.patch --]
[-- Type: text/x-diff, Size: 1952 bytes --]

diff --git a/lisp/isearch.el b/lisp/isearch.el
index e13a4dda83..0ad97a092f 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -3535,6 +3535,7 @@ isearch-close-unnecessary-overlays
 	    (overlay-put ov 'invisible (overlay-get ov 'isearch-invisible))
 	    (overlay-put ov 'isearch-invisible nil)))))))
 
+(defvar isearch-check-overlays nil)
 
 (defun isearch-range-invisible (beg end)
   "Return t if all the text from BEG to END is invisible."
@@ -3546,7 +3547,7 @@ isearch-range-invisible
 	    (can-be-opened (eq search-invisible 'open))
 	    ;; the list of overlays that could be opened
 	    (crt-overlays nil))
-	(when (and can-be-opened isearch-hide-immediately)
+	(when (and can-be-opened isearch-hide-immediately (not isearch-check-overlays))
 	  (isearch-close-unnecessary-overlays beg end))
 	;; If the following character is currently invisible,
 	;; skip all characters with that same `invisible' property value.
@@ -3585,9 +3586,10 @@ isearch-range-invisible
 	(if (>= (point) end)
 	    (if (and can-be-opened (consp crt-overlays))
 		(progn
-		  (setq isearch-opened-overlays
-			(append isearch-opened-overlays crt-overlays))
-		  (mapc 'isearch-open-overlay-temporary crt-overlays)
+		  (unless isearch-check-overlays
+		    (setq isearch-opened-overlays
+			  (append isearch-opened-overlays crt-overlays))
+		    (mapc 'isearch-open-overlay-temporary crt-overlays))
 		  nil)
 	      (setq isearch-hidden t)))))))
 
@@ -3880,8 +3885,10 @@ isearch-lazy-highlight-search
 	  (if (or (not success)
 		  (= (point) bound) ; like (bobp) (eobp) in `isearch-search'.
 		  (= (match-beginning 0) (match-end 0))
-		  (funcall isearch-filter-predicate
-			   (match-beginning 0) (match-end 0)))
+		  (let ((search-invisible (and search-invisible 'open))
+		        (isearch-check-overlays t))
+		    (funcall isearch-filter-predicate
+			     (match-beginning 0) (match-end 0))))
 	      (setq retry nil)))
 	success)
     (error nil)))

bug#40808: 27.0.91; inaccuracy in isearch-lazy-count

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: Abdullah Asad <abdullah@net-c.com>,  40808@debbugs.gnu.org
> Date: Wed, 29 Apr 2020 02:54:06 +0300
> 
> But there is still one corner case I'm worried about:
> isearch-lazy-count still counts matches that can't be opened
> and can't be visited, such as hidden links in org-mode.
> 
> To not count them we need to bind 'search-invisible' to 'open'
> in isearch-lazy-highlight-search, but the problem is that
> isearch-filter-predicate and isearch-range-invisible will
> temporarily open them, whereas we need just to check
> whether they can be opened.
> 
> So in the following patch I added a new variable isearch-check-overlays
> that could instruct isearch-range-invisible to not open overlays
> when it's non-nil that means we need only to check them, not open:

Fine with me, but please make this change on master, not on emacs-27.

bug#40808: 27.0.91; inaccuracy in isearch-lazy-count

[Juri Linkov]

>> But there is still one corner case I'm worried about:
>> isearch-lazy-count still counts matches that can't be opened
>> and can't be visited, such as hidden links in org-mode.
>>
>> To not count them we need to bind 'search-invisible' to 'open'
>> in isearch-lazy-highlight-search, but the problem is that
>> isearch-filter-predicate and isearch-range-invisible will
>> temporarily open them, whereas we need just to check
>> whether they can be opened.
>>
>> So in the following patch I added a new variable isearch-check-overlays
>> that could instruct isearch-range-invisible to not open overlays
>> when it's non-nil that means we need only to check them, not open:
>
> Fine with me, but please make this change on master, not on emacs-27.

Right decision, because org-mode is broken anyway, and this fix won't make
it better.  A test case that demonstrates the problem in org-mode:

0. emacs -Q
1. visit emacs/etc/ORG-NEWS
2. isearch for "http": C-s http C-s C-s C-s ...

it eventually stops at org links where "http" is invisible, such as

  [[https://orgmode.org/worg/library-of-babel.html][here]]

The problem is that org-mode puts the text property 'invisible'
only when the link is scrolled into view, I guess it puts the property
using font-lock.

So at least in emacs-27 now it consistently counts all matches
ignoring their visibility.

bug#40808: 27.0.91; inaccuracy in isearch-lazy-count

[Juri Linkov]

>> Fine with me, but please make this change on master, not on emacs-27.
>
> Right decision, because org-mode is broken anyway, and this fix won't make
> it better.  A test case that demonstrates the problem in org-mode:
>
> 0. emacs -Q
> 1. visit emacs/etc/ORG-NEWS
> 2. isearch for "http": C-s http C-s C-s C-s ...
>
> it eventually stops at org links where "http" is invisible, such as
>
>   [[https://orgmode.org/worg/library-of-babel.html][here]]
>
> The problem is that org-mode puts the text property 'invisible'
> only when the link is scrolled into view, I guess it puts the property
> using font-lock.
>
> So at least in emacs-27 now it consistently counts all matches
> ignoring their visibility.

I'm still unsure about pushing this change.  The current state of things:

- in emacs-27 and master now it counts all matches, including invisible,
  regardless of the value of search-invisible;

- the proposed change for master doesn't include in the count these
  invisible matches that can't be visited by opening overlays;

- but this fix is pointless for org-mode where visiting invisible
  matches is broken, and where its count varies after visiting
  and opening hidden overlays.

bug#40808: 27.0.91; inaccuracy in isearch-lazy-count

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 40808@debbugs.gnu.org,  abdullah@net-c.com
> Date: Thu, 30 Apr 2020 23:18:08 +0300
> 
> I'm still unsure about pushing this change.  The current state of things:
> 
> - in emacs-27 and master now it counts all matches, including invisible,
>   regardless of the value of search-invisible;
> 
> - the proposed change for master doesn't include in the count these
>   invisible matches that can't be visited by opening overlays;
> 
> - but this fix is pointless for org-mode where visiting invisible
>   matches is broken, and where its count varies after visiting
>   and opening hidden overlays.

Would it be possible to fix the breakage in Org on master?

bug#40808: 27.0.91; inaccuracy in isearch-lazy-count

[Juri Linkov]

>> I'm still unsure about pushing this change.  The current state of things:
>>
>> - in emacs-27 and master now it counts all matches, including invisible,
>>   regardless of the value of search-invisible;
>>
>> - the proposed change for master doesn't include in the count these
>>   invisible matches that can't be visited by opening overlays;
>>
>> - but this fix is pointless for org-mode where visiting invisible
>>   matches is broken, and where its count varies after visiting
>>   and opening hidden overlays.
>
> Would it be possible to fix the breakage in Org on master?

We need help from Org developers to fix the breakage in Org.

Meanwhile, I reverted a part of the recent fix on emacs-27
that tried to fix a long-standing corner case of lazy-highlighting
in outlines, but whose side-effect might slow down lazy-highlighting
in collapsed outlines.  This needs more testing on master.

Whereas the fix for isearch-lazy-count still remains in the release branch.

bug#40808: Org-mode inaccuracy in isearch-lazy-count

[Juri Linkov]

Hi Bastien,

Could you please look at this problem as well:

>>> - but this fix is pointless for org-mode where visiting invisible
>>>   matches is broken, and where its count varies after visiting
>>>   and opening hidden overlays.
>>
>> Would it be possible to fix the breakage in Org on master?
>
> We need help from Org developers to fix the breakage in Org.

bug#40808: Org-mode inaccuracy in isearch-lazy-count

[Bastien]

Hi Juri,

Juri Linkov <juri@linkov.net> writes:

> Could you please look at this problem as well:

thanks for the heads up -- I will have a look at it this week.

-- 
 Bastien

bug#40808: Org-mode inaccuracy in isearch-lazy-count

[Stefan Kangas]

Hi Bastien,

Bastien <bzg@gnu.org> writes:

> Juri Linkov <juri@linkov.net> writes:
>
>> Could you please look at this problem as well:
>
> thanks for the heads up -- I will have a look at it this week.

Did you have any chance to look into this?  Thanks.