bug#24627: 24.5; (thing-at-point 'list) may return a non-empty string without a list

bug#24627: 24.5; (thing-at-point 'list) may return a non-empty string without a list

[Tino Calancha]

From: Tino Calancha <tino.calancha@gmail.com>
To: bug-gnu-emacs@gnu.org
Subject: 24.5; (thing-at-point 'list) make return a non-empty string without a list
--text follows this line--

emacs -Q thingatpt.el
> C-p
M-: (thing-at-point 'list t) RET

=> ";;; thingatpt.el ends here\n"

(list-at-point) returns nil as it should.
Same behavior in master branch.

In GNU Emacs 24.5.1 (x86_64-unknown-linux-gnu, GTK+ Version 3.20.5)
  of 2016-06-02 on calancha-pc
Windowing system distributor `The X.Org Foundation', version 11.0.11804000
System Description:	Debian GNU/Linux testing (stretch)

Important settings:
   value of $LANG: en_US.UTF-8
   locale-coding-system: utf-8-unix

Major mode: Emacs-Lisp

Minor modes in effect:
   tooltip-mode: t
   electric-indent-mode: t
   mouse-wheel-mode: t
   tool-bar-mode: t
   menu-bar-mode: t
   file-name-shadow-mode: t
   global-font-lock-mode: t
   font-lock-mode: t
   blink-cursor-mode: t
   auto-composition-mode: t
   auto-encryption-mode: t
   auto-compression-mode: t
   line-number-mode: t
   transient-mark-mode: t

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Mark set
";;; thingatpt.el ends here
"

Load-path shadows:
None found.

Features:
(shadow sort gnus-util mail-extr emacsbug message format-spec rfc822 mml
easymenu mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231
mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums
mm-util help-fns mail-prsvr mail-utils thingatpt time-date tooltip
electric uniquify ediff-hook vc-hooks lisp-float-type mwheel x-win x-dnd
tool-bar dnd fontset image regexp-opt fringe tabulated-list newcomment
lisp-mode prog-mode register page menu-bar rfn-eshadow timer select
scroll-bar mouse jit-lock font-lock syntax facemenu font-core frame cham
georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese hebrew greek romanian slovak czech european ethiopic
indian cyrillic chinese case-table epa-hook jka-cmpr-hook help simple
abbrev minibuffer nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote make-network-process
dbusbind gfilenotify dynamic-setting system-font-setting
font-render-setting move-toolbar gtk x-toolkit x multi-tty emacs)

Memory information:
((conses 16 72272 4417)
  (symbols 48 17618 0)
  (miscs 40 40 138)
  (strings 32 9497 4205)
  (string-bytes 1 260150)
  (vectors 16 8998)
  (vector-slots 8 384193 16741)
  (floats 8 64 85)
  (intervals 56 290 1)
  (buffers 960 12)
  (heap 1024 9017 976))

bug#24627: 24.5; (thing-at-point 'list) may return a non-empty string without a list

[Andreas Röhler]

On 06.10.2016 11:43, Tino Calancha wrote:

bounds-of-thing-at-point:

       ;; Try moving forward, then back.
       (funcall ;; First move to end.
        (or (get thing 'end-op)

Still think bounds-of-thing-at-point should move backward first

parse-partial-sexp offers some handy info for beginning, not end

bug#24627: 24.5; (thing-at-point 'list) may return a non-empty string without a list

[Tino Calancha]

Andreas Röhler <andreas.roehler@easy-emacs.de> writes:

> On 06.10.2016 11:43, Tino Calancha wrote:
>
> bounds-of-thing-at-point:
>
>       ;; Try moving forward, then back.
>       (funcall ;; First move to end.
>        (or (get thing 'end-op)
>
> Still think bounds-of-thing-at-point should move backward first
>
> parse-partial-sexp offers some handy info for beginning, not end

I agree with you that `bounds-of-thing-at-point' needs some work.  I have
noticed other issues with it.  We might work on them once we fix this bug.

In the example in this thread the problem arise because
`thing-at-point-bounds-of-list-at-point', which is the actual
function doing the job here.  We need to fix this function.
Without it, the previous example works:

emacs -Q thingatpt.el -eval "(require 'thingatpt)"
M-: (put 'list 'bounds-of-thing-at-point nil) RET
> C-p
M-: (thing-at-point 'list t) RET
=> nil

bug#24627: 24.5; (thing-at-point 'list) may return a non-empty string without a list

[Andreas Röhler]

On 11.10.2016 05:42, Tino Calancha wrote:
> Andreas Röhler <andreas.roehler@easy-emacs.de> writes:
>
>> On 06.10.2016 11:43, Tino Calancha wrote:
>>
>> bounds-of-thing-at-point:
>>
>>        ;; Try moving forward, then back.
>>        (funcall ;; First move to end.
>>         (or (get thing 'end-op)
>>
>> Still think bounds-of-thing-at-point should move backward first
>>
>> parse-partial-sexp offers some handy info for beginning, not end
> I agree with you that `bounds-of-thing-at-point' needs some work.  I have
> noticed other issues with it.  We might work on them once we fix this bug.
>
> In the example in this thread the problem arise because
> `thing-at-point-bounds-of-list-at-point', which is the actual
> function doing the job here.  We need to fix this function.
> Without it, the previous example works:
>
> emacs -Q thingatpt.el -eval "(require 'thingatpt)"
> M-: (put 'list 'bounds-of-thing-at-point nil) RET
>> C-p
> M-: (thing-at-point 'list t) RET
> => nil

With

(defun foo ())

at first open paren

M-: (thing-at-point 'list t) RET

returns the whole thing, right.

But at second open paren returns the whole function too - where it 
should return the empty arg-list

bug#24627: 24.5; (thing-at-point 'list) may return a non-empty string without a list

[Tino Calancha]

Andreas Röhler <andreas.roehler@easy-emacs.de> writes:

> With
>
> (defun foo ())
>
> at first open paren
>
> M-: (thing-at-point 'list t) RET
>
> returns the whole thing, right.
>
> But at second open paren returns the whole function too - where it
> should return the empty arg-list

Following patch do the right thing in my example and your example.
Feel free to check it with some other examples that you may have:

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
From dc2703a43a7b5dae60f7e88805971472253d4600 Mon Sep 17 00:00:00 2001
From: Tino Calancha <tino.calancha@gmail.com>
Date: Wed, 12 Oct 2016 01:16:52 +0900
Subject: [PATCH] (thing-at-point 'list) return nil if no list at point

* lisp/thingatpt.el (thing-at-point-bounds-of-list-at-point):
Check first if we are at the beginning of a top-level sexp (Bug#24627).
Escape '[' and ']' in doc string.
---
 lisp/thingatpt.el | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/lisp/thingatpt.el b/lisp/thingatpt.el
index 6d1014b..acacff2 100644
--- a/lisp/thingatpt.el
+++ b/lisp/thingatpt.el
@@ -219,22 +219,18 @@ 'beginning-of-sexp
 
 (defun thing-at-point-bounds-of-list-at-point ()
   "Return the bounds of the list at point.
-[Internal function used by `bounds-of-thing-at-point'.]"
+\[Internal function used by `bounds-of-thing-at-point'.\]"
   (save-excursion
     (let ((opoint (point))
-	  (beg (ignore-errors
-		 (up-list -1)
-		 (point))))
+          (beg (if (looking-at-p "(")
+                   (point)
+                 (ignore-errors
+                   (up-list -1)
+                   (point)))))
       (ignore-errors
-	(if beg
-	    (progn (forward-sexp)
-		   (cons beg (point)))
-	  ;; Are we are at the beginning of a top-level sexp?
-	  (forward-sexp)
-	  (let ((end (point)))
-	    (backward-sexp)
-	    (if (>= opoint (point))
-		(cons opoint end))))))))
+        (when beg
+          (forward-sexp)
+          (cons beg (point)))))))
 
 ;; Defuns
 
-- 
2.9.3

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

In GNU Emacs 26.0.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.22.1)
 of 2016-10-11
Repository revision: 9640e9f4e95cd95c04875e90a4ff638e1e51f977

bug#24627: 24.5; (thing-at-point 'list) may return a non-empty string without a list

[Noam Postavsky]

On Tue, Oct 11, 2016 at 12:29 PM, Tino Calancha <tino.calancha@gmail.com> wrote:
> Escape '[' and ']' in doc string.

Huh? Why?

> -[Internal function used by `bounds-of-thing-at-point'.]"
> +\[Internal function used by `bounds-of-thing-at-point'.\]"

bug#24627: 24.5; (thing-at-point 'list) may return a non-empty string without a list

[Tino Calancha]

On Tue, 11 Oct 2016, Noam Postavsky wrote:

> On Tue, Oct 11, 2016 at 12:29 PM, Tino Calancha <tino.calancha@gmail.com> wrote:
>> Escape '[' and ']' in doc string.
>
> Huh? Why?
>
>> -[Internal function used by `bounds-of-thing-at-point'.]"
>> +\[Internal function used by `bounds-of-thing-at-point'.\]"
Right, i should just escape the open '[', as follows:
+\[Internal function used by `bounds-of-thing-at-point'.]"

bug#24627: 24.5; (thing-at-point 'list) may return a non-empty string without a list

[Noam Postavsky]

On Tue, Oct 11, 2016 at 1:09 PM, Tino Calancha <tino.calancha@gmail.com> wrote:
>
>
> On Tue, 11 Oct 2016, Noam Postavsky wrote:
>
>> On Tue, Oct 11, 2016 at 12:29 PM, Tino Calancha <tino.calancha@gmail.com>
>> wrote:
>>>
>>> Escape '[' and ']' in doc string.
>>
>>
>> Huh? Why?
>>
>>> -[Internal function used by `bounds-of-thing-at-point'.]"
>>> +\[Internal function used by `bounds-of-thing-at-point'.\]"
>
> Right, i should just escape the open '[', as follows:
> +\[Internal function used by `bounds-of-thing-at-point'.]"

Oh, I thought only round parens need to be escaped in the 1st column.
Is there something that looks for square brackets too?

bug#24627: "internal" designation [was: bug#24627: 24.5; (thing-at-point 'list) ...]

[Drew Adams]

>  (defun thing-at-point-bounds-of-list-at-point ()
>    "Return the bounds of the list at point.
> -[Internal function used by `bounds-of-thing-at-point'.]"
> +\[Internal function used by `bounds-of-thing-at-point'.\]"

FWIW:

I object to such an "internal" designation being in that doc string.

What's the point of that?  What makes this function particularly
"internal"?  Seems like a gratuitous characterization (at best).
That the function is used by `bounds-of-thing-at-point' is obvious,
and anyway that fact does not belong in its doc string.

IMHO, there is too much of this trying to wall off this or that as
beomg in some sense "internal" (with no explanation, including no
code comment, as to what makes it internal).

The definition typically given for this characterization is that the
thing so designated is _liable to change_.  Big deal - lots of stuff
is liable to change.  Saying that doesn't help anyone.  How liable?
Why liable?

And when such a thing does change, it is likely that other things, not
designated as "internal" also change, including user-visible behavior.

IOW, the thing walled off is often not really internal at all - the
code is not just one implementation of a given (stable, non-internal)
interface.  Typically, there is nothing special or tentative about
the code.

Emacs and Emacs Lisp are things that invite users to dig into and
change them.  Emacs is not your typical software use and development.
(Likewise, free software, BTW: there is no solid separation between
user and developer.)

For Emacs, this "internal" designation is generally a useless crutch,
IMO.  And my impression is that recently (the last several years) its
use has been spread much more.  In the more distant past it was very
rarely resorted to, if at all.  And I don't think anyone suffered
from its lack of use.  No one needed to be warned that this or that
might change.

My sense is that this has been used more and more simply as a way of
warding off users from offering suggestions about the thing that is
so "protected", whether it be requesting better doc or something else.

My estimation of the "internal" label contagion is this: from useless
to nefarious.  It seems unemacsy, trying to put an unnecessary wall
between Emacs development and Emacs users.  There should be little or
no reason for Emacs to tell users "don't use this".

Removing the "internal" nonsense for this function would be a start...

bug#24627: 24.5; (thing-at-point 'list) may return a non-empty string without a list

[Tino Calancha]

On Tue, 11 Oct 2016, Noam Postavsky wrote:

> On Tue, Oct 11, 2016 at 1:09 PM, Tino Calancha <tino.calancha@gmail.com> wrote:
>>
>>
>> On Tue, 11 Oct 2016, Noam Postavsky wrote:
>>
>>> On Tue, Oct 11, 2016 at 12:29 PM, Tino Calancha <tino.calancha@gmail.com>
>>> wrote:
>>>>
>>>> Escape '[' and ']' in doc string.
>>>
>>>
>>> Huh? Why?
>>>
>>>> -[Internal function used by `bounds-of-thing-at-point'.]"
>>>> +\[Internal function used by `bounds-of-thing-at-point'.\]"
>>
>> Right, i should just escape the open '[', as follows:
>> +\[Internal function used by `bounds-of-thing-at-point'.]"
>
> Oh, I thought only round parens need to be escaped in the 1st column.
> Is there something that looks for square brackets too?
I thought the same than you.  I just discovered in this bug that square
parens also matters:
Compare I), II) w/o and w/ escaped '[':
I) It confuses `beginning-of-defun':
emacs -Q thingatpt.el
M-g g 229 RET
C-M-a ; go to line 222 instead of line 220.

II) It confuses `indent-for-tab-command':
emacs -Q thingatpt.el
M-g g 220 RET
C-SPC C-M-e TAB ; indentation doesn't change and it looks wrong.

bug#24627: "internal" designation [was: bug#24627: 24.5; (thing-at-point 'list) ...]

[Drew Adams]

Sorry - I meant to send my reply about "internal" to emacs-devel, not here.

bug#24627: 24.5; (thing-at-point 'list) may return a non-empty string without a list

[Andreas Röhler]

On 11.10.2016 18:29, Tino Calancha wrote:
> Andreas Röhler <andreas.roehler@easy-emacs.de> writes:
>
>> With
>>
>> (defun foo ())
>>
>> at first open paren
>>
>> M-: (thing-at-point 'list t) RET
>>
>> returns the whole thing, right.
>>
>> But at second open paren returns the whole function too - where it
>> should return the empty arg-list
> Following patch do the right thing in my example and your example.
> Feel free to check it with some other examples that you may have:
>
> ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
>  From dc2703a43a7b5dae60f7e88805971472253d4600 Mon Sep 17 00:00:00 2001
> From: Tino Calancha <tino.calancha@gmail.com>
> Date: Wed, 12 Oct 2016 01:16:52 +0900
> Subject: [PATCH] (thing-at-point 'list) return nil if no list at point
>
> * lisp/thingatpt.el (thing-at-point-bounds-of-list-at-point):
> Check first if we are at the beginning of a top-level sexp (Bug#24627).
> Escape '[' and ']' in doc string.
> ---
>   lisp/thingatpt.el | 22 +++++++++-------------
>   1 file changed, 9 insertions(+), 13 deletions(-)
>
> diff --git a/lisp/thingatpt.el b/lisp/thingatpt.el
> index 6d1014b..acacff2 100644
> --- a/lisp/thingatpt.el
> +++ b/lisp/thingatpt.el
> @@ -219,22 +219,18 @@ 'beginning-of-sexp
>   
>   (defun thing-at-point-bounds-of-list-at-point ()
>     "Return the bounds of the list at point.
> -[Internal function used by `bounds-of-thing-at-point'.]"
> +\[Internal function used by `bounds-of-thing-at-point'.\]"
>     (save-excursion
>       (let ((opoint (point))
> -	  (beg (ignore-errors
> -		 (up-list -1)
> -		 (point))))
> +          (beg (if (looking-at-p "(")
> +                   (point)
> +                 (ignore-errors
> +                   (up-list -1)
> +                   (point)))))
>         (ignore-errors
> -	(if beg
> -	    (progn (forward-sexp)
> -		   (cons beg (point)))
> -	  ;; Are we are at the beginning of a top-level sexp?
> -	  (forward-sexp)
> -	  (let ((end (point)))
> -	    (backward-sexp)
> -	    (if (>= opoint (point))
> -		(cons opoint end))))))))
> +        (when beg
> +          (forward-sexp)
> +          (cons beg (point)))))))
>   
>   ;; Defuns
>   

Hmm, what if cursor is inside a string or comment?

BTW "list" might be more universal if understood syntactically
What about writing

(eq 4 (car (syntax-after (point))))

bug#24627: 24.5; (thing-at-point 'list) may return a non-empty string without a list

[Tino Calancha]

Andreas Röhler <andreas.roehler@easy-emacs.de> writes:

> Hmm, what if cursor is inside a string or comment?
The list will be returned anyway as thingatpt always does.
AFAICT, skipping lists inside comments/strings would be a new feature
for this lib: better request that in a separated bug report.


> BTW "list" might be more universal if understood syntactically
> What about writing
>
> (eq 4 (car (syntax-after (point))))
Agreed.  Thank you!
Here is the new patch:

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
From 71da9ad4f6bbc307c5fb3f8bd0c6621312b2d4f4 Mon Sep 17 00:00:00 2001
From: Tino Calancha <tino.calancha@gmail.com>
Date: Wed, 12 Oct 2016 13:49:32 +0900
Subject: [PATCH] (thing-at-point 'list) return nil if no list at point

* lisp/thingatpt.el (thing-at-point-bounds-of-list-at-point):
Check first if we are at the beginning of a top-level sexp (Bug#24627).
Escape '[' in doc string.
---
 lisp/thingatpt.el | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/lisp/thingatpt.el b/lisp/thingatpt.el
index 6d1014b..421dcde 100644
--- a/lisp/thingatpt.el
+++ b/lisp/thingatpt.el
@@ -219,22 +219,18 @@ 'beginning-of-sexp
 
 (defun thing-at-point-bounds-of-list-at-point ()
   "Return the bounds of the list at point.
-[Internal function used by `bounds-of-thing-at-point'.]"
+\[Internal function used by `bounds-of-thing-at-point'.]"
   (save-excursion
     (let ((opoint (point))
-	  (beg (ignore-errors
-		 (up-list -1)
-		 (point))))
+          (beg (if (eq 4 (car (syntax-after (point))))
+                   (point)
+                 (ignore-errors
+                   (up-list -1)
+                   (point)))))
       (ignore-errors
-	(if beg
-	    (progn (forward-sexp)
-		   (cons beg (point)))
-	  ;; Are we are at the beginning of a top-level sexp?
-	  (forward-sexp)
-	  (let ((end (point)))
-	    (backward-sexp)
-	    (if (>= opoint (point))
-		(cons opoint end))))))))
+        (when beg
+          (forward-sexp)
+          (cons beg (point)))))))
 
 ;; Defuns
 
-- 
2.9.3

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
In GNU Emacs 26.0.50.2 (x86_64-pc-linux-gnu, GTK+ Version 3.22.1)
 of 2016-10-12
Repository revision: 9640e9f4e95cd95c04875e90a4ff638e1e51f977

bug#24627: 24.5; (thing-at-point 'list) may return a non-empty string without a list

[Andreas Röhler]

On 12.10.2016 06:58, Tino Calancha wrote:
> Andreas Röhler <andreas.roehler@easy-emacs.de> writes:
>
>> Hmm, what if cursor is inside a string or comment?
> The list will be returned anyway as thingatpt always does.
> AFAICT, skipping lists inside comments/strings would be a new feature
> for this lib: better request that in a separated bug report.
>
>
>> BTW "list" might be more universal if understood syntactically
>> What about writing
>>
>> (eq 4 (car (syntax-after (point))))
> Agreed.  Thank you!
> Here is the new patch:
>
> ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
>  From 71da9ad4f6bbc307c5fb3f8bd0c6621312b2d4f4 Mon Sep 17 00:00:00 2001
> From: Tino Calancha <tino.calancha@gmail.com>
> Date: Wed, 12 Oct 2016 13:49:32 +0900
> Subject: [PATCH] (thing-at-point 'list) return nil if no list at point
>
> * lisp/thingatpt.el (thing-at-point-bounds-of-list-at-point):
> Check first if we are at the beginning of a top-level sexp (Bug#24627).
> Escape '[' in doc string.
> ---
>   lisp/thingatpt.el | 22 +++++++++-------------
>   1 file changed, 9 insertions(+), 13 deletions(-)
>
> diff --git a/lisp/thingatpt.el b/lisp/thingatpt.el
> index 6d1014b..421dcde 100644
> --- a/lisp/thingatpt.el
> +++ b/lisp/thingatpt.el
> @@ -219,22 +219,18 @@ 'beginning-of-sexp
>   
>   (defun thing-at-point-bounds-of-list-at-point ()
>     "Return the bounds of the list at point.
> -[Internal function used by `bounds-of-thing-at-point'.]"
> +\[Internal function used by `bounds-of-thing-at-point'.]"
>     (save-excursion
>       (let ((opoint (point))
> -	  (beg (ignore-errors
> -		 (up-list -1)
> -		 (point))))
> +          (beg (if (eq 4 (car (syntax-after (point))))
> +                   (point)
> +                 (ignore-errors
> +                   (up-list -1)
> +                   (point)))))
>         (ignore-errors
> -	(if beg
> -	    (progn (forward-sexp)
> -		   (cons beg (point)))
> -	  ;; Are we are at the beginning of a top-level sexp?
> -	  (forward-sexp)
> -	  (let ((end (point)))
> -	    (backward-sexp)
> -	    (if (>= opoint (point))
> -		(cons opoint end))))))))
> +        (when beg
> +          (forward-sexp)
> +          (cons beg (point)))))))
>   
>   ;; Defuns
>   

beg still needs a check like

(not (nth 8 (parse-partial-sexp (point-min) (point))))

otherwise it could match inside a string or comment

bug#24627: 24.5; (thing-at-point 'list) may return a non-empty string without a list

[Tino Calancha]

Andreas Röhler <andreas.roehler@easy-emacs.de> writes:

> beg still needs a check like
>
> (not (nth 8 (parse-partial-sexp (point-min) (point))))
>
> otherwise it could match inside a string or comment

I have the feeling that this should return the local list
at point, even if inside a string or comment.  Then, if
point is inside a comment/string and there is no list there,
the function might look for a list around (i.e., outside) that
comment/string region.  See patch below.

Anyway, neither the doc string of `thing-at-point' nor
`thing-at-point-bounds-of-list-at-point' mention what expect
when point is inside a comment/string.  That's why i believe it
might be better to request that in a different bug report.
Writting additional tests also might be helpful to find a robust implementation.

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
From bfc9b7fb739dfeab09c2ffd064a6ebe65a28b686 Mon Sep 17 00:00:00 2001
From: Tino Calancha <tino.calancha@gmail.com>
Date: Thu, 13 Oct 2016 16:34:35 +0900
Subject: [PATCH] (thing-at-point 'list) return nil if no list at point

* lisp/thingatpt.el (thing-at-point-bounds-of-list-at-point):
Check first if we are at the beginning of a top-level sexp (Bug#24627).
If found a list inside a comment or string return it.  Otherwise, look
for a list around the comment/string.
Escape '[' in doc string.
---
 lisp/thingatpt.el | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/lisp/thingatpt.el b/lisp/thingatpt.el
index 6d1014b..656d2c7 100644
--- a/lisp/thingatpt.el
+++ b/lisp/thingatpt.el
@@ -219,22 +219,26 @@ 'beginning-of-sexp
 
 (defun thing-at-point-bounds-of-list-at-point ()
   "Return the bounds of the list at point.
-[Internal function used by `bounds-of-thing-at-point'.]"
+\[Internal function used by `bounds-of-thing-at-point'.]"
   (save-excursion
-    (let ((opoint (point))
-	  (beg (ignore-errors
-		 (up-list -1)
-		 (point))))
+    (let* ((opoint (point))
+	   (st (syntax-ppss))
+	   (find-list-fn (lambda ()
+                           (ignore-errors
+                             (up-list -1)
+                             (point))))
+	   (beg (if (eq 4 (car (syntax-after (point))))
+		    (point)
+		  (funcall find-list-fn))))
+      ;; If inside a string or comment and there is no list
+      ;; at point, check for a list surrounding the string/comment region.
+      (when (and (nth 8 st) (= opoint (point)))
+	(goto-char (nth 8 st))
+	(setq beg (funcall find-list-fn)))
       (ignore-errors
-	(if beg
-	    (progn (forward-sexp)
-		   (cons beg (point)))
-	  ;; Are we are at the beginning of a top-level sexp?
-	  (forward-sexp)
-	  (let ((end (point)))
-	    (backward-sexp)
-	    (if (>= opoint (point))
-		(cons opoint end))))))))
+        (when beg
+          (forward-sexp)
+          (cons beg (point)))))))
 
 ;; Defuns
 
-- 
2.9.3

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
In GNU Emacs 26.0.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.22.1)
 of 2016-10-13
Repository revision: 1dd54e3eef7543720eff161457677a35fae2435c

bug#24627: 24.5; (thing-at-point 'list) may return a non-empty string without a list

[Andreas Röhler]

On 13.10.2016 10:50, Tino Calancha wrote:
> Andreas Röhler <andreas.roehler@easy-emacs.de> writes:
>
>> beg still needs a check like
>>
>> (not (nth 8 (parse-partial-sexp (point-min) (point))))
>>
>> otherwise it could match inside a string or comment
> I have the feeling that this should return the local list
> at point, even if inside a string or comment.

Yes, but that would be reported by pps. However, when point is at 
opening delimiter, this is not recognised by pps. Then we must be sure 
not being inside a string or comment, where an opening delimiter is 
meaningless, i.e. just a literal.

IMO all needed is  something like

(beg (or (nth 1 (parse-partial-sexp...))

          (and (eq 4 (car (syntax-after (point))))
               (not (nth 8 (parse-partial-sexp...))
               (point)))))
      

Should both fail, there is not list at point.


>   Then, if
> point is inside a comment/string and there is no list there,
> the function might look for a list around (i.e., outside) that
> comment/string region.  See patch below.
>
> Anyway, neither the doc string of `thing-at-point' nor
> `thing-at-point-bounds-of-list-at-point' mention what expect
> when point is inside a comment/string.  That's why i believe it
> might be better to request that in a different bug report.
> Writting additional tests also might be helpful to find a robust implementation.
>
> ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
>  From bfc9b7fb739dfeab09c2ffd064a6ebe65a28b686 Mon Sep 17 00:00:00 2001
> From: Tino Calancha <tino.calancha@gmail.com>
> Date: Thu, 13 Oct 2016 16:34:35 +0900
> Subject: [PATCH] (thing-at-point 'list) return nil if no list at point
>
> * lisp/thingatpt.el (thing-at-point-bounds-of-list-at-point):
> Check first if we are at the beginning of a top-level sexp (Bug#24627).
> If found a list inside a comment or string return it.  Otherwise, look
> for a list around the comment/string.
> Escape '[' in doc string.
> ---
>   lisp/thingatpt.el | 32 ++++++++++++++++++--------------
>   1 file changed, 18 insertions(+), 14 deletions(-)
>
> diff --git a/lisp/thingatpt.el b/lisp/thingatpt.el
> index 6d1014b..656d2c7 100644
> --- a/lisp/thingatpt.el
> +++ b/lisp/thingatpt.el
> @@ -219,22 +219,26 @@ 'beginning-of-sexp
>   
>   (defun thing-at-point-bounds-of-list-at-point ()
>     "Return the bounds of the list at point.
> -[Internal function used by `bounds-of-thing-at-point'.]"
> +\[Internal function used by `bounds-of-thing-at-point'.]"
>     (save-excursion
> -    (let ((opoint (point))
> -	  (beg (ignore-errors
> -		 (up-list -1)
> -		 (point))))
> +    (let* ((opoint (point))
> +	   (st (syntax-ppss))
> +	   (find-list-fn (lambda ()
> +                           (ignore-errors
> +                             (up-list -1)
> +                             (point))))
> +	   (beg (if (eq 4 (car (syntax-after (point))))
> +		    (point)
> +		  (funcall find-list-fn))))
> +      ;; If inside a string or comment and there is no list
> +      ;; at point, check for a list surrounding the string/comment region.
> +      (when (and (nth 8 st) (= opoint (point)))
> +	(goto-char (nth 8 st))
> +	(setq beg (funcall find-list-fn)))
>         (ignore-errors
> -	(if beg
> -	    (progn (forward-sexp)
> -		   (cons beg (point)))
> -	  ;; Are we are at the beginning of a top-level sexp?
> -	  (forward-sexp)
> -	  (let ((end (point)))
> -	    (backward-sexp)
> -	    (if (>= opoint (point))
> -		(cons opoint end))))))))
> +        (when beg
> +          (forward-sexp)
> +          (cons beg (point)))))))
>   
>   ;; Defuns
>   

bug#24627: 24.5; (thing-at-point 'list) may return a non-empty string without a list

[Tino Calancha]

Andreas Röhler <andreas.roehler@easy-emacs.de> writes:

> On 13.10.2016 10:50, Tino Calancha wrote:
>> Andreas Röhler <andreas.roehler@easy-emacs.de> writes:
>>
>>> beg still needs a check like
>>>
>>> (not (nth 8 (parse-partial-sexp (point-min) (point))))
>>>
>>> otherwise it could match inside a string or comment
>> I have the feeling that this should return the local list
>> at point, even if inside a string or comment.
>
> Yes, but that would be reported by pps. However, when point is at
> opening delimiter, this is not recognised by pps. Then we must be sure
> not being inside a string or comment, where an opening delimiter is
> meaningless, i.e. just a literal.
>
> IMO all needed is  something like
>
> (beg (or (nth 1 (parse-partial-sexp...))
>
>          (and (eq 4 (car (syntax-after (point))))
>               (not (nth 8 (parse-partial-sexp...))
>               (point)))))
>      
>
> Should both fail, there is not list at point.
Thank you.  I think i got what you mean.
I need to invert the order of the above `or':
(nth 1 (parse-partial-sexp...))
need to appear the second.  Otherwise,
(with-temp-buffer
  (insert "(foo (a b) bar)")
  (goto-char 6)
  (list-at-point))

will return:
(foo (a b) bar)
instead of:
(a b)

The new patch pass following test:
(We might want to add this test into test/lisp/thingatpt-tests.el)

(ert-deftest list-at-point-tests ()
  "Test `list-at-point'."
  (let ((string-result '(("(a \"b\" c)" . (a "b" c))
                         (";(a \"b\" c)")
                         ("(a \"b\" c\n)" . (a "b" c))
                         ("\"(a b c)\"")
                         ("(a ;(b c d)\ne)" . (a e))
                         ("(foo\n(a ;(b c d)\ne) bar)" . (a e))
                         ("(foo\na ;(b c d)\ne bar)" . (foo a e bar))
                         ("(foo\n(a \"(b c d)\"\ne) bar)" . (a "(b c d)" e))
                         ("(b\n(a ;(foo c d)\ne) bar)" . (a e))
                         ("(princ \"(a b c)\")" . (princ "(a b c)"))
                         ("(defun foo ()\n  \"Test function.\"\n  ;;(a b)\n  nil)" . (defun foo nil "Test function." nil)))))
    (dolist (str-res string-result)
      (with-temp-buffer
        (emacs-lisp-mode)
        (insert (car str-res))
        (re-search-backward "\\((a\\|^a\\)")
        (should (equal (list-at-point)
                       (cdr str-res)))))))


This is the new patch:
please, let me know if it's OK for you and feel free to suggest
additional tests.

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
From 693aeed2a7251d23885ee53db9bf7026c7c1af3f Mon Sep 17 00:00:00 2001
From: Tino Calancha <tino.calancha@gmail.com>
Date: Sat, 15 Oct 2016 18:21:36 +0900
Subject: [PATCH] (thing-at-point 'list) return nil if no list at point

* lisp/thingatpt.el (thing-at-point-bounds-of-list-at-point):
Check first if we are at the beginning of a top-level sexp (Bug#24627).
If point is inside a comment or string, look for a list out of the
comment/string.
Escape '[' in doc string.
---
 lisp/thingatpt.el | 25 ++++++++++---------------
 1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/lisp/thingatpt.el b/lisp/thingatpt.el
index 6d1014b..e423630 100644
--- a/lisp/thingatpt.el
+++ b/lisp/thingatpt.el
@@ -219,22 +219,17 @@ 'beginning-of-sexp
 
 (defun thing-at-point-bounds-of-list-at-point ()
   "Return the bounds of the list at point.
-[Internal function used by `bounds-of-thing-at-point'.]"
+\[Internal function used by `bounds-of-thing-at-point'.]"
   (save-excursion
-    (let ((opoint (point))
-	  (beg (ignore-errors
-		 (up-list -1)
-		 (point))))
-      (ignore-errors
-	(if beg
-	    (progn (forward-sexp)
-		   (cons beg (point)))
-	  ;; Are we are at the beginning of a top-level sexp?
-	  (forward-sexp)
-	  (let ((end (point)))
-	    (backward-sexp)
-	    (if (>= opoint (point))
-		(cons opoint end))))))))
+    (let* ((st (parse-partial-sexp (point-min) (point)))
+           (beg (or (and (eq 4 (car (syntax-after (point))))
+                         (not (nth 8 st))
+                         (point))
+                    (nth 1 st))))
+      (when beg
+        (goto-char beg)
+        (forward-sexp)
+        (cons beg (point))))))
 
 ;; Defuns
 
-- 
2.9.3

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

In GNU Emacs 26.0.50.4 (x86_64-pc-linux-gnu, GTK+ Version 3.22.1)
 of 2016-10-15
Repository revision: b0f1d23ec482aa71a0ae0251f6f44f4b8d261259

bug#24627: 24.5; (thing-at-point 'list) may return a non-empty string without a list

[Andreas Röhler]

On 15.10.2016 11:44, Tino Calancha wrote:
> Andreas Röhler <andreas.roehler@easy-emacs.de> writes:
>
>> On 13.10.2016 10:50, Tino Calancha wrote:
>>> Andreas Röhler <andreas.roehler@easy-emacs.de> writes:
>>>
>>>> beg still needs a check like
>>>>
>>>> (not (nth 8 (parse-partial-sexp (point-min) (point))))
>>>>
>>>> otherwise it could match inside a string or comment
>>> I have the feeling that this should return the local list
>>> at point, even if inside a string or comment.
>> Yes, but that would be reported by pps. However, when point is at
>> opening delimiter, this is not recognised by pps. Then we must be sure
>> not being inside a string or comment, where an opening delimiter is
>> meaningless, i.e. just a literal.
>>
>> IMO all needed is  something like
>>
>> (beg (or (nth 1 (parse-partial-sexp...))
>>
>>           (and (eq 4 (car (syntax-after (point))))
>>                (not (nth 8 (parse-partial-sexp...))
>>                (point)))))
>>       
>>
>> Should both fail, there is not list at point.
> Thank you.  I think i got what you mean.
> I need to invert the order of the above `or':
> (nth 1 (parse-partial-sexp...))
> need to appear the second.  Otherwise,
> (with-temp-buffer
>    (insert "(foo (a b) bar)")
>    (goto-char 6)
>    (list-at-point))
>
> will return:
> (foo (a b) bar)
> instead of:
> (a b)

Ah, good catch. Thanks back.