GNU bug report logs - #35708
[27.0.50]: thingatpt.el, thing-at-point-looking-at redundant

Previous Next

Package: emacs;

Reported by: Andreas Röhler <andreas.roehler <at> easy-emacs.de>

Date: Mon, 13 May 2019 07:19:01 UTC

Severity: wishlist

Tags: notabug, wontfix

Done: Noam Postavsky <npostavs <at> gmail.com>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 35708 in the body.
You can then email your comments to 35708 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#35708; Package emacs. (Mon, 13 May 2019 07:19:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Andreas Röhler <andreas.roehler <at> easy-emacs.de>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Mon, 13 May 2019 07:19:02 GMT) Full text and rfc822 format available.

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

From: Andreas Röhler <andreas.roehler <at> easy-emacs.de>
To: bug-gnu-emacs <at> gnu.org
Subject: [27.0.50]: thingatpt.el, thing-at-point-looking-at redundant
Date: Mon, 13 May 2019 09:19:20 +0200
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





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#35708; Package emacs. (Mon, 13 May 2019 12:42:02 GMT) Full text and rfc822 format available.

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

From: Noam Postavsky <npostavs <at> gmail.com>
To: Andreas Röhler <andreas.roehler <at> easy-emacs.de>
Cc: 35708 <at> debbugs.gnu.org
Subject: Re: bug#35708: [27.0.50]: thingatpt.el,
 thing-at-point-looking-at redundant
Date: Mon, 13 May 2019 08:41:03 -0400
[Message part 1 (text/plain, inline)]
Andreas Röhler <andreas.roehler <at> 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?

[0001-Add-thing-at-point-looking-at-test-Bug-35708.patch (text/x-diff, inline)]
From 79b55e8e6dfee9cba9e464860546dbab2cdd36d8 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs <at> 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


Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#35708; Package emacs. (Mon, 13 May 2019 18:32:03 GMT) Full text and rfc822 format available.

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

From: Andreas Röhler <andreas.roehler <at> easy-emacs.de>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: 35708 <at> debbugs.gnu.org
Subject: Re: bug#35708: [27.0.50]: thingatpt.el, thing-at-point-looking-at
 redundant
Date: Mon, 13 May 2019 20:31:54 +0200
On 13.05.19 14:41, Noam Postavsky wrote:
> Andreas Röhler <andreas.roehler <at> 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)))






Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#35708; Package emacs. (Mon, 13 May 2019 19:26:01 GMT) Full text and rfc822 format available.

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

From: npostavs <at> gmail.com
To: Andreas Röhler <andreas.roehler <at> easy-emacs.de>
Cc: 35708 <at> debbugs.gnu.org, Noam Postavsky <npostavs <at> gmail.com>
Subject: Re: bug#35708: [27.0.50]: thingatpt.el,
 thing-at-point-looking-at redundant
Date: Mon, 13 May 2019 15:25:38 -0400
Andreas Röhler <andreas.roehler <at> 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.






Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#35708; Package emacs. (Tue, 14 May 2019 10:02:01 GMT) Full text and rfc822 format available.

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

From: Andreas Röhler <andreas.roehler <at> easy-emacs.de>
To: npostavs <at> gmail.com
Cc: 35708 <at> debbugs.gnu.org
Subject: Re: bug#35708: [27.0.50]: thingatpt.el, thing-at-point-looking-at
 redundant
Date: Tue, 14 May 2019 12:02:15 +0200
On 13.05.19 21:25, npostavs <at> gmail.com wrote:
> Andreas Röhler <andreas.roehler <at> 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)))





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#35708; Package emacs. (Tue, 14 May 2019 10:49:01 GMT) Full text and rfc822 format available.

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

From: Andreas Röhler <andreas.roehler <at> easy-emacs.de>
To: npostavs <at> gmail.com
Cc: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#35708: [27.0.50]: thingatpt.el, thing-at-point-looking-at
 redundant
Date: Tue, 14 May 2019 12:49:07 +0200
On 14.05.19 12:02, Andreas Röhler wrote:
>
> On 13.05.19 21:25, npostavs <at> gmail.com wrote:
>> Andreas Röhler <andreas.roehler <at> 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)))






Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#35708; Package emacs. (Tue, 14 May 2019 14:36:02 GMT) Full text and rfc822 format available.

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

From: npostavs <at> gmail.com
To: Andreas Röhler <andreas.roehler <at> easy-emacs.de>
Cc: 35708 <at> debbugs.gnu.org, npostavs <at> gmail.com
Subject: Re: bug#35708: [27.0.50]: thingatpt.el,
 thing-at-point-looking-at redundant
Date: Tue, 14 May 2019 10:34:55 -0400
Andreas Röhler <andreas.roehler <at> 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)))




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#35708; Package emacs. (Wed, 15 May 2019 07:00:02 GMT) Full text and rfc822 format available.

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

From: Andreas Röhler <andreas.roehler <at> easy-emacs.de>
To: npostavs <at> gmail.com
Cc: 35708 <at> debbugs.gnu.org
Subject: Re: bug#35708: [27.0.50]: thingatpt.el, thing-at-point-looking-at
 redundant
Date: Wed, 15 May 2019 08:59:19 +0200
Am 14.05.19 um 16:34 schrieb npostavs <at> gmail.com:
> Andreas Röhler <andreas.roehler <at> 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)))






Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#35708; Package emacs. (Wed, 15 May 2019 10:12:01 GMT) Full text and rfc822 format available.

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

From: Andreas Röhler <andreas.roehler <at> easy-emacs.de>
To: npostavs <at> gmail.com
Cc: 35708 <at> debbugs.gnu.org
Subject: Re: bug#35708: [27.0.50]: thingatpt.el, thing-at-point-looking-at
 redundant
Date: Wed, 15 May 2019 12:11:05 +0200
[Message part 1 (text/plain, inline)]
Am 15.05.19 um 08:59 schrieb Andreas Röhler:
>
> Am 14.05.19 um 16:34 schrieb npostavs <at> gmail.com:
>> Andreas Röhler <andreas.roehler <at> 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)))))


[Message part 2 (text/html, inline)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#35708; Package emacs. (Mon, 20 May 2019 17:18:01 GMT) Full text and rfc822 format available.

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

From: Noam Postavsky <npostavs <at> gmail.com>
To: Andreas Röhler <andreas.roehler <at> easy-emacs.de>
Cc: 35708 <at> debbugs.gnu.org
Subject: Re: bug#35708: [27.0.50]: thingatpt.el,
 thing-at-point-looking-at redundant
Date: Mon, 20 May 2019 13:17:33 -0400
tags 35708 notabug wontfix
close 35708
quit

Andreas Röhler <andreas.roehler <at> 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.




Added tag(s) notabug and wontfix. Request was from Noam Postavsky <npostavs <at> gmail.com> to control <at> debbugs.gnu.org. (Mon, 20 May 2019 17:18:02 GMT) Full text and rfc822 format available.

bug closed, send any further explanations to 35708 <at> debbugs.gnu.org and Andreas Röhler <andreas.roehler <at> easy-emacs.de> Request was from Noam Postavsky <npostavs <at> gmail.com> to control <at> debbugs.gnu.org. (Mon, 20 May 2019 17:18:02 GMT) Full text and rfc822 format available.

bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Tue, 18 Jun 2019 11:24:04 GMT) Full text and rfc822 format available.

This bug report was last modified 4 years and 285 days ago.

Previous Next


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