unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#24627: 24.5; (thing-at-point 'list) may return a non-empty string without a list
@ 2016-10-06  9:43 Tino Calancha
  2016-10-07  6:42 ` Andreas Röhler
  0 siblings, 1 reply; 18+ messages in thread
From: Tino Calancha @ 2016-10-06  9:43 UTC (permalink / raw)
  To: 24627; +Cc: tino-emacs


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





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

* bug#24627: 24.5; (thing-at-point 'list) may return a non-empty string without a list
  2016-10-06  9:43 bug#24627: 24.5; (thing-at-point 'list) may return a non-empty string without a list Tino Calancha
@ 2016-10-07  6:42 ` Andreas Röhler
  2016-10-11  3:42   ` Tino Calancha
  0 siblings, 1 reply; 18+ messages in thread
From: Andreas Röhler @ 2016-10-07  6:42 UTC (permalink / raw)
  To: 24627



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





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

* bug#24627: 24.5; (thing-at-point 'list) may return a non-empty string without a list
  2016-10-07  6:42 ` Andreas Röhler
@ 2016-10-11  3:42   ` Tino Calancha
  2016-10-11 15:37     ` Andreas Röhler
  0 siblings, 1 reply; 18+ messages in thread
From: Tino Calancha @ 2016-10-11  3:42 UTC (permalink / raw)
  To: Andreas Röhler; +Cc: 24627

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





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

* bug#24627: 24.5; (thing-at-point 'list) may return a non-empty string without a list
  2016-10-11  3:42   ` Tino Calancha
@ 2016-10-11 15:37     ` Andreas Röhler
  2016-10-11 16:29       ` Tino Calancha
  0 siblings, 1 reply; 18+ messages in thread
From: Andreas Röhler @ 2016-10-11 15:37 UTC (permalink / raw)
  To: Tino Calancha; +Cc: 24627



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








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

* bug#24627: 24.5; (thing-at-point 'list) may return a non-empty string without a list
  2016-10-11 15:37     ` Andreas Röhler
@ 2016-10-11 16:29       ` Tino Calancha
  2016-10-11 16:47         ` Noam Postavsky
                           ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Tino Calancha @ 2016-10-11 16:29 UTC (permalink / raw)
  To: Andreas Röhler; +Cc: 24627, 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






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

* bug#24627: 24.5; (thing-at-point 'list) may return a non-empty string without a list
  2016-10-11 16:29       ` Tino Calancha
@ 2016-10-11 16:47         ` Noam Postavsky
  2016-10-11 17:09           ` Tino Calancha
  2016-10-11 17:16         ` bug#24627: "internal" designation [was: bug#24627: 24.5; (thing-at-point 'list) ...] Drew Adams
  2016-10-11 18:40         ` bug#24627: 24.5; (thing-at-point 'list) may return a non-empty string without a list Andreas Röhler
  2 siblings, 1 reply; 18+ messages in thread
From: Noam Postavsky @ 2016-10-11 16:47 UTC (permalink / raw)
  To: Tino Calancha; +Cc: 24627

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'.\]"





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

* bug#24627: 24.5; (thing-at-point 'list) may return a non-empty string without a list
  2016-10-11 16:47         ` Noam Postavsky
@ 2016-10-11 17:09           ` Tino Calancha
  2016-10-11 17:15             ` Noam Postavsky
  0 siblings, 1 reply; 18+ messages in thread
From: Tino Calancha @ 2016-10-11 17:09 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: 24627, 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'.]"





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

* bug#24627: 24.5; (thing-at-point 'list) may return a non-empty string without a list
  2016-10-11 17:09           ` Tino Calancha
@ 2016-10-11 17:15             ` Noam Postavsky
  2016-10-11 17:21               ` Tino Calancha
  0 siblings, 1 reply; 18+ messages in thread
From: Noam Postavsky @ 2016-10-11 17:15 UTC (permalink / raw)
  To: Tino Calancha; +Cc: 24627

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?





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

* bug#24627: "internal" designation   [was: bug#24627: 24.5; (thing-at-point 'list) ...]
  2016-10-11 16:29       ` Tino Calancha
  2016-10-11 16:47         ` Noam Postavsky
@ 2016-10-11 17:16         ` Drew Adams
  2016-10-11 17:21           ` Drew Adams
  2016-10-11 18:40         ` bug#24627: 24.5; (thing-at-point 'list) may return a non-empty string without a list Andreas Röhler
  2 siblings, 1 reply; 18+ messages in thread
From: Drew Adams @ 2016-10-11 17:16 UTC (permalink / raw)
  To: Tino Calancha, Andreas Röhler; +Cc: 24627

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





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

* bug#24627: 24.5; (thing-at-point 'list) may return a non-empty string without a list
  2016-10-11 17:15             ` Noam Postavsky
@ 2016-10-11 17:21               ` Tino Calancha
  0 siblings, 0 replies; 18+ messages in thread
From: Tino Calancha @ 2016-10-11 17:21 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: 24627, 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.






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

* bug#24627: "internal" designation   [was: bug#24627: 24.5; (thing-at-point 'list) ...]
  2016-10-11 17:16         ` bug#24627: "internal" designation [was: bug#24627: 24.5; (thing-at-point 'list) ...] Drew Adams
@ 2016-10-11 17:21           ` Drew Adams
  0 siblings, 0 replies; 18+ messages in thread
From: Drew Adams @ 2016-10-11 17:21 UTC (permalink / raw)
  To: Tino Calancha, Andreas Röhler; +Cc: 24627

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





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

* bug#24627: 24.5; (thing-at-point 'list) may return a non-empty string without a list
  2016-10-11 16:29       ` Tino Calancha
  2016-10-11 16:47         ` Noam Postavsky
  2016-10-11 17:16         ` bug#24627: "internal" designation [was: bug#24627: 24.5; (thing-at-point 'list) ...] Drew Adams
@ 2016-10-11 18:40         ` Andreas Röhler
  2016-10-12  4:58           ` Tino Calancha
  2 siblings, 1 reply; 18+ messages in thread
From: Andreas Röhler @ 2016-10-11 18:40 UTC (permalink / raw)
  To: Tino Calancha; +Cc: 24627



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






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

* bug#24627: 24.5; (thing-at-point 'list) may return a non-empty string without a list
  2016-10-11 18:40         ` bug#24627: 24.5; (thing-at-point 'list) may return a non-empty string without a list Andreas Röhler
@ 2016-10-12  4:58           ` Tino Calancha
  2016-10-12  7:10             ` Andreas Röhler
  0 siblings, 1 reply; 18+ messages in thread
From: Tino Calancha @ 2016-10-12  4:58 UTC (permalink / raw)
  To: Andreas Röhler; +Cc: 24627

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





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

* bug#24627: 24.5; (thing-at-point 'list) may return a non-empty string without a list
  2016-10-12  4:58           ` Tino Calancha
@ 2016-10-12  7:10             ` Andreas Röhler
  2016-10-13  8:50               ` Tino Calancha
  0 siblings, 1 reply; 18+ messages in thread
From: Andreas Röhler @ 2016-10-12  7:10 UTC (permalink / raw)
  To: Tino Calancha; +Cc: 24627



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





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

* bug#24627: 24.5; (thing-at-point 'list) may return a non-empty string without a list
  2016-10-12  7:10             ` Andreas Röhler
@ 2016-10-13  8:50               ` Tino Calancha
  2016-10-13 17:50                 ` Andreas Röhler
  0 siblings, 1 reply; 18+ messages in thread
From: Tino Calancha @ 2016-10-13  8:50 UTC (permalink / raw)
  To: Andreas Röhler; +Cc: 24627, 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





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

* bug#24627: 24.5; (thing-at-point 'list) may return a non-empty string without a list
  2016-10-13  8:50               ` Tino Calancha
@ 2016-10-13 17:50                 ` Andreas Röhler
  2016-10-15  9:44                   ` Tino Calancha
  0 siblings, 1 reply; 18+ messages in thread
From: Andreas Röhler @ 2016-10-13 17:50 UTC (permalink / raw)
  To: Tino Calancha; +Cc: 24627



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
>   






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

* bug#24627: 24.5; (thing-at-point 'list) may return a non-empty string without a list
  2016-10-13 17:50                 ` Andreas Röhler
@ 2016-10-15  9:44                   ` Tino Calancha
  2016-10-15 10:26                     ` Andreas Röhler
  0 siblings, 1 reply; 18+ messages in thread
From: Tino Calancha @ 2016-10-15  9:44 UTC (permalink / raw)
  To: Andreas Röhler; +Cc: 24627, 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





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

* bug#24627: 24.5; (thing-at-point 'list) may return a non-empty string without a list
  2016-10-15  9:44                   ` Tino Calancha
@ 2016-10-15 10:26                     ` Andreas Röhler
  0 siblings, 0 replies; 18+ messages in thread
From: Andreas Röhler @ 2016-10-15 10:26 UTC (permalink / raw)
  To: Tino Calancha; +Cc: 24627



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.






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

end of thread, other threads:[~2016-10-15 10:26 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-10-06  9:43 bug#24627: 24.5; (thing-at-point 'list) may return a non-empty string without a list Tino Calancha
2016-10-07  6:42 ` Andreas Röhler
2016-10-11  3:42   ` Tino Calancha
2016-10-11 15:37     ` Andreas Röhler
2016-10-11 16:29       ` Tino Calancha
2016-10-11 16:47         ` Noam Postavsky
2016-10-11 17:09           ` Tino Calancha
2016-10-11 17:15             ` Noam Postavsky
2016-10-11 17:21               ` Tino Calancha
2016-10-11 17:16         ` bug#24627: "internal" designation [was: bug#24627: 24.5; (thing-at-point 'list) ...] Drew Adams
2016-10-11 17:21           ` Drew Adams
2016-10-11 18:40         ` bug#24627: 24.5; (thing-at-point 'list) may return a non-empty string without a list Andreas Röhler
2016-10-12  4:58           ` Tino Calancha
2016-10-12  7:10             ` Andreas Röhler
2016-10-13  8:50               ` Tino Calancha
2016-10-13 17:50                 ` Andreas Röhler
2016-10-15  9:44                   ` Tino Calancha
2016-10-15 10:26                     ` Andreas Röhler

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