* bug#35708: [27.0.50]: thingatpt.el, thing-at-point-looking-at redundant
@ 2019-05-13 7:19 Andreas Röhler
2019-05-13 12:41 ` Noam Postavsky
0 siblings, 1 reply; 10+ messages in thread
From: Andreas Röhler @ 2019-05-13 7:19 UTC (permalink / raw)
To: 35708
Hi all,
as result of ‘thing-at-point-looking-at’ finally is delivered by
‘looking-at’,
don't see any sense in calling ‘re-search-forward’ first.
Best,
Andreas
GNU Emacs 27.0.50 (build 1, i686-pc-linux-gnu, GTK+ Version 3.14.5) of
2019-04-29
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#35708: [27.0.50]: thingatpt.el, thing-at-point-looking-at redundant
2019-05-13 7:19 bug#35708: [27.0.50]: thingatpt.el, thing-at-point-looking-at redundant Andreas Röhler
@ 2019-05-13 12:41 ` Noam Postavsky
2019-05-13 18:31 ` Andreas Röhler
0 siblings, 1 reply; 10+ messages in thread
From: Noam Postavsky @ 2019-05-13 12:41 UTC (permalink / raw)
To: Andreas Röhler; +Cc: 35708
[-- Attachment #1: Type: text/plain, Size: 364 bytes --]
Andreas Röhler <andreas.roehler@easy-emacs.de> writes:
> as result of ‘thing-at-point-looking-at’ finally is delivered by
> ‘looking-at’,
> don't see any sense in calling ‘re-search-forward’ first.
The test added below fails if the re-search-forward in
thing-at-point-looking-at is commented out. Does that tell you what the
"sense" is?
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 1120 bytes --]
From 79b55e8e6dfee9cba9e464860546dbab2cdd36d8 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Mon, 13 May 2019 08:39:00 -0400
Subject: [PATCH] ; Add thing-at-point-looking-at test (Bug#35708)
* test/lisp/thingatpt-tests.el (thing-at-point-looking-at): New test.
---
test/lisp/thingatpt-tests.el | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/test/lisp/thingatpt-tests.el b/test/lisp/thingatpt-tests.el
index 452fcc6895..339a4a4b81 100644
--- a/test/lisp/thingatpt-tests.el
+++ b/test/lisp/thingatpt-tests.el
@@ -131,4 +131,15 @@ (ert-deftest thing-at-point-url-in-comment ()
(goto-char 23)
(should (equal (thing-at-point 'url) "http://foo/bar(baz)"))))
+(ert-deftest thing-at-point-looking-at ()
+ (with-temp-buffer
+ (insert "1abcd 2abcd 3abcd")
+ (goto-char (point-min))
+ (let ((m2 (progn (search-forward "2abcd")
+ (match-data))))
+ (goto-char (point-min))
+ (search-forward "2ab")
+ (should (thing-at-point-looking-at "2abcd"))
+ (should (equal (match-data) m2)))))
+
;;; thingatpt.el ends here
--
2.11.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* bug#35708: [27.0.50]: thingatpt.el, thing-at-point-looking-at redundant
2019-05-13 12:41 ` Noam Postavsky
@ 2019-05-13 18:31 ` Andreas Röhler
2019-05-13 19:25 ` npostavs
0 siblings, 1 reply; 10+ messages in thread
From: Andreas Röhler @ 2019-05-13 18:31 UTC (permalink / raw)
To: Noam Postavsky; +Cc: 35708
On 13.05.19 14:41, Noam Postavsky wrote:
> Andreas Röhler <andreas.roehler@easy-emacs.de> writes:
>
>> as result of ‘thing-at-point-looking-at’ finally is delivered by
>> ‘looking-at’,
>> don't see any sense in calling ‘re-search-forward’ first.
> The test added below fails if the re-search-forward in
> thing-at-point-looking-at is commented out. Does that tell you what the
> "sense" is?
>
Thought at something like below, which should pass the test:
(defun ar-thing-at-point-looking-at (regexp)
"Return t if regexp matches at or before point, nil otherwise."
(save-excursion
(while (not (or (looking-at regexp)(bolp)))
(forward-char -1))
(looking-at regexp)))
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#35708: [27.0.50]: thingatpt.el, thing-at-point-looking-at redundant
2019-05-13 18:31 ` Andreas Röhler
@ 2019-05-13 19:25 ` npostavs
2019-05-14 10:02 ` Andreas Röhler
0 siblings, 1 reply; 10+ messages in thread
From: npostavs @ 2019-05-13 19:25 UTC (permalink / raw)
To: Andreas Röhler; +Cc: 35708, Noam Postavsky
Andreas Röhler <andreas.roehler@easy-emacs.de> writes:
> Thought at something like below, which should pass the test:
>
> (defun ar-thing-at-point-looking-at (regexp)
> "Return t if regexp matches at or before point, nil otherwise."
> (save-excursion
> (while (not (or (looking-at regexp)(bolp)))
> (forward-char -1))
> (looking-at regexp)))
I think it's an optimization to use re-search-backward instead of moving
on character at a time and calling looking-at in lisp.
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#35708: [27.0.50]: thingatpt.el, thing-at-point-looking-at redundant
2019-05-13 19:25 ` npostavs
@ 2019-05-14 10:02 ` Andreas Röhler
2019-05-14 10:49 ` Andreas Röhler
0 siblings, 1 reply; 10+ messages in thread
From: Andreas Röhler @ 2019-05-14 10:02 UTC (permalink / raw)
To: npostavs; +Cc: 35708
On 13.05.19 21:25, npostavs@gmail.com wrote:
> Andreas Röhler <andreas.roehler@easy-emacs.de> writes:
>
>> Thought at something like below, which should pass the test:
>>
>> (defun ar-thing-at-point-looking-at (regexp)
>> "Return t if regexp matches at or before point, nil otherwise."
>> (save-excursion
>> (while (not (or (looking-at regexp)(bolp)))
>> (forward-char -1))
>> (looking-at regexp)))
> I think it's an optimization to use re-search-backward instead of moving
> on character at a time and calling looking-at in lisp.
>
>
Hmm, current thing-at-point-looking-at might be slow with large buffers.
The slightly modified test should reveal it:
(ert-deftest thing-at-point-looking-at-2 ()
(with-temp-buffer
(insert "1abcd 222abcd")
(dotimes (_ 99999) (insert " asdf "))
(goto-char (point-min))
(let ((m2 (progn (search-forward "2abcd")
(match-data))))
(goto-char (point-min))
(search-forward "2ab")
(should (thing-at-point-looking-at "2abcd"))
(should (equal (match-data) m2)))))
But let me correct the alternative delivered, as it didn't match before
point:
(defun ar-regexp-atpt (regexp)
"Return t if REGEXP matches at or before point, nil otherwise.
Changes match-data"
(save-excursion
(if (looking-at regexp)
(while
(save-excursion
(and (not (bobp))
(progn (backward-char) (looking-at regexp)))))
(while (not (or (bobp) (backward-char) (looking-at regexp))))
(ar-regexp-atpt regexp))
(looking-at regexp)))
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#35708: [27.0.50]: thingatpt.el, thing-at-point-looking-at redundant
2019-05-14 10:02 ` Andreas Röhler
@ 2019-05-14 10:49 ` Andreas Röhler
2019-05-14 14:34 ` npostavs
0 siblings, 1 reply; 10+ messages in thread
From: Andreas Röhler @ 2019-05-14 10:49 UTC (permalink / raw)
To: npostavs; +Cc: 35708
On 14.05.19 12:02, Andreas Röhler wrote:
>
> On 13.05.19 21:25, npostavs@gmail.com wrote:
>> Andreas Röhler <andreas.roehler@easy-emacs.de> writes:
>>
>>> Thought at something like below, which should pass the test:
>>>
>>> (defun ar-thing-at-point-looking-at (regexp)
>>> "Return t if regexp matches at or before point, nil otherwise."
>>> (save-excursion
>>> (while (not (or (looking-at regexp)(bolp)))
>>> (forward-char -1))
>>> (looking-at regexp)))
>> I think it's an optimization to use re-search-backward instead of moving
>> on character at a time and calling looking-at in lisp.
>>
>>
>
> Hmm, current thing-at-point-looking-at might be slow with large
> buffers. The slightly modified test should reveal it:
>
> (ert-deftest thing-at-point-looking-at-2 ()
> (with-temp-buffer
> (insert "1abcd 222abcd")
> (dotimes (_ 99999) (insert " asdf "))
> (goto-char (point-min))
> (let ((m2 (progn (search-forward "2abcd")
> (match-data))))
> (goto-char (point-min))
> (search-forward "2ab")
> (should (thing-at-point-looking-at "2abcd"))
> (should (equal (match-data) m2)))))
>
> But let me correct the alternative delivered, as it didn't match
> before point:
>
> (defun ar-regexp-atpt (regexp)
>
> "Return t if REGEXP matches at or before point, nil otherwise.
>
> Changes match-data"
> (save-excursion
> (if (looking-at regexp)
> (while
> (save-excursion
> (and (not (bobp))
> (progn (backward-char) (looking-at regexp)))))
> (while (not (or (bobp) (backward-char) (looking-at regexp))))
> (ar-regexp-atpt regexp))
> (looking-at regexp)))
>
>
>
>
Another fix, as a bug showed up when testing (ar-regexp-atpt "[a-z]+"):
(defun ar-regexp-atpt (regexp)
"Return t if REGEXP matches at or before point, nil otherwise.
Changes match-data"
(save-excursion
(if (looking-at regexp)
(while
(and (not (bobp))
(or (progn (backward-char) (looking-at regexp))
(forward-char 1))))
(while (not (or (bobp) (backward-char) (looking-at regexp))))
(ar-regexp-atpt regexp))
(looking-at regexp)))
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#35708: [27.0.50]: thingatpt.el, thing-at-point-looking-at redundant
2019-05-14 10:49 ` Andreas Röhler
@ 2019-05-14 14:34 ` npostavs
2019-05-15 6:59 ` Andreas Röhler
0 siblings, 1 reply; 10+ messages in thread
From: npostavs @ 2019-05-14 14:34 UTC (permalink / raw)
To: Andreas Röhler; +Cc: 35708, npostavs
Andreas Röhler <andreas.roehler@easy-emacs.de> writes:
>> Hmm, current thing-at-point-looking-at might be slow with large
>> buffers. The slightly modified test should reveal it:
>>
>> (ert-deftest thing-at-point-looking-at-2 ()
>> (with-temp-buffer
>> (insert "1abcd 222abcd")
>> (dotimes (_ 99999) (insert " asdf "))
>> (goto-char (point-min))
>> (search-forward "2ab")
>> (should (thing-at-point-looking-at "2abcd"))
Yes, in this case, since the loop over looking-at only needs to iterate
twice, so it will be faster. But what about when there is no match?
E.g.,
(with-temp-buffer
(dotimes (_ 99999) (insert " asdf "))
(goto-char (point-max))
(list :ar-regexp-atpt (benchmark-run (ar-regexp-atpt "foo"))
:thing-at-point-looking-at (benchmark-run (thing-at-point-looking-at "foo"))))
> Another fix, as a bug showed up when testing (ar-regexp-atpt "[a-z]+"):
>
> (defun ar-regexp-atpt (regexp)
> "Return t if REGEXP matches at or before point, nil otherwise.
>
> Changes match-data"
> (save-excursion
> (if (looking-at regexp)
> (while
> (and (not (bobp))
> (or (progn (backward-char) (looking-at regexp))
> (forward-char 1))))
> (while (not (or (bobp) (backward-char) (looking-at regexp))))
> (ar-regexp-atpt regexp))
What's this recursive call for? It triggers (error "Lisp nesting
exceeds ‘max-lisp-eval-depth’") in the benchmark above.
> (looking-at regexp)))
^ permalink raw reply [flat|nested] 10+ messages in thread* bug#35708: [27.0.50]: thingatpt.el, thing-at-point-looking-at redundant
2019-05-14 14:34 ` npostavs
@ 2019-05-15 6:59 ` Andreas Röhler
2019-05-15 10:11 ` Andreas Röhler
2019-05-20 17:17 ` Noam Postavsky
0 siblings, 2 replies; 10+ messages in thread
From: Andreas Röhler @ 2019-05-15 6:59 UTC (permalink / raw)
To: npostavs; +Cc: 35708
Am 14.05.19 um 16:34 schrieb npostavs@gmail.com:
> Andreas Röhler <andreas.roehler@easy-emacs.de> writes:
>
>>> Hmm, current thing-at-point-looking-at might be slow with large
>>> buffers. The slightly modified test should reveal it:
>>>
>>> (ert-deftest thing-at-point-looking-at-2 ()
>>> (with-temp-buffer
>>> (insert "1abcd 222abcd")
>>> (dotimes (_ 99999) (insert " asdf "))
>>> (goto-char (point-min))
>>> (search-forward "2ab")
>>> (should (thing-at-point-looking-at "2abcd"))
> Yes, in this case, since the loop over looking-at only needs to iterate
> twice, so it will be faster. But what about when there is no match?
> E.g.,
>
> (with-temp-buffer
> (dotimes (_ 99999) (insert " asdf "))
> (goto-char (point-max))
> (list :ar-regexp-atpt (benchmark-run (ar-regexp-atpt "foo"))
> :thing-at-point-looking-at (benchmark-run (thing-at-point-looking-at "foo"))))
>
>> Another fix, as a bug showed up when testing (ar-regexp-atpt "[a-z]+"):
>>
>> (defun ar-regexp-atpt (regexp)
>> "Return t if REGEXP matches at or before point, nil otherwise.
>>
>> Changes match-data"
>> (save-excursion
>> (if (looking-at regexp)
>> (while
>> (and (not (bobp))
>> (or (progn (backward-char) (looking-at regexp))
>> (forward-char 1))))
>> (while (not (or (bobp) (backward-char) (looking-at regexp))))
>> (ar-regexp-atpt regexp))
> What's this recursive call for? It triggers (error "Lisp nesting
> exceeds ‘max-lisp-eval-depth’") in the benchmark above.
>
>> (looking-at regexp)))
The recursive call needed a guard: (unless (bobp)
It is called after function went backward while not looking-at matches,
Now the result for the 99999 is
(:ar-regexp-atpt (0.774574453 0 0.0) :thing-at-point-looking-at
(0.000798669 0 0.0))
The fixed form:
(defun ar-regexp-atpt (regexp)
"Return t if REGEXP matches at or before point, nil otherwise.
Changes match-data"
(save-excursion
(if (looking-at regexp)
(while
(and (not (bobp))
(or (progn (backward-char) (looking-at regexp))
(forward-char 1))))
(while (not (or (bobp) (backward-char) (looking-at regexp))))
(unless (bobp) (ar-regexp-atpt regexp)))
(looking-at regexp)))
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#35708: [27.0.50]: thingatpt.el, thing-at-point-looking-at redundant
2019-05-15 6:59 ` Andreas Röhler
@ 2019-05-15 10:11 ` Andreas Röhler
2019-05-20 17:17 ` Noam Postavsky
1 sibling, 0 replies; 10+ messages in thread
From: Andreas Röhler @ 2019-05-15 10:11 UTC (permalink / raw)
To: npostavs; +Cc: 35708
[-- Attachment #1: Type: text/plain, Size: 2755 bytes --]
Am 15.05.19 um 08:59 schrieb Andreas Röhler:
>
> Am 14.05.19 um 16:34 schrieb npostavs@gmail.com:
>> Andreas Röhler <andreas.roehler@easy-emacs.de> writes:
>>
>>>> Hmm, current thing-at-point-looking-at might be slow with large
>>>> buffers. The slightly modified test should reveal it:
>>>>
>>>> (ert-deftest thing-at-point-looking-at-2 ()
>>>> (with-temp-buffer
>>>> (insert "1abcd 222abcd")
>>>> (dotimes (_ 99999) (insert " asdf "))
>>>> (goto-char (point-min))
>>>> (search-forward "2ab")
>>>> (should (thing-at-point-looking-at "2abcd"))
>> Yes, in this case, since the loop over looking-at only needs to iterate
>> twice, so it will be faster. But what about when there is no match?
>> E.g.,
>>
>> (with-temp-buffer
>> (dotimes (_ 99999) (insert " asdf "))
>> (goto-char (point-max))
>> (list :ar-regexp-atpt (benchmark-run (ar-regexp-atpt "foo"))
>> :thing-at-point-looking-at (benchmark-run
>> (thing-at-point-looking-at "foo"))))
>>
>>> Another fix, as a bug showed up when testing (ar-regexp-atpt "[a-z]+"):
>>>
>>> (defun ar-regexp-atpt (regexp)
>>> "Return t if REGEXP matches at or before point, nil otherwise.
>>>
>>> Changes match-data"
>>> (save-excursion
>>> (if (looking-at regexp)
>>> (while
>>> (and (not (bobp))
>>> (or (progn (backward-char) (looking-at regexp))
>>> (forward-char 1))))
>>> (while (not (or (bobp) (backward-char) (looking-at regexp))))
>>> (ar-regexp-atpt regexp))
>> What's this recursive call for? It triggers (error "Lisp nesting
>> exceeds ‘max-lisp-eval-depth’") in the benchmark above.
>>
>>> (looking-at regexp)))
>
>
> The recursive call needed a guard: (unless (bobp)
>
> It is called after function went backward while not looking-at matches,
>
> Now the result for the 99999 is
>
> (:ar-regexp-atpt (0.774574453 0 0.0) :thing-at-point-looking-at
> (0.000798669 0 0.0))
>
>
> The fixed form:
>
>
Make sure match pos includes cursor pos:
(defun ar-regexp-atpt (regexp)
"Return t if REGEXP matches at or before point, nil otherwise.
Changes match-data"
(save-excursion
(let ((orig (point)))
(if (looking-at regexp)
(while
(and (not (bobp))
(or (progn (backward-char) (looking-at regexp))
(forward-char 1))))
(while (not (or (bobp) (backward-char) (looking-at regexp))))
(unless (bobp) (ar-regexp-atpt regexp)))
(and
(looking-at regexp)
(<= (match-beginning 0) orig)
(>= (match-end 0) orig)))))
[-- Attachment #2: Type: text/html, Size: 4943 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#35708: [27.0.50]: thingatpt.el, thing-at-point-looking-at redundant
2019-05-15 6:59 ` Andreas Röhler
2019-05-15 10:11 ` Andreas Röhler
@ 2019-05-20 17:17 ` Noam Postavsky
1 sibling, 0 replies; 10+ messages in thread
From: Noam Postavsky @ 2019-05-20 17:17 UTC (permalink / raw)
To: Andreas Röhler; +Cc: 35708
tags 35708 notabug wontfix
close 35708
quit
Andreas Röhler <andreas.roehler@easy-emacs.de> writes:
> Now the result for the 99999 is
>
> (:ar-regexp-atpt (0.774574453 0 0.0)
> :thing-at-point-looking-at (0.000798669 0 0.0))
I trust that explains well enough why the current implementation is
used.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-05-20 17:17 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-05-13 7:19 bug#35708: [27.0.50]: thingatpt.el, thing-at-point-looking-at redundant Andreas Röhler
2019-05-13 12:41 ` Noam Postavsky
2019-05-13 18:31 ` Andreas Röhler
2019-05-13 19:25 ` npostavs
2019-05-14 10:02 ` Andreas Röhler
2019-05-14 10:49 ` Andreas Röhler
2019-05-14 14:34 ` npostavs
2019-05-15 6:59 ` Andreas Röhler
2019-05-15 10:11 ` Andreas Röhler
2019-05-20 17:17 ` Noam Postavsky
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).