bug#24605: Subject: 25.1.50; form-at-point might fail for some THINGS

bug#24605: Subject: 25.1.50; form-at-point might fail for some THINGS

[Tino Calancha]

[-- Attachment #1: Type: text/plain, Size: 2532 bytes --]


Because `thing-at-point' doesn't return always a string, as
`form-at-point' currently assumes.  It might return a different
type depending on what you have in
(get thing 'thing-at-point)

emacs -Q:

(with-temp-buffer
   (require 'thingatpt)
   (insert "1")
   (let ((res
          (cond ((and (number-at-point)
                      (form-at-point 'number)) 0)
                ((and (number-at-point)
                      (not (form-at-point 'number))) -1)
                ((and (not (number-at-point))
                      (form-at-point 'number)) -2)
                ((and (not (number-at-point))
                      (not (form-at-point 'number))) -3))))
     (pcase res
       ('0 (message "OK, both 'number-at-point and 'form-at-point successfull"))
       (-1 (message "'number-at-point' works but (form-at-point 'number) fails"))
       (-2 (message "(form-at-point 'number) works but 'number-at-point' fails"))
       (-3 (message "Both 'number-at-point' and (form-at-point 'number) fails")))))
=> "’number-at-point’ works but (form-at-point ’number) fails"

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
From d587d00fc1379b8bd2511d5491c0c664669fef87 Mon Sep 17 00:00:00 2001
From: Tino Calancha <tino.calancha@gmail.com>
Date: Tue, 4 Oct 2016 14:48:36 +0900
Subject: [PATCH] form-at-point work for all kind of THINGS

* lisp/thingatpt.el (form-at-point):
Use thing-at-point--read-from-whole-string only if thing-at-point
returns a string (Bug#24605).
---
  lisp/thingatpt.el | 8 +++++---
  1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/lisp/thingatpt.el b/lisp/thingatpt.el
index df5c52d..304df7b 100644
--- a/lisp/thingatpt.el
+++ b/lisp/thingatpt.el
@@ -586,9 +586,11 @@ 'read-from-whole-string
    "This is an internal thingatpt function and should not be used.")

  (defun form-at-point (&optional thing pred)
-  (let ((sexp (ignore-errors
-		(thing-at-point--read-from-whole-string
-		 (thing-at-point (or thing 'sexp))))))
+  (let* ((obj (thing-at-point (or thing 'sexp)))
+         (sexp (ignore-errors
+                 (if (stringp obj)
+                     (thing-at-point--read-from-whole-string obj)
+                   obj))))
      (if (or (not pred) (funcall pred sexp)) sexp)))

  ;;;###autoload
-- 
2.9.3

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

In GNU Emacs 25.1.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.22.0)
  of 2016-10-04 built on calancha-pc
Repository revision: 74b4f13842f3119f98797ea76d9be42457b330e1

bug#24605: Subject: 25.1.50; form-at-point might fail for some THINGS

[npostavs]

tags 24605 patch
severity 24605 minor
quit

Tino Calancha <tino.calancha@gmail.com> writes:

> Because `thing-at-point' doesn't return always a string, as
> `form-at-point' currently assumes.
[...]
>
> (with-temp-buffer
>   (require 'thingatpt)
>   (insert "1")
>   (let ((res
>          (cond ((and (number-at-point)
>                      (form-at-point 'number)) 0)
>                ((and (number-at-point)
>                      (not (form-at-point 'number))) -1)
>                ((and (not (number-at-point))
>                      (form-at-point 'number)) -2)
>                ((and (not (number-at-point))
>                      (not (form-at-point 'number))) -3))))
>     (pcase res
>       ('0 (message "OK, both 'number-at-point and 'form-at-point successfull"))
>       (-1 (message "'number-at-point' works but (form-at-point 'number) fails"))
>       (-2 (message "(form-at-point 'number) works but 'number-at-point' fails"))
>       (-3 (message "Both 'number-at-point' and (form-at-point 'number) fails")))))
> => "’number-at-point’ works but (form-at-point ’number) fails"

Well, it looks like `form-at-point' was assumed to be used only for
implementing the foo-at-point functions, (form-at-point 'sexp 'numberp)
does work fine for this.  But we may as well let (form-at-point 'number)
work too, since it's the more obvious way to call it.

>  (defun form-at-point (&optional thing pred)
> -  (let ((sexp (ignore-errors
> -		(thing-at-point--read-from-whole-string
> -		 (thing-at-point (or thing 'sexp))))))
> +  (let* ((obj (thing-at-point (or thing 'sexp)))
> +         (sexp (ignore-errors
> +                 (if (stringp obj)
> +                     (thing-at-point--read-from-whole-string obj)
> +                   obj))))

I suggest keeping the `ignore-errors' strictly around the
`thing-at-point--read-from-whole-string' call.

>      (if (or (not pred) (funcall pred sexp)) sexp)))

bug#24605: Subject: 25.1.50; form-at-point might fail for some THINGS

[Tino Calancha]

Thank you for the e-mail.

On Tue, 4 Oct 2016, npostavs@users.sourceforge.net wrote:

> Well, it looks like `form-at-point' was assumed to be used only for
> implementing the foo-at-point functions, (form-at-point 'sexp 'numberp)
> does work fine for this.  But we may as well let (form-at-point 'number)
> work too, since it's the more obvious way to call it.

In my case i use lib thingatpt+, so i have:
(get 'list 'thing-at-point)
=> tap-list-at-point

That causes `list-at-point' always fails regardless on where
it is the point, for the same reason: `tap-list-at-point' returns
a cons not an string.

> I suggest keeping the `ignore-errors' strictly around the
> `thing-at-point--read-from-whole-string' call.

Ok, corrected the patch:

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
From ea4c09df929451f4268ea4d608ee11bb4249854f Mon Sep 17 00:00:00 2001
From: Tino Calancha <tino.calancha@gmail.com>
Date: Wed, 5 Oct 2016 12:25:07 +0900
Subject: [PATCH] form-at-point work for all kind of THINGS

* lisp/thingatpt.el (form-at-point):
Use thing-at-point--read-from-whole-string only if thing-at-point
returns a string (Bug#24605).
---
  lisp/thingatpt.el | 8 +++++---
  1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/lisp/thingatpt.el b/lisp/thingatpt.el
index df5c52d..6d1014b 100644
--- a/lisp/thingatpt.el
+++ b/lisp/thingatpt.el
@@ -586,9 +586,11 @@ 'read-from-whole-string
    "This is an internal thingatpt function and should not be used.")

  (defun form-at-point (&optional thing pred)
-  (let ((sexp (ignore-errors
-		(thing-at-point--read-from-whole-string
-		 (thing-at-point (or thing 'sexp))))))
+  (let* ((obj (thing-at-point (or thing 'sexp)))
+         (sexp (if (stringp obj)
+                   (ignore-errors
+                     (thing-at-point--read-from-whole-string obj))
+                 obj)))
      (if (or (not pred) (funcall pred sexp)) sexp)))

  ;;;###autoload
-- 
2.9.3

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
In GNU Emacs 25.1.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.22.0)
  of 2016-10-04 built on calancha-pc
Repository revision: e2913dc880b9843bf69cf885270551bafeb46120

bug#24605: Subject: 25.1.50; form-at-point might fail for some THINGS

[Noam Postavsky]

On Tue, Oct 4, 2016 at 11:35 PM, Tino Calancha <tino.calancha@gmail.com> wrote:
> Ok, corrected the patch:

Looks good to me.

bug#24605: Subject: 25.1.50; form-at-point might fail for some THINGS

[Tino Calancha]

On Wed, 5 Oct 2016, Noam Postavsky wrote:

> On Tue, Oct 4, 2016 at 11:35 PM, Tino Calancha <tino.calancha@gmail.com> wrote:
>> Ok, corrected the patch:
>
> Looks good to me.
Thank you very much.
I will push it to master branch in a few days if i don't see further 
comments.

bug#24605: Subject: 25.1.50; form-at-point might fail for some THINGS

[Tino Calancha]

On Wed, 5 Oct 2016, Tino Calancha wrote:

>> Looks good to me.
> Thank you very much.
> I will push it to master branch in a few days if i don't see further 
> comments.
Opps i said master because i am too used to say that.
Probably the fix should go to the emacs-25 branch, right?

bug#24605: Subject: 25.1.50; form-at-point might fail for some THINGS

[Eli Zaretskii]

> From: Tino Calancha <tino.calancha@gmail.com>
> Date: Wed, 5 Oct 2016 21:38:16 +0900 (JST)
> Cc: 24605@debbugs.gnu.org, Noam Postavsky <npostavs@users.sourceforge.net>
> 
> > I will push it to master branch in a few days if i don't see further 
> > comments.
> Opps i said master because i am too used to say that.
> Probably the fix should go to the emacs-25 branch, right?

It depends on when was this problem introduced.  Can you tell?

bug#24605: Subject: 25.1.50; form-at-point might fail for some THINGS

[Tino Calancha]

On Wed, 5 Oct 2016, Eli Zaretskii wrote:

>> From: Tino Calancha <tino.calancha@gmail.com>
>> Date: Wed, 5 Oct 2016 21:38:16 +0900 (JST)
>> Cc: 24605@debbugs.gnu.org, Noam Postavsky <npostavs@users.sourceforge.net>
>>
>>> I will push it to master branch in a few days if i don't see further
>>> comments.
>> Opps i said master because i am too used to say that.
>> Probably the fix should go to the emacs-25 branch, right?
>
> It depends on when was this problem introduced.  Can you tell?
The problem in my snippet code appear since commit 748b0d84 (Emacs-24).
That solves my question: it should go to the master branch.
Thank you.

bug#24605: Subject: 25.1.50; form-at-point might fail for some THINGS

[Eli Zaretskii]

> From: Tino Calancha <tino.calancha@gmail.com>
> Date: Wed, 5 Oct 2016 22:52:37 +0900 (JST)
> cc: Tino Calancha <tino.calancha@gmail.com>, 24605@debbugs.gnu.org, 
>     npostavs@users.sourceforge.net
> 
> > It depends on when was this problem introduced.  Can you tell?
> The problem in my snippet code appear since commit 748b0d84 (Emacs-24).
> That solves my question: it should go to the master branch.

Right, thanks.

bug#24605: Subject: 25.1.50; form-at-point might fail for some THINGS

[Andreas Röhler]

On 04.10.2016 08:22, Tino Calancha wrote:
> (with-temp-buffer
>   (require 'thingatpt)
>   (insert "1")

An interesting test-case would be

(with-temp-buffer
   (require 'thingatpt)
   (insert "1a")
   (forward-char -1)
    ...

Than cursor is at "a" and every form fails.

Same with (forward-char -2)

bug#24605: Subject: 25.1.50; form-at-point might fail for some THINGS

[Tino Calancha]

Noam Postavsky <npostavs@users.sourceforge.net> writes:

> Looks good to me.

Pushed fix to master as commit 9640e9f4