bug#31772: 26.1; (thing-at-point 'list) regression

bug#31772: 26.1; (thing-at-point 'list) regression

[Leo Liu]

Due to change

commit 76e297c15f6312a83599aab216be0396e9aac5c5
Author: Tino Calancha
Date:   Thu Nov 3 20:33:19 2016 +0900

(thing-at-point 'list) no longer works in comments; always return nil.

bug#31772: 26.1; (thing-at-point 'list) regression

[Eli Zaretskii]

> From: Leo Liu <sdl.web@gmail.com>
> Date: Sun, 10 Jun 2018 11:58:39 +0800
> Cc: tino.calancha@gmail.com
> 
> Due to change
> 
> commit 76e297c15f6312a83599aab216be0396e9aac5c5
> Author: Tino Calancha
> Date:   Thu Nov 3 20:33:19 2016 +0900
> 
> (thing-at-point 'list) no longer works in comments; always return nil.

Should it?  AFACS, in Emacs 25 the above returned nonsensical values
when invoked inside Lisp comments.

bug#31772: 26.1; (thing-at-point 'list) regression

[Tino Calancha]

Eli Zaretskii <eliz@gnu.org> writes:

>> Due to change
>> 
>> commit 76e297c15f6312a83599aab216be0396e9aac5c5
>> Author: Tino Calancha
>> Date:   Thu Nov 3 20:33:19 2016 +0900
>> 
>> (thing-at-point 'list) no longer works in comments; always return nil.
>
> Should it?  AFACS, in Emacs 25 the above returned nonsensical values
> when invoked inside Lisp comments.

[CC the relevant thingatpt experts]

Initially I thought that; after discuss with Emacs colleages
I changed my mine.

If you are in a buffer in emacs-lisp-mode, I just see
one list of length 3 in the following line:
("foo" "(bar)" "baz")

"(bar)" is a string.

If we comment the line, then there is no list at all; just a comment.
That is tested in thingatpt-tests.el (thing-at-point-bug24627).

You could change to fundamental-mode, then you would get again
the full list: because the line is not commented anymore.

To me this behaviour has sense, and it's predictible.  Maybe others
disagree.

PD: I must admit I am a bit worry everytime we start discussing
about this lib... :-S

bug#31772: 26.1; (thing-at-point 'list) regression

[Leo Liu]

On 2018-06-10 18:03 +0300, Eli Zaretskii wrote:
> Should it?  AFACS, in Emacs 25 the above returned nonsensical values
> when invoked inside Lisp comments.

I just checked it and seem to behave beautifully and usefully. What do
you see that are nonsensical?

Leo

bug#31772: 26.1; (thing-at-point 'list) regression

[Leo Liu]

On 2018-06-11 00:21 +0900, Tino Calancha wrote:
> Initially I thought that; after discuss with Emacs colleages
> I changed my mine.
>
> If you are in a buffer in emacs-lisp-mode, I just see
> one list of length 3 in the following line:
> ("foo" "(bar)" "baz")
>
> "(bar)" is a string.

I think you are solving the problem the wrong way. The original
behaviour of (thing-at-point 'list) relied on up-list and is intuitive
and sensible. After all who doesn't use up-list/down-list/kill-sexp etc.
(thing-at-point 'list) is well-aligned with them simply by letting
up-list do it.

But the point is a decade-old function that is functional and used in
people's init or packages are changed in drastically incompatible way. I
noticed the issue only after upgrade to emacs 26.1 a few days ago and
discovered https://github.com/leoliu/easy-kill/issues/28.

*SIDE NOTE*

One of the issues with Emacs development (I feel it more deeply after I
changed to use stable releases), people (in most cases only 1 user) find
some feature doesn't entirely suit their needs and they change it often
without full-perspective how it is used. For emacs 26.1 I have at least
three irritating annoyances (not counting crashes which are often beyond
my reach) that I need to code away one way or another.

Leo

bug#31772: 26.1; (thing-at-point 'list) regression

[Eli Zaretskii]

> From:  Leo Liu <sdl.web@gmail.com>
> Cc: 31772@debbugs.gnu.org,  tino.calancha@gmail.com
> Date: Mon, 11 Jun 2018 10:32:14 +0800
> 
> I just checked it and seem to behave beautifully and usefully. What do
> you see that are nonsensical?

In "emacs -Q", go to the 'h' in "that" on the first line, and then:

  M-: (thing-at-point 'list) RET
   => #("hat" 0 3 (fontified nil face font-lock-comment-face))

That's not even a word, let alone a list.  Is that useful?  I think
nil is more useful, since nothing should be a list inside a comment.

bug#31772: 26.1; (thing-at-point 'list) regression

[Tino Calancha]

On Mon, 11 Jun 2018, Leo Liu wrote:

> *SIDE NOTE*
>
> One of the issues with Emacs development (I feel it more deeply after I
> changed to use stable releases), people (in most cases only 1 user) find
> some feature doesn't entirely suit their needs and they change it often
> without full-perspective how it is used. For emacs 26.1 I have at least
> three irritating annoyances (not counting crashes which are often beyond
> my reach) that I need to code away one way or another.
Thank you Leo,

Since you seem a skilled Emacs user, you are very welcome to join
us, all-volunteer Emacs developers, to keep improving Emacs and delight us 
with a 'full-perspective how to use it'.  Honestly, I am really looking 
forward to you join us.  I wish all the Emacs users happy.
That would be awesome.

Feel free also to propose any patch on this issue and discuss it here.

bug#31772: 26.1; (thing-at-point 'list) regression

[Eli Zaretskii]

> From:  Leo Liu <sdl.web@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  31772@debbugs.gnu.org,  Noam Postavsky <npostavs@users.sourceforge.net>,  Dmitry Gutov <dgutov@yandex.ru>
> Date: Mon, 11 Jun 2018 11:17:08 +0800
> 
> > If you are in a buffer in emacs-lisp-mode, I just see
> > one list of length 3 in the following line:
> > ("foo" "(bar)" "baz")
> >
> > "(bar)" is a string.
> 
> I think you are solving the problem the wrong way. The original
> behaviour of (thing-at-point 'list) relied on up-list and is intuitive
> and sensible.

But up-list signals an error in the comment in *scratch*, for example,
so I'm not sure what consistency are we talking about here.

> But the point is a decade-old function that is functional and used in
> people's init or packages are changed in drastically incompatible way. I
> noticed the issue only after upgrade to emacs 26.1 a few days ago and
> discovered https://github.com/leoliu/easy-kill/issues/28.
> 
> *SIDE NOTE*
> 
> One of the issues with Emacs development (I feel it more deeply after I
> changed to use stable releases), people (in most cases only 1 user) find
> some feature doesn't entirely suit their needs and they change it often
> without full-perspective how it is used.

That's unfair, to say the least.  During the entire development of
Emacs 26, there was a conscious effort to avoid backward-incompatible
changes as much as possible, and in many cases leave an option behind
that would get previous behavior, sometimes people felt it was too
zealous.  Yes, several incompatible changes might have fallen through
the cracks, but if you think it's too much, please consider being more
active on the development list, and make sure your voice is heard when
these decisions are made.

bug#31772: 26.1; (thing-at-point 'list) regression

[Leo Liu]

On 2018-06-12 00:06 +0900, Tino Calancha wrote:
> Since you seem a skilled Emacs user, you are very welcome to join us,
> all-volunteer Emacs developers, to keep improving Emacs and delight us
> with a 'full-perspective how to use it'. 

No one has a full-perspective. We all work with our partial perspective.
That's not an issue. I am simply pointing out the irony it only takes
one complaint to change something that's in Emacs for a long time but we
often forget the majority that are happy with the feature never come and
tell us from time to time what a great feature it is, they love it and
don't change it.

For example, a lot of lisp programmers use paredit (not part of emacs).
Two or three major releases ago, the syntax of @ in (emacs-)lisp mode
was changed so that some unusual function names containing ,@ could
fontify properly, in doing so typing `,@(' in paredit gives you `,@ ()'
instead of `,@()'.

> Honestly, I am really looking forward to you join us. I wish all the
> Emacs users happy. That would be awesome.

I already joined in but I am a little short of time lately.

Leo

bug#31772: 26.1; (thing-at-point 'list) regression

[Tino Calancha]

On Tue, 12 Jun 2018, Leo Liu wrote:

> On 2018-06-12 00:06 +0900, Tino Calancha wrote:
>> Since you seem a skilled Emacs user, you are very welcome to join us,
>> all-volunteer Emacs developers, to keep improving Emacs and delight us
>> with a 'full-perspective how to use it'.
>
> No one has a full-perspective. We all work with our partial perspective.
> That's not an issue. I am simply pointing out the irony it only takes
> one complaint to change something that's in Emacs for a long time but we
> often forget the majority that are happy with the feature never come and
> tell us from time to time what a great feature it is, they love it and
> don't change it.
>
> For example, a lot of lisp programmers use paredit (not part of emacs).
> Two or three major releases ago, the syntax of @ in (emacs-)lisp mode
> was changed so that some unusual function names containing ,@ could
> fontify properly, in doing so typing `,@(' in paredit gives you `,@ ()'
> instead of `,@()'.
You are not alone.  This also happen to me.

For example, Dmitry makes obsolote (to be 
more precise rebind Q to a new command) `dired-do-query-replace-regexp'.
I like this command very much.  I still use it and don't understand why
it shouln't been bind by default to the classical Q.  But it's also
true that I wasn't collaborating with Emacs at the time that change was
made.  This people spent their free time to make Emacs great for free.
I really thanks them so much, that I became one of them.

I cannot blame Dmitry, just thanks him for his great contributions
to the project and time invest on it.  I think he is awesome, as all
Emacs guys.

>> Honestly, I am really looking forward to you join us. I wish all the
>> Emacs users happy. That would be awesome.
>
> I already joined in but I am a little short of time lately.
Oh so sad to heard that.  I understand the work is the more important.

That been said, if you can find some time in the future in your weekends,
or just read the Emacs devel list, or the bug list and speak up when the 
people are asking for opinions, I would be very happy to listen your 
feedback.  At the end we all want to make Emacs better and better.
That's the only goal.  We all love Emacs, right?

bug#31772: 26.1; (thing-at-point 'list) regression

[Eli Zaretskii]

> From: Leo Liu <sdl.web@gmail.com>
> Date: Tue, 12 Jun 2018 00:08:27 +0800
> Cc: 31772@debbugs.gnu.org
> 
> I am simply pointing out the irony it only takes one complaint to
> change something that's in Emacs for a long time but we often forget
> the majority that are happy with the feature never come and tell us
> from time to time what a great feature it is, they love it and don't
> change it.

No, we never forget that.  And one complaint is usually not enough to
make such changes.  on the contrary, sometimes the disputes about such
complaints are too long and exhaustive.  You can see many examples of
that in the archives and on the bug tracker.

But yes, the number of people here is relatively small, so each voice
counts.  Please be sure to voice yours, I'm certain it will have a
significant effect on the outcome.

> I already joined in but I am a little short of time lately.

We all are.  Thanks in advance for your contributions, both in
opinions and in code.

bug#31772: 26.1; (thing-at-point 'list) regression

[Leo Liu]

On 2018-06-11 18:10 +0300, Eli Zaretskii wrote:
> But up-list signals an error in the comment in *scratch*, for example,
> so I'm not sure what consistency are we talking about here.

If you have a line of lisp code that is commented out, up-list/down-list
behave just as well. If there is no list it signals an error. I think it
has been like that for ages.

> That's unfair, to say the least.  During the entire development of
> Emacs 26, there was a conscious effort to avoid backward-incompatible
> changes as much as possible, and in many cases leave an option behind
> that would get previous behavior, sometimes people felt it was too
> zealous.  Yes, several incompatible changes might have fallen through
> the cracks, but if you think it's too much, please consider being more
> active on the development list, and make sure your voice is heard when
> these decisions are made.

Sorry. After sending the email I also felt it might be a bit negative. I
have sent another email just now to elaborate on what I meant.

Leo

bug#31772: 26.1; (thing-at-point 'list) regression

[Leo Liu]

On 2018-06-11 17:58 +0300, Eli Zaretskii wrote:
> In "emacs -Q", go to the 'h' in "that" on the first line, and then:
>
>   M-: (thing-at-point 'list) RET
>    => #("hat" 0 3 (fontified nil face font-lock-comment-face))
>
> That's not even a word, let alone a list.  Is that useful?  I think
> nil is more useful, since nothing should be a list inside a comment.

I have noticed this while coding easy-kill since it built on
thingatpt.el. It was of minor annoyance so I ignored it.

It seems to make a lot of sense to fallback on `sexp' as a degenerated
case. Do you agree the previous behaviour of (thing-at-point 'list) is
mostly sane?

Additional points.

show-paren-mode works inside comments and strings. One of the craziest
things it does is if you have open paren in one string and a close paren
in another, it works too.

characters in and out of comments usually have the same syntax. so
finding a list in comments or strings is not wrong. It's smart and it's
useful. It serves us very well.

Leo

bug#31772: 26.1; (thing-at-point 'list) regression

[Leo Liu]

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

Hi there,

I have been using 26.1 as my main editor for the last few months and
this breakage remains a pain point in my day-to-day editing. For example
whenever I rewrite a function, I normally comment out the old one (to
keep the linter, pretty-printer or whatnot happy) and write the new one
from scratch, occasionally copy things from the old one to save typing
and this bug gets in the way many times a day. I propose a patch that
doesn't divert too much from the old and tried behaviour.

The idea that is currently in thing-at-point-bounds-of-list-at-point is
fine for a higher level function such as list-at-point but doing it
there affects all functions that build on it including some from
thingatpt.el itself.

I hope you can find time to review the patch and come to a solution for
26.2 which I very much look forward to.

Thanks,
Leo


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: thing.diff --]
[-- Type: text/x-patch, Size: 2739 bytes --]

diff --git a/lisp/thingatpt.el b/lisp/thingatpt.el
index 6a978fe9..8da31a03 100644
--- a/lisp/thingatpt.el
+++ b/lisp/thingatpt.el
@@ -221,15 +221,12 @@ The bounds of THING are determined by `bounds-of-thing-at-point'."
   "Return the bounds of the list at point.
 \[Internal function used by `bounds-of-thing-at-point'.]"
   (save-excursion
-    (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))))))
+    (if (ignore-errors (up-list -1))
+	(ignore-errors (cons (point) (progn (forward-sexp) (point))))
+      (let ((bound (bounds-of-thing-at-point 'sexp)))
+	(and bound
+	     (<= (car bound) (point)) (< (point) (cdr bound))
+	     bound)))))
 
 ;; Defuns
 
diff --git a/test/lisp/thingatpt-tests.el b/test/lisp/thingatpt-tests.el
index cfb57de6..6093c209 100644
--- a/test/lisp/thingatpt-tests.el
+++ b/test/lisp/thingatpt-tests.el
@@ -84,20 +84,18 @@ position to retrieve THING.")
       (goto-char (nth 1 test))
       (should (equal (thing-at-point (nth 2 test)) (nth 3 test))))))
 
-;; These tests reflect the actual behavior of
-;; `thing-at-point-bounds-of-list-at-point'.
-(ert-deftest thing-at-point-bug24627 ()
-  "Test for https://debbugs.gnu.org/24627 ."
+;; See bug#24627 and bug#31772.
+(ert-deftest thing-at-point-bounds-of-list-at-point ()
   (let ((string-result '(("(a \"b\" c)" . (a "b" c))
-                         (";(a \"b\" c)")
+                         (";(a \"b\" c)" . (a "b" c))
                          ("(a \"b\" c\n)" . (a "b" c))
-                         ("\"(a b c)\"")
+                         ("\"(a b c)\"" . (a b c))
                          ("(a ;(b c d)\ne)" . (a e))
-                         ("(foo\n(a ;(b c d)\ne) bar)" . (a e))
+                         ("(foo\n(a ;(b c d)\ne) bar)" . (foo (a e) bar))
                          ("(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)"))
+                         ("(foo\n(a \"(b c d)\"\ne) bar)" . (foo (a "(b c d)" e) bar))
+                         ("(b\n(a ;(foo c d)\ne) bar)" . (b (a e) bar))
+                         ("(princ \"(a b c)\")" . (a b c))
                          ("(defun foo ()\n  \"Test function.\"\n  ;;(a b)\n  nil)" . (defun foo nil "Test function." nil))))
         (file
          (expand-file-name "lisp/thingatpt.el" source-directory))

bug#31772: 26.1; (thing-at-point 'list) regression

[Andreas Röhler]

On 06.09.2018 12:37, Leo Liu wrote:
> Hi there,
> 
> I have been using 26.1 as my main editor for the last few months and
> this breakage remains a pain point in my day-to-day editing. For example
> whenever I rewrite a function, I normally comment out the old one (to
> keep the linter, pretty-printer or whatnot happy) and write the new one
> from scratch, occasionally copy things from the old one to save typing
> and this bug gets in the way many times a day. I propose a patch that
> doesn't divert too much from the old and tried behaviour.
> 
> The idea that is currently in thing-at-point-bounds-of-list-at-point is
> fine for a higher level function such as list-at-point but doing it
> there affects all functions that build on it including some from
> thingatpt.el itself.
> 
> I hope you can find time to review the patch and come to a solution for
> 26.2 which I very much look forward to.
> 
> Thanks,
> Leo
> 

Hi Leo,

lets consider the following proposed change of tests:

-                         ("(foo\n(a ;(b c d)\ne) bar)" . (a e))
+                         ("(foo\n(a ;(b c d)\ne) bar)" . (foo (a e) bar))

As the ert-test mentioned  calls (re-search-backward "\\((a\\|^a\\)")

point will be behind foo at "(a". I.e. "foo" belongs to outer list, not 
to list-at-point. The desired result shown by this change looks wrong, 
"(foo" should not be part of.

Maybe I'm missing something.
May you provide a standalone example where current behavior breaks your 
code?

Thanks,
Andreas
gladly using GNU Emacs 27.0.50 (build 1, i686-pc-linux-gnu, GTK+ Version 
3.14.5) of 2018-08-29

bug#31772: 26.1; (thing-at-point 'list) regression

[Leo Liu]

On 2018-09-06 21:01 +0200, Andreas Röhler wrote:
> Hi Leo,
>
> lets consider the following proposed change of tests:
>
> -                         ("(foo\n(a ;(b c d)\ne) bar)" . (a e))
> +                         ("(foo\n(a ;(b c d)\ne) bar)" . (foo (a e) bar))
>
> As the ert-test mentioned  calls (re-search-backward "\\((a\\|^a\\)")
>
> point will be behind foo at "(a". I.e. "foo" belongs to outer list,
> not to list-at-point. The desired result shown by this change looks
> wrong, "(foo" should not be part of.
>
> Maybe I'm missing something.
> May you provide a standalone example where current behavior breaks
> your code?

I carefully considered this case when preparing the patch.

In the last 10 years (thing-at-point 'list) always returns the enclosing
list so I opted to keep this for now. It is confusing and I don't like
it personally.

It can be tweaked to look at "list" at point first if that's what
everybody wants.

Cheers,
Leo

bug#31772: 26.1; (thing-at-point 'list) regression

[Andreas Röhler]

On 07.09.2018 06:42, Leo Liu wrote:
> On 2018-09-06 21:01 +0200, Andreas Röhler wrote:
>> Hi Leo,
>>
>> lets consider the following proposed change of tests:
>>
>> -                         ("(foo\n(a ;(b c d)\ne) bar)" . (a e))
>> +                         ("(foo\n(a ;(b c d)\ne) bar)" . (foo (a e) bar))
>>
>> As the ert-test mentioned  calls (re-search-backward "\\((a\\|^a\\)")
>>
>> point will be behind foo at "(a". I.e. "foo" belongs to outer list,
>> not to list-at-point. The desired result shown by this change looks
>> wrong, "(foo" should not be part of.
>>
>> Maybe I'm missing something.
>> May you provide a standalone example where current behavior breaks
>> your code?
> 
> I carefully considered this case when preparing the patch.
> 
> In the last 10 years (thing-at-point 'list) always returns the enclosing
> list so I opted to keep this for now. It is confusing and I don't like
> it personally.


To check the current correct behavior insert the following lines and try 
the command provided below:

(foo\n(a ;(b c d)
e) bar)

(defun mylist ()
   (interactive)
   (save-excursion
     (goto-char (point-min))
     (search-forward "(a")
     (forward-char -1)
     (thing-at-point 'list t)))


 From "(" it reads the list opened at point.

bug#31772: 26.1; (thing-at-point 'list) regression

[Leo Liu]

On 2018-09-07 10:17 +0200, Andreas Röhler wrote:
> To check the current correct behavior insert the following lines

With all due respect that is not the issue at stake. The issue with the
code in 26.1 is changing the semantics abruptly without consideration
for backwards compatibility. It is a change that is broadly not in line
with everything else in Emacs.

Leo

bug#31772: 26.1; (thing-at-point 'list) regression

[Eli Zaretskii]

> From:  Leo Liu <sdl.web@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, Tino Calancha <tino.calancha@gmail.com>
> Date: Thu, 06 Sep 2018 18:37:11 +0800
> 
> I have been using 26.1 as my main editor for the last few months and
> this breakage remains a pain point in my day-to-day editing. For example
> whenever I rewrite a function, I normally comment out the old one (to
> keep the linter, pretty-printer or whatnot happy) and write the new one
> from scratch, occasionally copy things from the old one to save typing
> and this bug gets in the way many times a day. I propose a patch that
> doesn't divert too much from the old and tried behaviour.

Thanks.  Can you summarize how the behavior with your patch will be
different from what we had in Emacs 25 and before?

> The idea that is currently in thing-at-point-bounds-of-list-at-point is
> fine for a higher level function such as list-at-point but doing it
> there affects all functions that build on it including some from
> thingatpt.el itself.

Would it be possible to modify list-at-point so that it keeps the
current behavior, perhaps as an option?  I'd like to find a solution
that doesn't just revert to the old behavior, but allows those who
need the new behavior to have it in some reasonable way.

Thanks.

bug#31772: 26.1; (thing-at-point 'list) regression

[Leo Liu]

On 2018-09-11 11:31 +0300, Eli Zaretskii wrote:
[snipped 13 lines]
> Thanks.  Can you summarize how the behavior with your patch will be
> different from what we had in Emacs 25 and before?

The difference is on what to return when (up-list -1) fails. They both
try to return the sexp at point but the patched behaviour try returning
the whole sexp while the one in <= 25.1 cut off the sexp from point.

Looking at the code in 25.1 it looks to me the original author had a
thinko i.e. he/she meant to write:

(if (>= opoint (point))
    (cons (point) end))

but instead write

(if (>= opoint (point))
    (cons opoint end))

which resulted in some weird cases that you mentioned in previous email.

[snipped 6 lines]
> Would it be possible to modify list-at-point so that it keeps the
> current behavior, perhaps as an option?  I'd like to find a solution
> that doesn't just revert to the old behavior, but allows those who
> need the new behavior to have it in some reasonable way.

Yes, this can be done. For example:

(defun list-at-point (&optional ignore-comment-or-string)
  "Return the Lisp list at point, or nil if none is found.
If IGNORE-COMMENT-OR-STRING is non-nil comments and strings are
treated as white space."
  (let ((ppss (and ignore-comment-or-string (syntax-ppss))))
    (save-excursion
      (goto-char (or (nth 8 ppss) (point)))
      (form-at-point 'list 'listp))))

>
> Thanks.

Thanks. -Leo

bug#31772: 26.1; (thing-at-point 'list) regression

[Eli Zaretskii]

> From:  Leo Liu <sdl.web@gmail.com>
> Cc: 31772@debbugs.gnu.org,  tino.calancha@gmail.com
> Date: Tue, 11 Sep 2018 18:26:36 +0800
> 
> The difference is on what to return when (up-list -1) fails. They both
> try to return the sexp at point but the patched behaviour try returning
> the whole sexp while the one in <= 25.1 cut off the sexp from point.
> 
> Looking at the code in 25.1 it looks to me the original author had a
> thinko i.e. he/she meant to write:
> 
> (if (>= opoint (point))
>     (cons (point) end))
> 
> but instead write
> 
> (if (>= opoint (point))
>     (cons opoint end))
> 
> which resulted in some weird cases that you mentioned in previous email.

Right.

> > Would it be possible to modify list-at-point so that it keeps the
> > current behavior, perhaps as an option?  I'd like to find a solution
> > that doesn't just revert to the old behavior, but allows those who
> > need the new behavior to have it in some reasonable way.
> 
> Yes, this can be done. For example:
> 
> (defun list-at-point (&optional ignore-comment-or-string)
>   "Return the Lisp list at point, or nil if none is found.
> If IGNORE-COMMENT-OR-STRING is non-nil comments and strings are
> treated as white space."
>   (let ((ppss (and ignore-comment-or-string (syntax-ppss))))
>     (save-excursion
>       (goto-char (or (nth 8 ppss) (point)))
>       (form-at-point 'list 'listp))))

Would you mind submitting a patch that includes the above, and also
fixes/augments the tests accordingly?  I think these changes then
could go into the emacs-26 branch, unless someone raises objections.

Thanks.

bug#31772: 26.1; (thing-at-point 'list) regression

[Andreas Röhler]

On 11.09.2018 13:16, Eli Zaretskii wrote:
>> From:  Leo Liu <sdl.web@gmail.com>
>> Cc: 31772@debbugs.gnu.org,  tino.calancha@gmail.com
>> Date: Tue, 11 Sep 2018 18:26:36 +0800
>>
>> The difference is on what to return when (up-list -1) fails. They both
>> try to return the sexp at point but the patched behaviour try returning
>> the whole sexp while the one in <= 25.1 cut off the sexp from point.
>>
>> Looking at the code in 25.1 it looks to me the original author had a
>> thinko i.e. he/she meant to write:
>>
>> (if (>= opoint (point))
>>      (cons (point) end))
>>
>> but instead write
>>
>> (if (>= opoint (point))
>>      (cons opoint end))
>>
>> which resulted in some weird cases that you mentioned in previous email.
> 
> Right.
> 
>>> Would it be possible to modify list-at-point so that it keeps the
>>> current behavior, perhaps as an option?  I'd like to find a solution
>>> that doesn't just revert to the old behavior, but allows those who
>>> need the new behavior to have it in some reasonable way.
>>
>> Yes, this can be done. For example:
>>optional ignore-comment-or-string)
>>    "Return the Lisp list at
>> (defun list-at-point (& point, or nil if none is found.
>> If IGNORE-COMMENT-OR-STRING is non-nil comments and strings are
>> treated as white space."
>>    (let ((ppss (and ignore-comment-or-string (syntax-ppss))))
>>      (save-excursion
>>        (goto-char (or (nth 8 ppss) (point)))
>>        (form-at-point 'list 'listp))))
> 
> Would you mind submitting a patch that includes the above, and also
> fixes/augments the tests accordingly?  I think these changes then
> could go into the emacs-26 branch, unless someone raises objections.
> 
> Thanks.
> 
> 
> 



The basic decision to make is to put some section as comment or not.

If code is commented out, it looses its lisp-syntax. It's plain text.
Therefor no lisp-list can exist and no need for a var 
ignore-comment-or-string.

bug#31772: 26.1; (thing-at-point 'list) regression

[Eli Zaretskii]

> From: Andreas Röhler <andreas.roehler@easy-emacs.de>
> Date: Tue, 11 Sep 2018 13:52:42 +0200
> 
> If code is commented out, it looses its lisp-syntax. It's plain text.
> Therefor no lisp-list can exist and no need for a var 
> ignore-comment-or-string.

I think I agree with Leo on this and disagree with you.

bug#31772: 26.1; (thing-at-point 'list) regression

[Leo Liu]

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

On 2018-09-11 14:16 +0300, Eli Zaretskii wrote:
> Would you mind submitting a patch that includes the above, and also
> fixes/augments the tests accordingly?  I think these changes then
> could go into the emacs-26 branch, unless someone raises objections.

Please find the new patch in the attachment. Thanks a lot for helping
straighten this out.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: thing.diff --]
[-- Type: text/x-patch, Size: 5310 bytes --]

diff --git a/lisp/thingatpt.el b/lisp/thingatpt.el
index 6a978fe9..fdc12f32 100644
--- a/lisp/thingatpt.el
+++ b/lisp/thingatpt.el
@@ -221,15 +221,12 @@ The bounds of THING are determined by `bounds-of-thing-at-point'."
   "Return the bounds of the list at point.
 \[Internal function used by `bounds-of-thing-at-point'.]"
   (save-excursion
-    (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))))))
+    (if (ignore-errors (up-list -1))
+	(ignore-errors (cons (point) (progn (forward-sexp) (point))))
+      (let ((bound (bounds-of-thing-at-point 'sexp)))
+	(and bound
+	     (<= (car bound) (point)) (< (point) (cdr bound))
+	     bound)))))
 
 ;; Defuns
 
@@ -608,8 +605,13 @@ Signal an error if the entire string was not used."
 
 (put 'number 'thing-at-point 'number-at-point)
 ;;;###autoload
-(defun list-at-point ()
-  "Return the Lisp list at point, or nil if none is found."
-  (form-at-point 'list 'listp))
+(defun list-at-point (&optional ignore-comment-or-string)
+  "Return the Lisp list at point, or nil if none is found.
+If IGNORE-COMMENT-OR-STRING is non-nil comments and strings are
+treated as white space."
+  (let ((ppss (and ignore-comment-or-string (syntax-ppss))))
+    (save-excursion
+      (goto-char (or (nth 8 ppss) (point)))
+      (form-at-point 'list 'listp))))
 
 ;;; thingatpt.el ends here
diff --git a/test/lisp/thingatpt-tests.el b/test/lisp/thingatpt-tests.el
index cfb57de6..43172f4f 100644
--- a/test/lisp/thingatpt-tests.el
+++ b/test/lisp/thingatpt-tests.el
@@ -84,41 +84,44 @@ position to retrieve THING.")
       (goto-char (nth 1 test))
       (should (equal (thing-at-point (nth 2 test)) (nth 3 test))))))
 
-;; These tests reflect the actual behavior of
-;; `thing-at-point-bounds-of-list-at-point'.
-(ert-deftest thing-at-point-bug24627 ()
-  "Test for https://debbugs.gnu.org/24627 ."
-  (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))))
-        (file
-         (expand-file-name "lisp/thingatpt.el" source-directory))
-        buf)
-    ;; Test for `thing-at-point'.
-    (when (file-exists-p file)
-      (unwind-protect
-          (progn
-            (setq buf (find-file file))
-            (goto-char (point-max))
-            (forward-line -1)
-            (should-not (thing-at-point 'list)))
-        (kill-buffer buf)))
-    ;; Tests for `list-at-point'.
-    (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)))))))
+;; See bug#24627 and bug#31772.
+(ert-deftest thing-at-point-bounds-of-list-at-point ()
+  (cl-macrolet ((with-test-buffer (str &rest body)
+                  `(with-temp-buffer
+                     (emacs-lisp-mode)
+                     (insert ,str)
+                     (search-backward "|")
+                     (delete-char 1)
+                     ,@body)))
+    (let ((tests1
+           '(("|(a \"b\" c)" (a "b" c))
+             (";|(a \"b\" c)" (a "b" c) nil)
+             ("|(a \"b\" c\n)" (a "b" c))
+             ("\"|(a b c)\"" (a b c) nil)
+             ("|(a ;(b c d)\ne)" (a e))
+             ("(foo\n|(a ;(b c d)\ne) bar)" (foo (a e) bar))
+             ("(foo\n|a ;(b c d)\ne bar)" (foo a e bar))
+             ("(foo\n|(a \"(b c d)\"\ne) bar)" (foo (a "(b c d)" e) bar))
+             ("(b\n|(a ;(foo c d)\ne) bar)" (b (a e) bar))
+             ("(princ \"|(a b c)\")" (a b c) (princ "(a b c)"))
+             ("(defun foo ()\n  \"Test function.\"\n  ;;|(a b)\n  nil)"
+              (defun foo nil "Test function." nil)
+              (defun foo nil "Test function." nil))))
+          (tests2
+           '(("|list-at-point" . "list-at-point")
+             ("list-|at-point" . "list-at-point")
+             ("list-at-point|" . nil)
+             ("|(a b c)" . "(a b c)")
+             ("(a b c)|" . nil))))
+      ;; Tests for `list-at-point'.
+      (dolist (test tests1)
+        (with-test-buffer (car test)
+          (should (equal (list-at-point) (cl-second test)))
+          (when (cddr test)
+            (should (equal (list-at-point t) (cl-third test))))))
+      (dolist (test tests2)
+        (with-test-buffer (car test)
+          (should (equal (thing-at-point 'list) (cdr test))))))))
 
 (ert-deftest thing-at-point-url-in-comment ()
   (with-temp-buffer

bug#31772: 26.1; (thing-at-point 'list) regression

[Eli Zaretskii]

> From:  Leo Liu <sdl.web@gmail.com>
> Cc: 31772@debbugs.gnu.org,  tino.calancha@gmail.com
> Date: Tue, 11 Sep 2018 20:36:53 +0800
> 
> On 2018-09-11 14:16 +0300, Eli Zaretskii wrote:
> > Would you mind submitting a patch that includes the above, and also
> > fixes/augments the tests accordingly?  I think these changes then
> > could go into the emacs-26 branch, unless someone raises objections.
> 
> Please find the new patch in the attachment. Thanks a lot for helping
> straighten this out.

Thanks, let's wait for a few days to let people comment, raise
objections, etc.

bug#31772: 26.1; (thing-at-point 'list) regression

[Leo Liu]

Fixed in 26.2.

On 2018-09-11 15:39 +0300, Eli Zaretskii wrote:
> Thanks, let's wait for a few days to let people comment, raise
> objections, etc.

I just pushed it to emacs-26. Thanks again.

Leo

bug#31772: 26.1; (thing-at-point 'list) regression

[Eli Zaretskii]

> From:  Leo Liu <sdl.web@gmail.com>
> Cc: 31772-done@debbugs.gnu.org,  tino.calancha@gmail.com
> Date: Fri, 14 Sep 2018 22:55:08 +0800
> 
> Fixed in 26.2.

Thanks, but that was too fast, especially for a somewhat controversial
change such as this one.  Please in the future allow at least one week
in such cases.

bug#31772: 26.1; (thing-at-point 'list) regression

[Leo Liu]

On 2018-09-15 12:06 +0300, Eli Zaretskii wrote:
> Thanks, but that was too fast, especially for a somewhat controversial
> change such as this one.  Please in the future allow at least one week
> in such cases.

Absolutely, Eli.

I was expecting a storm (said to be the worst of the year) and was also
travelling early next day (which was rescheduled to Monday). I was
expecting no internnet for at least a Month, so I thought I made the
commit considering there were also some local minor tweaks. Otherwise I
would have left it here knowing someone would eventually take are of it.

BTW, thanks for adding the detailed News entry.

Leo

bug#31772: 26.1; (thing-at-point 'list) regression

[Eli Zaretskii]

> From:  Leo Liu <sdl.web@gmail.com>
> Cc: 31772@debbugs.gnu.org,  tino.calancha@gmail.com
> Date: Sat, 15 Sep 2018 20:58:06 +0800
> 
> On 2018-09-15 12:06 +0300, Eli Zaretskii wrote:
> > Thanks, but that was too fast, especially for a somewhat controversial
> > change such as this one.  Please in the future allow at least one week
> > in such cases.
> 
> Absolutely, Eli.
> 
> I was expecting a storm (said to be the worst of the year) and was also
> travelling early next day (which was rescheduled to Monday). I was
> expecting no internnet for at least a Month, so I thought I made the
> commit considering there were also some local minor tweaks. Otherwise I
> would have left it here knowing someone would eventually take are of it.

Understood, and thanks for working on this.