[PATCH] Add option COUNT argument to text-property-search functions

[PATCH] Add option COUNT argument to text-property-search functions

[James N V Cash]

This allows searching forward for the Nth occurance of the text
property, analogously to re-search-forward/backward.

---
 lisp/emacs-lisp/text-property-search.el | 59 +++++++++++++++++++------
 1 file changed, 45 insertions(+), 14 deletions(-)

diff --git a/lisp/emacs-lisp/text-property-search.el b/lisp/emacs-lisp/text-property-search.el
index 7da02a9cb2..fa51488135 100644
--- a/lisp/emacs-lisp/text-property-search.el
+++ b/lisp/emacs-lisp/text-property-search.el
@@ -30,7 +30,7 @@
   beginning end value)

 (defun text-property-search-forward (property &optional value predicate
-                                              not-current)
+                                              not-current count)
   "Search for the next region of text where PREDICATE is true.
 PREDICATE is used to decide whether a value of PROPERTY should be
 considered as matching VALUE.
@@ -59,6 +59,9 @@ text-property-search-forward
 region that doesn't include point and has a value of PROPERTY
 that matches VALUE.

+If COUNT is a positive number, it will search forward COUNT times. If
+negative, it will perform text-property-search-backwards -COUNT times.
+
 If no matches can be found, return nil and don't move point.
 If found, move point to the end of the region and return a
 `prop-match' object describing the match.  To access the details
@@ -74,6 +77,9 @@ text-property-search-forward
    ;; No matches at the end of the buffer.
    ((eobp)
     nil)
+   ;; Negative count means search backwards
+   ((and (numberp count) (< count 0))
+    (text-property-search-backward property value predicate not-current (- count)))
    ;; We're standing in the property we're looking for, so find the
    ;; end.
    ((and (text-property--match-p value (get-text-property (point) property)
@@ -83,25 +89,36 @@ text-property-search-forward
    (t
     (let ((origin (point))
           (ended nil)
+          (count (or count 1))
+          (match t)
           pos)
-      ;; Find the next candidate.
+      ;; Find the COUNT-th next candidate.
       (while (not ended)
         (setq pos (next-single-property-change (point) property))
         (if (not pos)
             (progn
               (goto-char origin)
-              (setq ended t))
+              (setq ended match))
           (goto-char pos)
           (if (text-property--match-p value (get-text-property (point) property)
                                       predicate)
-              (setq ended
-                    (text-property--find-end-forward
-                     (point) property value predicate))
+              (progn
+                (setq match (text-property--find-end-forward
+                             (point) property value predicate))
+                (setq origin (point))
+                (cl-decf count)
+                (if (zerop count)
+                    (setq ended match)
+                  (setq pos (next-single-property-change (point) property))
+                  (if pos
+                      (goto-char pos)
+                    (goto-char origin)
+                    (setq ended match))))
             ;; Skip past this section of non-matches.
             (setq pos (next-single-property-change (point) property))
             (unless pos
               (goto-char origin)
-              (setq ended t)))))
+              (setq ended match)))))
       (and (not (eq ended t))
            ended)))))

@@ -133,7 +150,7 @@ text-property--find-end-forward


 (defun text-property-search-backward (property &optional value predicate
-                                               not-current)
+                                               not-current count)
   "Search for the previous region of text whose PROPERTY matches VALUE.

 Like `text-property-search-forward', which see, but searches backward,
@@ -147,6 +164,9 @@ text-property-search-backward
    ;; We're at the start of the buffer; no previous matches.
    ((bobp)
     nil)
+   ;; Negative count means search forwards
+   ((and (numberp count) (< count 0))
+    (text-property-search-forward property value predicate not-current (- count)))
    ;; We're standing in the property we're looking for, so find the
    ;; end.
    ((text-property--match-p
@@ -165,26 +185,37 @@ text-property-search-backward
    (t
     (let ((origin (point))
           (ended nil)
+          (count (or count 1))
+          (match t)
           pos)
       (forward-char -1)
-      ;; Find the previous candidate.
+      ;; Find the COUNT-th previous candidate.
       (while (not ended)
         (setq pos (previous-single-property-change (point) property))
         (if (not pos)
             (progn
               (goto-char origin)
-              (setq ended t))
+              (setq ended match))
           (goto-char (1- pos))
           (if (text-property--match-p value (get-text-property (point) property)
                                       predicate)
-              (setq ended
-                    (text-property--find-end-backward
-                     (point) property value predicate))
+              (progn
+                (setq match (text-property--find-end-backward
+                             (point) property value predicate))
+                (setq origin (point))
+                (cl-decf count)
+                (if (zerop count)
+                    (setq ended match)
+                  (setq pos (previous-single-property-change (point) property))
+                  (if pos
+                      (goto-char pos)
+                    (goto-char origin)
+                    (setq ended match))))
             ;; Skip past this section of non-matches.
             (setq pos (previous-single-property-change (point) property))
             (unless pos
               (goto-char origin)
-              (setq ended t)))))
+              (setq ended match)))))
       (and (not (eq ended t))
            ended)))))

--
2.25.1

Re: [PATCH] Add option COUNT argument to text-property-search functions

[Lars Ingebrigtsen]

James N V Cash <james.cash@occasionallycogent.com> writes:

> This allows searching forward for the Nth occurance of the text
> property, analogously to re-search-forward/backward.

I don't think I've ever registered seeing COUNT used in the wild for
those functions, and I think it's a design flaw to have them.  So I'm
not in favour of adding those to more functions.

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

Re: [PATCH] Add option COUNT argument to text-property-search functions

[James N. V. Cash]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> James N V Cash <james.cash@occasionallycogent.com> writes:
>
>> This allows searching forward for the Nth occurance of the text
>> property, analogously to re-search-forward/backward.
>
> I don't think I've ever registered seeing COUNT used in the wild for
> those functions, and I think it's a design flaw to have them.  So I'm
> not in favour of adding those to more functions.

Hah, fair enough! I was discussing something with Protesilaos previously
that would've been made easier by having this support a count (versus
wrapping it in a dotimes), but I certainly can see why you'd want
to avoid adding yet another optional argument.

Re: [PATCH] Add option COUNT argument to text-property-search functions

[Eli Zaretskii]

> From: James N V Cash <james.cash@occasionallycogent.com>
> Cc: 
> Date: Fri, 30 Jul 2021 15:20:59 -0400
> 
> +If COUNT is a positive number, it will search forward COUNT times. If
                                                                    ^^
Two spaces between sentences, please, per the US English conventions.

Also, could you please add tests for this new feature?

Thanks.

P.S. Would it be possible for you to use the same email address in
your contributions as the one you used in the copyright assignment?
It will help us verify that your assignment is on file.

Re: [PATCH] Add option COUNT argument to text-property-search functions

[James Cash]

Eli Zaretskii <eliz@gnu.org> writes:

>> +If COUNT is a positive number, it will search forward COUNT times. If
>                                                                     ^^
> Two spaces between sentences, please, per the US English conventions.

Fixed.

> Also, could you please add tests for this new feature?

Added!

> P.S. Would it be possible for you to use the same email address in
> your contributions as the one you used in the copyright assignment?
> It will help us verify that your assignment is on file.

Ah, sorry about that; replying from the email address from which I did
the copyright assignment (which isn't subscribed to the mailing list
though, so hopefully this gets through; I suppose I should switch which
address is subscribed).

---
 lisp/emacs-lisp/text-property-search.el       | 59 ++++++++++++++-----
 .../emacs-lisp/text-property-search-tests.el  | 25 ++++++++
 2 files changed, 70 insertions(+), 14 deletions(-)

diff --git a/lisp/emacs-lisp/text-property-search.el b/lisp/emacs-lisp/text-property-search.el
index 7da02a9cb2..e4fcbf2ed1 100644
--- a/lisp/emacs-lisp/text-property-search.el
+++ b/lisp/emacs-lisp/text-property-search.el
@@ -30,7 +30,7 @@
   beginning end value)

 (defun text-property-search-forward (property &optional value predicate
-                                              not-current)
+                                              not-current count)
   "Search for the next region of text where PREDICATE is true.
 PREDICATE is used to decide whether a value of PROPERTY should be
 considered as matching VALUE.
@@ -59,6 +59,9 @@ text-property-search-forward
 region that doesn't include point and has a value of PROPERTY
 that matches VALUE.

+If COUNT is a positive number, it will search forward COUNT times.  If
+negative, it will perform text-property-search-backwards -COUNT times.
+
 If no matches can be found, return nil and don't move point.
 If found, move point to the end of the region and return a
 `prop-match' object describing the match.  To access the details
@@ -71,6 +74,9 @@ text-property-search-forward
       (when (> (length string) 0)
         (intern string obarray)))))
   (cond
+   ;; Negative count means search backwards
+   ((and (numberp count) (< count 0))
+    (text-property-search-backward property value predicate not-current (- count)))
    ;; No matches at the end of the buffer.
    ((eobp)
     nil)
@@ -83,25 +89,36 @@ text-property-search-forward
    (t
     (let ((origin (point))
           (ended nil)
+          (count (or count 1))
+          (match t)
           pos)
-      ;; Find the next candidate.
+      ;; Find the COUNT-th next candidate.
       (while (not ended)
         (setq pos (next-single-property-change (point) property))
         (if (not pos)
             (progn
               (goto-char origin)
-              (setq ended t))
+              (setq ended match))
           (goto-char pos)
           (if (text-property--match-p value (get-text-property (point) property)
                                       predicate)
-              (setq ended
-                    (text-property--find-end-forward
-                     (point) property value predicate))
+              (progn
+                (setq match (text-property--find-end-forward
+                             (point) property value predicate))
+                (setq origin (point))
+                (cl-decf count)
+                (if (zerop count)
+                    (setq ended match)
+                  (setq pos (next-single-property-change (point) property))
+                  (if pos
+                      (goto-char pos)
+                    (goto-char origin)
+                    (setq ended match))))
             ;; Skip past this section of non-matches.
             (setq pos (next-single-property-change (point) property))
             (unless pos
               (goto-char origin)
-              (setq ended t)))))
+              (setq ended match)))))
       (and (not (eq ended t))
            ended)))))

@@ -133,7 +150,7 @@ text-property--find-end-forward


 (defun text-property-search-backward (property &optional value predicate
-                                               not-current)
+                                               not-current count)
   "Search for the previous region of text whose PROPERTY matches VALUE.

 Like `text-property-search-forward', which see, but searches backward,
@@ -144,6 +161,9 @@ text-property-search-backward
       (when (> (length string) 0)
         (intern string obarray)))))
   (cond
+   ;; Negative count means search forwards
+   ((and (numberp count) (< count 0))
+    (text-property-search-forward property value predicate not-current (- count)))
    ;; We're at the start of the buffer; no previous matches.
    ((bobp)
     nil)
@@ -165,26 +185,37 @@ text-property-search-backward
    (t
     (let ((origin (point))
           (ended nil)
+          (count (or count 1))
+          (match t)
           pos)
       (forward-char -1)
-      ;; Find the previous candidate.
+      ;; Find the COUNT-th previous candidate.
       (while (not ended)
         (setq pos (previous-single-property-change (point) property))
         (if (not pos)
             (progn
               (goto-char origin)
-              (setq ended t))
+              (setq ended match))
           (goto-char (1- pos))
           (if (text-property--match-p value (get-text-property (point) property)
                                       predicate)
-              (setq ended
-                    (text-property--find-end-backward
-                     (point) property value predicate))
+              (progn
+                (setq match (text-property--find-end-backward
+                             (point) property value predicate))
+                (setq origin (point))
+                (cl-decf count)
+                (if (zerop count)
+                    (setq ended match)
+                  (setq pos (previous-single-property-change (point) property))
+                  (if pos
+                      (goto-char pos)
+                    (goto-char origin)
+                    (setq ended match))))
             ;; Skip past this section of non-matches.
             (setq pos (previous-single-property-change (point) property))
             (unless pos
               (goto-char origin)
-              (setq ended t)))))
+              (setq ended match)))))
       (and (not (eq ended t))
            ended)))))

diff --git a/test/lisp/emacs-lisp/text-property-search-tests.el b/test/lisp/emacs-lisp/text-property-search-tests.el
index 90f06c3c4c..16ff6e87fd 100644
--- a/test/lisp/emacs-lisp/text-property-search-tests.el
+++ b/test/lisp/emacs-lisp/text-property-search-tests.el
@@ -79,12 +79,32 @@ text-property-search-forward-partial-non-current-bold-t
              '("bold2")
              10))

+(ert-deftest text-property-search-forward-non-current-bold-t-count-1 ()
+  (with-test (text-property-search-forward 'face 'bold t t 1)
+             '("bold1" "bold2")))
+
+(ert-deftest text-property-search-forward-non-current-bold-t-count-2 ()
+  (with-test (text-property-search-forward 'face 'bold t t 2)
+             '("bold2")))
+
+(ert-deftest text-property-search-backwards-non-current-bold-t-count--negative-2 ()
+  (with-test (text-property-search-backward 'face 'bold t t -2)
+             '("bold2")))
+
+(ert-deftest text-property-search-forward-non-current-bold-t-count-many ()
+  (with-test (text-property-search-forward 'face 'bold t t 100)
+             '("bold2")))

 (ert-deftest text-property-search-backward-bold-t ()
   (with-test (text-property-search-backward 'face 'bold t)
              '("bold2" "bold1")
              (point-max)))

+(ert-deftest text-property-search-forward-bold-t-negative-count ()
+  (with-test (text-property-search-forward 'face 'bold t nil -1)
+             '("bold2" "bold1")
+             (point-max)))
+
 (ert-deftest text-property-search-backward-bold-nil ()
   (with-test (text-property-search-backward 'face 'bold nil)
              '( "italic2 at the end" " and this is italic1" "This is ")
@@ -110,6 +130,11 @@ text-property-search-backward-partial-non-current-bold-t
              '("bold1")
              35))

+(ert-deftest text-property-search-backward-non-current-bold-t-count-2 ()
+  (with-test (text-property-search-backward 'face 'bold t t 2)
+             '("bold1")
+             (point-max)))
+
 (defmacro with-match-test (form beginning end value &optional point)
   `(with-temp-buffer
      (text-property-setup)
--
2.25.1

Re: [PATCH] Add option COUNT argument to text-property-search functions

[Dmitry Gutov]

On 30.07.2021 22:55, James N. V. Cash wrote:
> Hah, fair enough! I was discussing something with Protesilaos previously
> that would've been made easier by having this support a count (versus
> wrapping it in a dotimes), but I certainly can see why you'd want
> to avoid adding yet another optional argument.

Could you describe that usage scenario here

Re: [PATCH] Add option COUNT argument to text-property-search functions

[Protesilaos Stavrou]

On 2021-07-31, 00:56 +0300, Dmitry Gutov <dgutov@yandex.ru> wrote:

> On 30.07.2021 22:55, James N. V. Cash wrote:
>> Hah, fair enough! I was discussing something with Protesilaos previously
>> that would've been made easier by having this support a count (versus
>> wrapping it in a dotimes), but I certainly can see why you'd want
>> to avoid adding yet another optional argument.
>
> Could you describe that usage scenario here

The idea is to have a command that moves point.  So it could accept a
numeric argument to go to the COUNTth match, like 're-search-forward'.

This came about while experimenting with the new group headings in the
Completions' buffer, where we would like to jump between different
groups of matches in one go (see faces 'completions-group-title',
'completions-group-separator').

I don't have a ready-made recipe for you on 'emacs -Q', as I am not sure
what built-in command makes use of those group headings.  Though I can
try to prepare one if you need it.

-- 
Protesilaos Stavrou
https://protesilaos.com

Re: [PATCH] Add option COUNT argument to text-property-search functions

[Lars Ingebrigtsen]

Protesilaos Stavrou <info@protesilaos.com> writes:

> The idea is to have a command that moves point.  So it could accept a
> numeric argument to go to the COUNTth match, like 're-search-forward'.

I grepped through the Emacs sources, and the COUNT parameter to
`re-search-forward' was used in 2% of the usage cases, which is about
10000% more than I had imagined -- so perhaps it does make sense to add
a COUNT parameter to `text-property-search-*' (especially for symmetry
with `re-search-*').

But I'm for composability and trying to keeping function interfaces
simple and easy to reason about -- there's a tendency to stuff more and
more functionality down into functions when it's trivial to just use the
functions instead.  And these functions already have plenty of optional
parameters (and will probably grow even more in the future, if Emacs
history repeats itself), so I'm still not very excited about adding
COUNT.

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