GNU bug report logs - #36486
[PATCH] Add tests for text-property-search to check prop-match-value

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 36486 in the body.
You can then email your comments to 36486 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: Stefan Kangas <stefan <at> marxist.se>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Add tests for text-property-search to check prop-match-value
Date: Wed, 3 Jul 2019 11:44:09 +0200

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

Attached is a patch with additional test cases for
text-property-search which checks the returned match object.  Unlike
older test cases, these also check the value of prop-match-value.

Thanks,
Stefan Kangas

[0001-Add-tests-for-text-property-search-to-check-prop-mat.patch (application/octet-stream, attachment)]

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 36486 <at> debbugs.gnu.org
Subject: Re: bug#36486: [PATCH] Add tests for text-property-search to check
 prop-match-value
Date: Sat, 06 Jul 2019 16:25:35 +0200

Stefan Kangas <stefan <at> marxist.se> writes:

> Attached is a patch with additional test cases for
> text-property-search which checks the returned match object.  Unlike
> older test cases, these also check the value of prop-match-value.

Looks good; I've now applied them to the trunk.

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

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

From: Juri Linkov <juri <at> linkov.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 36486 <at> debbugs.gnu.org, Stefan Kangas <stefan <at> marxist.se>
Subject: Re: bug#36486: [PATCH] Add tests for text-property-search to check
 prop-match-value
Date: Mon, 08 Jul 2019 01:59:22 +0300

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

>> Attached is a patch with additional test cases for
>> text-property-search which checks the returned match object.  Unlike
>> older test cases, these also check the value of prop-match-value.
>
> Looks good; I've now applied them to the trunk.

BTW, I tried to use text-property-search as a command interactively,
but it lacks support for reading the second argument VALUE.
Here is the patch that implements it:

[text-property-search-value.patch (text/x-diff, inline)]

diff --git a/lisp/emacs-lisp/text-property-search.el b/lisp/emacs-lisp/text-property-search.el
index 41ca07057e..5027bc406c 100644
--- a/lisp/emacs-lisp/text-property-search.el
+++ b/lisp/emacs-lisp/text-property-search.el
@@ -54,10 +54,14 @@ text-property-search-forward
 that's matching), and `prop-match-value' (the value of PROPERTY
 at the start of the region)."
   (interactive
-   (list
-    (let ((string (completing-read "Search for property: " obarray)))
-      (when (> (length string) 0)
-        (intern string obarray)))))
+   (let* ((property (completing-read "Search for property: " obarray))
+          (property (when (> (length property) 0)
+                      (intern property obarray)))
+          (value (when property
+                   (completing-read "Search for property value: " obarray)))
+          (value (when (> (length value) 0)
+                   (intern value obarray))))
+     (list property value)))
   (cond
    ;; No matches at the end of the buffer.
    ((eobp)

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: 36486 <at> debbugs.gnu.org, Stefan Kangas <stefan <at> marxist.se>
Subject: Re: bug#36486: [PATCH] Add tests for text-property-search to check
 prop-match-value
Date: Mon, 08 Jul 2019 18:18:32 +0200

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

> +   (let* ((property (completing-read "Search for property: " obarray))
> +          (property (when (> (length property) 0)
> +                      (intern property obarray)))
> +          (value (when property
> +                   (completing-read "Search for property value: " obarray)))
> +          (value (when (> (length value) 0)
> +                   (intern value obarray))))

Hm...  well, the value doesn't have to be a symbol, so I don't think
this is quite right -- it could be a string, or, well, anything.  I
think that's why it doesn't prompt for value in the interactive form,
because it didn't seem useful.

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

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

From: Juri Linkov <juri <at> linkov.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 36486 <at> debbugs.gnu.org, Stefan Kangas <stefan <at> marxist.se>
Subject: Re: bug#36486: [PATCH] Add tests for text-property-search to check
 prop-match-value
Date: Mon, 08 Jul 2019 23:51:57 +0300

>> +   (let* ((property (completing-read "Search for property: " obarray))
>> +          (property (when (> (length property) 0)
>> +                      (intern property obarray)))
>> +          (value (when property
>> +                   (completing-read "Search for property value: " obarray)))
>> +          (value (when (> (length value) 0)
>> +                   (intern value obarray))))
>
> Hm...  well, the value doesn't have to be a symbol, so I don't think
> this is quite right -- it could be a string, or, well, anything.  I
> think that's why it doesn't prompt for value in the interactive form,
> because it didn't seem useful.

But searching for a certain font-lock face value is the most useful
application of this command.  How do you do that?

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: 36486 <at> debbugs.gnu.org, Stefan Kangas <stefan <at> marxist.se>
Subject: Re: bug#36486: [PATCH] Add tests for text-property-search to check
 prop-match-value
Date: Mon, 08 Jul 2019 23:54:36 +0200

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

> But searching for a certain font-lock face value is the most useful
> application of this command.  How do you do that?

Non-interactively?

(I've never used it to search for any faces.  :-))

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

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

From: Juri Linkov <juri <at> linkov.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 36486 <at> debbugs.gnu.org, Stefan Kangas <stefan <at> marxist.se>
Subject: Re: bug#36486: [PATCH] Add tests for text-property-search to check
 prop-match-value
Date: Tue, 09 Jul 2019 23:31:05 +0300

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

>> But searching for a certain font-lock face value is the most useful
>> application of this command.  How do you do that?
>
> Non-interactively?
>
> (I've never used it to search for any faces.  :-))

It is useful interactively to find errors fontified
with the error face.  Here is a better patch that
can read symbols as well as strings:

[text-property-search-value.2.patch (text/x-diff, inline)]

diff --git a/lisp/emacs-lisp/text-property-search.el b/lisp/emacs-lisp/text-property-search.el
index 41ca07057e..9c45cee3c1 100644
--- a/lisp/emacs-lisp/text-property-search.el
+++ b/lisp/emacs-lisp/text-property-search.el
@@ -54,10 +54,13 @@ text-property-search-forward
 that's matching), and `prop-match-value' (the value of PROPERTY
 at the start of the region)."
   (interactive
-   (list
-    (let ((string (completing-read "Search for property: " obarray)))
-      (when (> (length string) 0)
-        (intern string obarray)))))
+   (let* ((property (completing-read "Search for property: " obarray))
+          (property (when (> (length property) 0)
+                      (intern property obarray)))
+          (value (when property
+                   (read-from-minibuffer "Search for property value: "
+                                         nil nil t nil "nil"))))
+     (list property value)))
   (cond
    ;; No matches at the end of the buffer.
    ((eobp)

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: 36486 <at> debbugs.gnu.org, Stefan Kangas <stefan <at> marxist.se>
Subject: Re: bug#36486: [PATCH] Add tests for text-property-search to check
 prop-match-value
Date: Tue, 09 Jul 2019 23:52:21 +0200

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

> It is useful interactively to find errors fontified
> with the error face.  Here is a better patch that
> can read symbols as well as strings:

[...]

> +          (value (when property
> +                   (read-from-minibuffer "Search for property value: "
> +                                         nil nil t nil "nil"))))
> +     (list property value)))

I don't understand -- this will still return a symbol.  (And error out
if you enter stuff like "foo bar".)

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

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

From: Juri Linkov <juri <at> linkov.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 36486 <at> debbugs.gnu.org, Stefan Kangas <stefan <at> marxist.se>
Subject: Re: bug#36486: [PATCH] Add tests for text-property-search to check
 prop-match-value
Date: Thu, 11 Jul 2019 01:29:45 +0300

>> It is useful interactively to find errors fontified
>> with the error face.  Here is a better patch that
>> can read symbols as well as strings:
>
> [...]
>
>> +          (value (when property
>> +                   (read-from-minibuffer "Search for property value: "
>> +                                         nil nil t nil "nil"))))
>> +     (list property value)))
>
> I don't understand -- this will still return a symbol.  (And error out
> if you enter stuff like "foo bar".)

Yes, entering a symbol returns a symbol, entering a string
like "foo bar" returns a string "foo bar" literally.

Currently text-property-search-forward has more problems:
today I needed to search the property ‘face’ with the value
‘hi-yellow’ in the buffer with regexps highlighted by hi-lock.el.
Executing interactively:

  M-x text-property-search-forward RET face RET hi-yellow RET

failed to find the property because all hi-lock occurrences were
combined with font-lock text properties, i.e. all they had the
property ‘face’ with the value ‘(hi-yellow font-lock-keyword-face)’
and text-property-search-forward fails to find a value in the list
of values.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: 36486 <at> debbugs.gnu.org, Stefan Kangas <stefan <at> marxist.se>
Subject: Re: bug#36486: [PATCH] Add tests for text-property-search to check
 prop-match-value
Date: Thu, 11 Jul 2019 16:17:22 +0200

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

>> I don't understand -- this will still return a symbol.  (And error out
>> if you enter stuff like "foo bar".)
>
> Yes, entering a symbol returns a symbol, entering a string
> like "foo bar" returns a string "foo bar" literally.

Oh, right.  Hm.  Well, entering things with "..." is very unusual in
Emacs prompts, so if we want that, the prompt should at least say that
that's what's expected...

> Currently text-property-search-forward has more problems:
> today I needed to search the property ‘face’ with the value
> ‘hi-yellow’ in the buffer with regexps highlighted by hi-lock.el.
> Executing interactively:
>
>   M-x text-property-search-forward RET face RET hi-yellow RET
>
> failed to find the property because all hi-lock occurrences were
> combined with font-lock text properties, i.e. all they had the
> property ‘face’ with the value ‘(hi-yellow font-lock-keyword-face)’
> and text-property-search-forward fails to find a value in the list
> of values.

Yes, you can't really use text-property-search-forward to do that in any
meaningful manner, which is why I didn't add that to the interactive
bit.  It's a function useful almost only when programming, and the only
useful interactive thing is to vaguely poke around in the buffer.

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

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

From: Juri Linkov <juri <at> linkov.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 36486 <at> debbugs.gnu.org, Stefan Kangas <stefan <at> marxist.se>
Subject: Re: bug#36486: [PATCH] Add tests for text-property-search to check
 prop-match-value
Date: Fri, 12 Jul 2019 01:08:06 +0300

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

>>> I don't understand -- this will still return a symbol.  (And error out
>>> if you enter stuff like "foo bar".)
>>
>> Yes, entering a symbol returns a symbol, entering a string
>> like "foo bar" returns a string "foo bar" literally.
>
> Oh, right.  Hm.  Well, entering things with "..." is very unusual in
> Emacs prompts, so if we want that, the prompt should at least say that
> that's what's expected...

Fixed in the following patch.

>> Currently text-property-search-forward has more problems:
>> today I needed to search the property ‘face’ with the value
>> ‘hi-yellow’ in the buffer with regexps highlighted by hi-lock.el.
>> Executing interactively:
>>
>>   M-x text-property-search-forward RET face RET hi-yellow RET
>>
>> failed to find the property because all hi-lock occurrences were
>> combined with font-lock text properties, i.e. all they had the
>> property ‘face’ with the value ‘(hi-yellow font-lock-keyword-face)’
>> and text-property-search-forward fails to find a value in the list
>> of values.
>
> Yes, you can't really use text-property-search-forward to do that in any
> meaningful manner, which is why I didn't add that to the interactive
> bit.  It's a function useful almost only when programming, and the only
> useful interactive thing is to vaguely poke around in the buffer.

Implemented in this patch:

[text-property-search-value.3.patch (text/x-diff, inline)]

diff --git a/lisp/emacs-lisp/text-property-search.el b/lisp/emacs-lisp/text-property-search.el
index 41ca07057e..a44516992a 100644
--- a/lisp/emacs-lisp/text-property-search.el
+++ b/lisp/emacs-lisp/text-property-search.el
@@ -54,10 +54,13 @@ text-property-search-forward
 that's matching), and `prop-match-value' (the value of PROPERTY
 at the start of the region)."
   (interactive
-   (list
-    (let ((string (completing-read "Search for property: " obarray)))
-      (when (> (length string) 0)
-        (intern string obarray)))))
+   (let* ((property (completing-read "Search for property: " obarray))
+          (property (when (> (length property) 0)
+                      (intern property obarray)))
+          (value (when property
+                   (read-from-minibuffer "Search for property value (quote strings): "
+                                         nil nil t nil "nil"))))
+     (list property value)))
   (cond
    ;; No matches at the end of the buffer.
    ((eobp)
@@ -200,7 +203,9 @@ text-property--match-p
     (setq predicate #'equal))
    ((eq predicate nil)
     (setq predicate (lambda (val p-val)
-                      (not (equal val p-val))))))
+                      (not (if (and (listp p-val) (not (listp val)))
+                               (member val p-val)
+                             (equal val p-val)))))))
   (funcall predicate value prop-value))
 
 (provide 'text-property-search)

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: 36486 <at> debbugs.gnu.org, Stefan Kangas <stefan <at> marxist.se>
Subject: Re: bug#36486: [PATCH] Add tests for text-property-search to check
 prop-match-value
Date: Fri, 12 Jul 2019 16:37:21 +0200

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

> -                      (not (equal val p-val))))))
> +                      (not (if (and (listp p-val) (not (listp val)))
> +                               (member val p-val)
> +                             (equal val p-val)))))))

No, that's not an acceptable change, I think -- this function is a
search primitive, not a DWIM thing.  There's an abundance of things that
can be stored in text properties, to be compared with any number of
predicates.  You're trying to special-case it to search for faces, for
some reason, and that's not what it's for.

If you want a function to search for faces, that's something you can
write (based on this function), but that's not what this function is for.

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

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

From: Juri Linkov <juri <at> linkov.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 36486 <at> debbugs.gnu.org, Stefan Kangas <stefan <at> marxist.se>
Subject: Re: bug#36486: [PATCH] Add tests for text-property-search to check
 prop-match-value
Date: Fri, 12 Jul 2019 21:47:27 +0300

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

>> -                      (not (equal val p-val))))))
>> +                      (not (if (and (listp p-val) (not (listp val)))
>> +                               (member val p-val)
>> +                             (equal val p-val)))))))
>
> No, that's not an acceptable change, I think -- this function is a
> search primitive, not a DWIM thing.  There's an abundance of things that
> can be stored in text properties, to be compared with any number of
> predicates.  You're trying to special-case it to search for faces, for
> some reason, and that's not what it's for.

Then it's possible to add lambda to `predicate' arg in interactive spec only
like below.

> If you want a function to search for faces, that's something you can
> write (based on this function), but that's not what this function is for.

Of course, I can write and add it to ~/.emacs.  But the question is:
do you think this is generally useful?  For example, to search
hi-lock properties, etc.

[text-property-search-value.4.patch (text/x-diff, inline)]

diff --git a/lisp/emacs-lisp/text-property-search.el b/lisp/emacs-lisp/text-property-search.el
index 41ca07057e..867094539f 100644
--- a/lisp/emacs-lisp/text-property-search.el
+++ b/lisp/emacs-lisp/text-property-search.el
@@ -54,10 +54,16 @@ text-property-search-forward
 that's matching), and `prop-match-value' (the value of PROPERTY
 at the start of the region)."
   (interactive
-   (list
-    (let ((string (completing-read "Search for property: " obarray)))
-      (when (> (length string) 0)
-        (intern string obarray)))))
+   (let* ((property (completing-read "Search for property: " obarray))
+          (property (when (> (length property) 0)
+                      (intern property obarray)))
+          (value (when property
+                   (read-from-minibuffer "Search for property value (quote strings): "
+                                         nil nil t nil "nil"))))
+     (list property value (lambda (val p-val)
+                            (not (if (and (listp p-val) (not (listp val)))
+                                     (member val p-val)
+                                   (equal val p-val)))))))
   (cond
    ;; No matches at the end of the buffer.
    ((eobp)

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: 36486 <at> debbugs.gnu.org, Stefan Kangas <stefan <at> marxist.se>
Subject: Re: bug#36486: [PATCH] Add tests for text-property-search to check
 prop-match-value
Date: Sat, 13 Jul 2019 01:40:53 +0200

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

> Of course, I can write and add it to ~/.emacs.  But the question is:
> do you think this is generally useful?  For example, to search
> hi-lock properties, etc.

I don't think this code complication belongs in that function.  If there
should be a function to search for faces, then face-search-forward
should be written (and it would be a very simple call to
text-property-search-forward with the correct predicate).

But I haven't heard anybody clamouring for such a command, really...

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

Previous Next

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