all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* [PATCH] Add option COUNT argument to text-property-search functions
@ 2021-07-30 19:20 James N V Cash
  2021-07-30 19:51 ` Lars Ingebrigtsen
  2021-07-30 19:59 ` Eli Zaretskii
  0 siblings, 2 replies; 8+ messages in thread
From: James N V Cash @ 2021-07-30 19:20 UTC (permalink / raw)
  To: emacs-devel


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



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] Add option COUNT argument to text-property-search functions
  2021-07-30 19:20 [PATCH] Add option COUNT argument to text-property-search functions James N V Cash
@ 2021-07-30 19:51 ` Lars Ingebrigtsen
  2021-07-30 19:55   ` James N. V. Cash
  2021-07-30 19:59 ` Eli Zaretskii
  1 sibling, 1 reply; 8+ messages in thread
From: Lars Ingebrigtsen @ 2021-07-30 19:51 UTC (permalink / raw)
  To: James N V Cash; +Cc: emacs-devel

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



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Add option COUNT argument to text-property-search functions
  2021-07-30 19:51 ` Lars Ingebrigtsen
@ 2021-07-30 19:55   ` James N. V. Cash
  2021-07-30 21:56     ` Dmitry Gutov
  0 siblings, 1 reply; 8+ messages in thread
From: James N. V. Cash @ 2021-07-30 19:55 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Protesilaos Stavrou, emacs-devel

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.



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Add option COUNT argument to text-property-search functions
  2021-07-30 19:20 [PATCH] Add option COUNT argument to text-property-search functions James N V Cash
  2021-07-30 19:51 ` Lars Ingebrigtsen
@ 2021-07-30 19:59 ` Eli Zaretskii
  2021-07-30 20:49   ` James Cash
  1 sibling, 1 reply; 8+ messages in thread
From: Eli Zaretskii @ 2021-07-30 19:59 UTC (permalink / raw)
  To: James N V Cash; +Cc: emacs-devel

> 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.



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Add option COUNT argument to text-property-search functions
  2021-07-30 19:59 ` Eli Zaretskii
@ 2021-07-30 20:49   ` James Cash
  0 siblings, 0 replies; 8+ messages in thread
From: James Cash @ 2021-07-30 20:49 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

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



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] Add option COUNT argument to text-property-search functions
  2021-07-30 19:55   ` James N. V. Cash
@ 2021-07-30 21:56     ` Dmitry Gutov
  2021-07-31  6:56       ` Protesilaos Stavrou
  0 siblings, 1 reply; 8+ messages in thread
From: Dmitry Gutov @ 2021-07-30 21:56 UTC (permalink / raw)
  To: James N. V. Cash, Lars Ingebrigtsen; +Cc: Protesilaos Stavrou, emacs-devel

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



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Add option COUNT argument to text-property-search functions
  2021-07-30 21:56     ` Dmitry Gutov
@ 2021-07-31  6:56       ` Protesilaos Stavrou
  2021-07-31 11:36         ` Lars Ingebrigtsen
  0 siblings, 1 reply; 8+ messages in thread
From: Protesilaos Stavrou @ 2021-07-31  6:56 UTC (permalink / raw)
  To: Dmitry Gutov, James N. V. Cash, Lars Ingebrigtsen; +Cc: emacs-devel

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



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Add option COUNT argument to text-property-search functions
  2021-07-31  6:56       ` Protesilaos Stavrou
@ 2021-07-31 11:36         ` Lars Ingebrigtsen
  0 siblings, 0 replies; 8+ messages in thread
From: Lars Ingebrigtsen @ 2021-07-31 11:36 UTC (permalink / raw)
  To: Protesilaos Stavrou; +Cc: James N. V. Cash, emacs-devel, Dmitry Gutov

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



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-07-31 11:36 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-07-30 19:20 [PATCH] Add option COUNT argument to text-property-search functions James N V Cash
2021-07-30 19:51 ` Lars Ingebrigtsen
2021-07-30 19:55   ` James N. V. Cash
2021-07-30 21:56     ` Dmitry Gutov
2021-07-31  6:56       ` Protesilaos Stavrou
2021-07-31 11:36         ` Lars Ingebrigtsen
2021-07-30 19:59 ` Eli Zaretskii
2021-07-30 20:49   ` James Cash

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.