bug#55768: 26.3; isearch highlighting in dired

bug#55768: 26.3; isearch highlighting in dired

[Thierry EMERY]

[-- Attachment #1.1: Type: text/plain, Size: 959 bytes --]

Hello,

isearch fails to highlight in dired and displays the following error:
[image: image.png]

As dired sets text property 'invisible to a symbol, either:
'dired-hide-details-detail, 'dired-hide-details-information or
'dired-hide-details-link, in order for isearch to properly highlight the
searched string in a dired buffer, the following patch ought to be applied
to isearch-range-invisible:
diff -u isearch.el~ isearch.el
--- isearch.el~ 2019-09-10 02:32:49.000000000 +0200
+++ isearch.el 2022-06-02 17:13:55.490492201 +0200
@@ -2972,7 +2972,7 @@
  ;; skip all characters with that same `invisible' property value.
  ;; Do that over and over.
  (while (and (< (point) end) (invisible-p (point)))
-  (if (invisible-p (get-text-property (point) 'invisible))
+  (if (member (get-text-property (point) 'invisible)
buffer-invisibility-spec)
       (progn
  (goto-char (next-single-property-change (point) 'invisible
  nil end))

Kind Regards,

Thierry Emery

[-- Attachment #1.2: Type: text/html, Size: 1238 bytes --]

[-- Attachment #2: image.png --]
[-- Type: image/png, Size: 2580 bytes --]

bug#55768: 26.3; isearch highlighting in dired

[Lars Ingebrigtsen]

Thierry EMERY <thierryalemery@gmail.com> writes:

> isearch fails to highlight in dired and displays the following error:
> image.png

[...]

>   (while (and (< (point) end) (invisible-p (point)))
> -  (if (invisible-p (get-text-property (point) 'invisible))
> +  (if (member (get-text-property (point) 'invisible) buffer-invisibility-spec)

Hm...  shouldn't this just be

> -  (if (invisible-p (point))

?  The previous change was added by Juri; added to the CCs.

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

bug#55768: 26.3; isearch highlighting in dired

[Thierry EMERY]

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

(invisible-p (point)) has already been tested in the previous line, but it
only tests that there is a text property, not actual invisibility:
(defun invisible-p (pos)
  (get-text-property pos 'invisible))

If the text property `invisible' has another symbol than t as value and
`buffer-invisibility-spec' is a list (which is the case in dired, see
below), then the text will only be invisible if that symbol is currently
part of `buffer-invisibility-spec' (and that is used by
`dired-hide-details-mode').

buffer-invisibility-spec is a variable defined in ‘C source code’.
Its value is (t)
Local in buffer lisp; global value is t

  Automatically becomes buffer-local when set.

Documentation:
Invisibility spec of this buffer.
The default is t, which means that text is invisible if it has a non-nil
‘invisible’ property.
This variable can also be a list.  The list can have two kinds of elements:
‘ATOM’ and ‘(ATOM . ELLIPSIS)’.  A text character is invisible if its
‘invisible’ property is ‘ATOM’, or has an ‘invisible’ property that is a
list
that contains ‘ATOM’.
If the ‘(ATOM . ELLIPSIS)’ form is used, and ‘ELLIPSIS’ is non-nil, an
ellipsis will be displayed after the invisible characters.
Setting this variable is very fast, much faster than scanning all the text
in
the buffer looking for properties to change.

[-- Attachment #2: Type: text/html, Size: 1536 bytes --]

bug#55768: 26.3; isearch highlighting in dired

[Thierry EMERY]

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

Reading the documentation for `buffer-invisibility-spec' more thouroughly,
it now seems to me that a better patch would be:
diff -u isearch.el~ isearch.el
--- isearch.el~ 2019-09-10 02:32:49.000000000 +0200
+++ isearch.el 2022-06-03 09:40:59.991883469 +0200
@@ -2972,7 +2972,8 @@
  ;; skip all characters with that same `invisible' property value.
  ;; Do that over and over.
  (while (and (< (point) end) (invisible-p (point)))
-  (if (invisible-p (get-text-property (point) 'invisible))
+  (if (or (memq (get-text-property (point) 'invisible)
buffer-invisibility-spec)
+  (assq (get-text-property (point) 'invisible) buffer-invisibility-spec))
       (progn
  (goto-char (next-single-property-change (point) 'invisible
  nil end))


Le ven. 3 juin 2022 à 09:23, Thierry EMERY <thierryalemery@gmail.com> a
écrit :

> (invisible-p (point)) has already been tested in the previous line, but it
> only tests that there is a text property, not actual invisibility:
> (defun invisible-p (pos)
>   (get-text-property pos 'invisible))
>
> If the text property `invisible' has another symbol than t as value and
> `buffer-invisibility-spec' is a list (which is the case in dired, see
> below), then the text will only be invisible if that symbol is currently
> part of `buffer-invisibility-spec' (and that is used by
> `dired-hide-details-mode').
>
> buffer-invisibility-spec is a variable defined in ‘C source code’.
> Its value is (t)
> Local in buffer lisp; global value is t
>
>   Automatically becomes buffer-local when set.
>
> Documentation:
> Invisibility spec of this buffer.
> The default is t, which means that text is invisible if it has a non-nil
> ‘invisible’ property.
> This variable can also be a list.  The list can have two kinds of elements:
> ‘ATOM’ and ‘(ATOM . ELLIPSIS)’.  A text character is invisible if its
> ‘invisible’ property is ‘ATOM’, or has an ‘invisible’ property that is a
> list
> that contains ‘ATOM’.
> If the ‘(ATOM . ELLIPSIS)’ form is used, and ‘ELLIPSIS’ is non-nil, an
> ellipsis will be displayed after the invisible characters.
> Setting this variable is very fast, much faster than scanning all the text
> in
> the buffer looking for properties to change.
>
>

[-- Attachment #2: Type: text/html, Size: 2749 bytes --]

bug#55768: 26.3; isearch highlighting in dired

[Juri Linkov]

>> isearch fails to highlight in dired and displays the following error:
>> image.png
>
> [...]
>
>>   (while (and (< (point) end) (invisible-p (point)))
>> -  (if (invisible-p (get-text-property (point) 'invisible))
>> +  (if (member (get-text-property (point) 'invisible) buffer-invisibility-spec)
>
> Hm...  shouldn't this just be
>
>> -  (if (invisible-p (point))
>
> ?  The previous change was added by Juri; added to the CCs.

Actually, I just reverted to the previous version.
But the original change was 66e2e71d556785cd10270931c6fc0424b9dea6a6
from bug#8721.

bug#55768: 26.3; isearch highlighting in dired

[Lars Ingebrigtsen]

Thierry EMERY <thierryalemery@gmail.com> writes:

> (invisible-p (point)) has already been tested in the previous line, but it only tests that
> there is a text property, not actual invisibility:
> (defun invisible-p (pos)
>   (get-text-property pos 'invisible))

That's not the definition of invisible-p.  It's

DEFUN ("invisible-p", Finvisible_p, Sinvisible_p, 1, 1, 0,
       doc: /* Non-nil if text properties at POS cause text there to be currently invisible.

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

bug#55768: 26.3; isearch highlighting in dired

[Juri Linkov]

>> (invisible-p (point)) has already been tested in the previous line, but it only tests that
>> there is a text property, not actual invisibility:
>> (defun invisible-p (pos)
>>   (get-text-property pos 'invisible))
>
> That's not the definition of invisible-p.  It's
>
> DEFUN ("invisible-p", Finvisible_p, Sinvisible_p, 1, 1, 0,
>        doc: /* Non-nil if text properties at POS cause text there to be currently invisible.

I checked this again, and I see nothing wrong in the current implementation:

  (invisible-p (get-text-property (point) 'invisible))

is a valid call, because the arg of ‘invisible-p’ can be the actual value
of the ‘invisible’ text property.

bug#55768: 26.3; isearch highlighting in dired

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

>>> (invisible-p (point)) has already been tested in the previous line,
>>> but it only tests that
>>> there is a text property, not actual invisibility:
>>> (defun invisible-p (pos)
>>>   (get-text-property pos 'invisible))
>>
>> That's not the definition of invisible-p.  It's
>>
>> DEFUN ("invisible-p", Finvisible_p, Sinvisible_p, 1, 1, 0,
>>        doc: /* Non-nil if text properties at POS cause text there to
>> be currently invisible.
>
> I checked this again, and I see nothing wrong in the current implementation:
>
>   (invisible-p (get-text-property (point) 'invisible))
>
> is a valid call, because the arg of ‘invisible-p’ can be the actual value
> of the ‘invisible’ text property.

The claim was that invisible-p:

>>> it only tests that there is a text property, not actual
>>> invisibility

and then there's a definition of invisible-p, which indeed only tests
that.  I pointed out that that's not what invisible-p is defined as, or
what it does.

In other words, this:

	;; If the following character is currently invisible,
	;; skip all characters with that same `invisible' property value.
	;; Do that over and over.
	(while (and (< (point) end) (invisible-p (point)))
	  (if (invisible-p (get-text-property (point) 'invisible))

Is fine, but

(eq (invisible-p (point))
    (invisible-p (get-text-property (point) 'invisible)))

is always true.

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

bug#55768: 26.3; isearch highlighting in dired

[Juri Linkov]

> In other words, this:
>
> 	;; If the following character is currently invisible,
> 	;; skip all characters with that same `invisible' property value.
> 	;; Do that over and over.
> 	(while (and (< (point) end) (invisible-p (point)))
> 	  (if (invisible-p (get-text-property (point) 'invisible))
>
> Is fine, but
>
> (eq (invisible-p (point))
>     (invisible-p (get-text-property (point) 'invisible)))
>
> is always true.

I don't see how it's always true:

1. (invisible-p (point))
   checks invisibility of both: text properties and overlays.

2. (invisible-p (get-text-property (point) 'invisible))
   checks invisibility of text properties only.

3. (invisible-p (overlay-get o 'invisible))
   checks invisibility of overlays only.

There is a need for a separate check because overlays
can be opened, whereas text properties can't.

bug#55768: 26.3; isearch highlighting in dired

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

>> 	;; If the following character is currently invisible,
>> 	;; skip all characters with that same `invisible' property value.
>> 	;; Do that over and over.
>> 	(while (and (< (point) end) (invisible-p (point)))
>> 	  (if (invisible-p (get-text-property (point) 'invisible))

[...]

> 1. (invisible-p (point))
>    checks invisibility of both: text properties and overlays.

That's true.  Was that the intention here -- first to check invisibility
of both text properties and overlays and then only check invisibility of
text properties?

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

bug#55768: 26.3; isearch highlighting in dired

[Juri Linkov]

>>> 	;; If the following character is currently invisible,
>>> 	;; skip all characters with that same `invisible' property value.
>>> 	;; Do that over and over.
>>> 	(while (and (< (point) end) (invisible-p (point)))
>>> 	  (if (invisible-p (get-text-property (point) 'invisible))
>
> [...]
>
>> 1. (invisible-p (point))
>>    checks invisibility of both: text properties and overlays.
>
> That's true.  Was that the intention here -- first to check invisibility
> of both text properties and overlays and then only check invisibility of
> text properties?

Yep, that is my understanding too: first check both, then each separately.

bug#55768: 26.3; isearch highlighting in dired

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

>> That's true.  Was that the intention here -- first to check invisibility
>> of both text properties and overlays and then only check invisibility of
>> text properties?
>
> Yep, that is my understanding too: first check both, then each separately.

Right.

Anyway, back to the original issue -- Thierry, you reported this problem
for Emacs 26.3, but do you see it in more recent Emacs versions?

If so, can you give a complete recipe to reproduce it, starting from
"emacs -Q"?

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

bug#55768: 26.3; isearch highlighting in dired

[Thierry EMERY]

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

Hello,

Good news: the bug that i had in Emacs 26.3 is not present in 28.1.
With Emacs 28.1 compiled from source and started with "src/emacs -Q" i can
use incremental search (e.g. in dired) including for visible text (found)
or invisible text (not found), and highlighting is correct in all cases.
Also i see in Emacs 28.1 sources that `invisible-p' has been rewritten (in
C), and is more thorough than in 26.3 :-)

Thanks,

Thierry


Le jeu. 30 juin 2022 à 11:31, Lars Ingebrigtsen <larsi@gnus.org> a écrit :

> Juri Linkov <juri@linkov.net> writes:
>
> >> That's true.  Was that the intention here -- first to check invisibility
> >> of both text properties and overlays and then only check invisibility of
> >> text properties?
> >
> > Yep, that is my understanding too: first check both, then each
> separately.
>
> Right.
>
> Anyway, back to the original issue -- Thierry, you reported this problem
> for Emacs 26.3, but do you see it in more recent Emacs versions?
>
> If so, can you give a complete recipe to reproduce it, starting from
> "emacs -Q"?
>
> --
> (domestic pets only, the antidote for overdose, milk.)
>    bloggy blog: http://lars.ingebrigtsen.no
>

[-- Attachment #2: Type: text/html, Size: 1746 bytes --]

bug#55768: 26.3; isearch highlighting in dired

[Lars Ingebrigtsen]

Thierry EMERY <thierryalemery@gmail.com> writes:

> Good news: the bug that i had in Emacs 26.3 is not present in 28.1.
> With Emacs 28.1 compiled from source and started with "src/emacs -Q" i can use
> incremental search (e.g. in dired) including for visible text (found) or invisible text
> (not found), and highlighting is correct in all cases.

Thanks for checking; I'm closing this bug report, then.

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