* bug#31772: 26.1; (thing-at-point 'list) regression @ 2018-06-10 3:58 Leo Liu 2018-06-10 15:03 ` Eli Zaretskii 0 siblings, 1 reply; 29+ messages in thread From: Leo Liu @ 2018-06-10 3:58 UTC (permalink / raw) To: 31772; +Cc: tino.calancha 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#31772: 26.1; (thing-at-point 'list) regression 2018-06-10 3:58 bug#31772: 26.1; (thing-at-point 'list) regression Leo Liu @ 2018-06-10 15:03 ` Eli Zaretskii 2018-06-10 15:21 ` Tino Calancha 2018-06-11 2:32 ` Leo Liu 0 siblings, 2 replies; 29+ messages in thread From: Eli Zaretskii @ 2018-06-10 15:03 UTC (permalink / raw) To: Leo Liu; +Cc: 31772, tino.calancha > 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#31772: 26.1; (thing-at-point 'list) regression 2018-06-10 15:03 ` Eli Zaretskii @ 2018-06-10 15:21 ` Tino Calancha 2018-06-11 3:17 ` Leo Liu 2018-06-11 2:32 ` Leo Liu 1 sibling, 1 reply; 29+ messages in thread From: Tino Calancha @ 2018-06-10 15:21 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 31772, tino.calancha, Noam Postavsky, Dmitry Gutov, Leo Liu 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#31772: 26.1; (thing-at-point 'list) regression 2018-06-10 15:21 ` Tino Calancha @ 2018-06-11 3:17 ` Leo Liu 2018-06-11 15:06 ` Tino Calancha 2018-06-11 15:10 ` Eli Zaretskii 0 siblings, 2 replies; 29+ messages in thread From: Leo Liu @ 2018-06-11 3:17 UTC (permalink / raw) To: Tino Calancha; +Cc: Noam Postavsky, 31772, Dmitry Gutov 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#31772: 26.1; (thing-at-point 'list) regression 2018-06-11 3:17 ` Leo Liu @ 2018-06-11 15:06 ` Tino Calancha 2018-06-11 16:08 ` Leo Liu 2018-06-11 15:10 ` Eli Zaretskii 1 sibling, 1 reply; 29+ messages in thread From: Tino Calancha @ 2018-06-11 15:06 UTC (permalink / raw) To: Leo Liu; +Cc: 31772, Dmitry Gutov, Tino Calancha, Noam Postavsky 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#31772: 26.1; (thing-at-point 'list) regression 2018-06-11 15:06 ` Tino Calancha @ 2018-06-11 16:08 ` Leo Liu 2018-06-11 16:25 ` Tino Calancha 2018-06-11 16:34 ` Eli Zaretskii 0 siblings, 2 replies; 29+ messages in thread From: Leo Liu @ 2018-06-11 16:08 UTC (permalink / raw) To: Tino Calancha; +Cc: 31772 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#31772: 26.1; (thing-at-point 'list) regression 2018-06-11 16:08 ` Leo Liu @ 2018-06-11 16:25 ` Tino Calancha 2018-09-06 10:37 ` Leo Liu 2018-06-11 16:34 ` Eli Zaretskii 1 sibling, 1 reply; 29+ messages in thread From: Tino Calancha @ 2018-06-11 16:25 UTC (permalink / raw) To: Leo Liu; +Cc: 31772, Dmitry Gutov, Tino Calancha, Noam Postavsky 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? ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#31772: 26.1; (thing-at-point 'list) regression 2018-06-11 16:25 ` Tino Calancha @ 2018-09-06 10:37 ` Leo Liu 2018-09-06 19:01 ` Andreas Röhler 2018-09-11 8:31 ` Eli Zaretskii 0 siblings, 2 replies; 29+ messages in thread From: Leo Liu @ 2018-09-06 10:37 UTC (permalink / raw) To: 31772; +Cc: Tino Calancha [-- 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)) ^ permalink raw reply related [flat|nested] 29+ messages in thread
* bug#31772: 26.1; (thing-at-point 'list) regression 2018-09-06 10:37 ` Leo Liu @ 2018-09-06 19:01 ` Andreas Röhler 2018-09-07 4:42 ` Leo Liu 2018-09-11 8:31 ` Eli Zaretskii 1 sibling, 1 reply; 29+ messages in thread From: Andreas Röhler @ 2018-09-06 19:01 UTC (permalink / raw) To: Leo Liu; +Cc: 31772 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#31772: 26.1; (thing-at-point 'list) regression 2018-09-06 19:01 ` Andreas Röhler @ 2018-09-07 4:42 ` Leo Liu 2018-09-07 8:17 ` Andreas Röhler 0 siblings, 1 reply; 29+ messages in thread From: Leo Liu @ 2018-09-07 4:42 UTC (permalink / raw) To: Andreas Röhler; +Cc: 31772 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#31772: 26.1; (thing-at-point 'list) regression 2018-09-07 4:42 ` Leo Liu @ 2018-09-07 8:17 ` Andreas Röhler 2018-09-08 0:09 ` Leo Liu 0 siblings, 1 reply; 29+ messages in thread From: Andreas Röhler @ 2018-09-07 8:17 UTC (permalink / raw) To: Leo Liu; +Cc: 31772 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#31772: 26.1; (thing-at-point 'list) regression 2018-09-07 8:17 ` Andreas Röhler @ 2018-09-08 0:09 ` Leo Liu 0 siblings, 0 replies; 29+ messages in thread From: Leo Liu @ 2018-09-08 0:09 UTC (permalink / raw) To: Andreas Röhler; +Cc: 31772 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#31772: 26.1; (thing-at-point 'list) regression 2018-09-06 10:37 ` Leo Liu 2018-09-06 19:01 ` Andreas Röhler @ 2018-09-11 8:31 ` Eli Zaretskii 2018-09-11 10:26 ` Leo Liu 1 sibling, 1 reply; 29+ messages in thread From: Eli Zaretskii @ 2018-09-11 8:31 UTC (permalink / raw) To: Leo Liu; +Cc: 31772, tino.calancha > 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#31772: 26.1; (thing-at-point 'list) regression 2018-09-11 8:31 ` Eli Zaretskii @ 2018-09-11 10:26 ` Leo Liu 2018-09-11 11:16 ` Eli Zaretskii 0 siblings, 1 reply; 29+ messages in thread From: Leo Liu @ 2018-09-11 10:26 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 31772, tino.calancha 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#31772: 26.1; (thing-at-point 'list) regression 2018-09-11 10:26 ` Leo Liu @ 2018-09-11 11:16 ` Eli Zaretskii 2018-09-11 11:52 ` Andreas Röhler 2018-09-11 12:36 ` Leo Liu 0 siblings, 2 replies; 29+ messages in thread From: Eli Zaretskii @ 2018-09-11 11:16 UTC (permalink / raw) To: Leo Liu; +Cc: 31772, tino.calancha > 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#31772: 26.1; (thing-at-point 'list) regression 2018-09-11 11:16 ` Eli Zaretskii @ 2018-09-11 11:52 ` Andreas Röhler 2018-09-11 12:10 ` Eli Zaretskii 2018-09-11 12:36 ` Leo Liu 1 sibling, 1 reply; 29+ messages in thread From: Andreas Röhler @ 2018-09-11 11:52 UTC (permalink / raw) To: 31772 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#31772: 26.1; (thing-at-point 'list) regression 2018-09-11 11:52 ` Andreas Röhler @ 2018-09-11 12:10 ` Eli Zaretskii 0 siblings, 0 replies; 29+ messages in thread From: Eli Zaretskii @ 2018-09-11 12:10 UTC (permalink / raw) To: Andreas Röhler; +Cc: 31772 > 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#31772: 26.1; (thing-at-point 'list) regression 2018-09-11 11:16 ` Eli Zaretskii 2018-09-11 11:52 ` Andreas Röhler @ 2018-09-11 12:36 ` Leo Liu 2018-09-11 12:39 ` Eli Zaretskii 1 sibling, 1 reply; 29+ messages in thread From: Leo Liu @ 2018-09-11 12:36 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 31772, tino.calancha [-- 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 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* bug#31772: 26.1; (thing-at-point 'list) regression 2018-09-11 12:36 ` Leo Liu @ 2018-09-11 12:39 ` Eli Zaretskii 2018-09-14 14:55 ` Leo Liu 0 siblings, 1 reply; 29+ messages in thread From: Eli Zaretskii @ 2018-09-11 12:39 UTC (permalink / raw) To: Leo Liu; +Cc: 31772, tino.calancha > 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#31772: 26.1; (thing-at-point 'list) regression 2018-09-11 12:39 ` Eli Zaretskii @ 2018-09-14 14:55 ` Leo Liu 2018-09-15 9:06 ` Eli Zaretskii 0 siblings, 1 reply; 29+ messages in thread From: Leo Liu @ 2018-09-14 14:55 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 31772-done, tino.calancha 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#31772: 26.1; (thing-at-point 'list) regression 2018-09-14 14:55 ` Leo Liu @ 2018-09-15 9:06 ` Eli Zaretskii 2018-09-15 12:58 ` Leo Liu 0 siblings, 1 reply; 29+ messages in thread From: Eli Zaretskii @ 2018-09-15 9:06 UTC (permalink / raw) To: Leo Liu; +Cc: 31772, tino.calancha > 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#31772: 26.1; (thing-at-point 'list) regression 2018-09-15 9:06 ` Eli Zaretskii @ 2018-09-15 12:58 ` Leo Liu 2018-09-15 13:28 ` Eli Zaretskii 0 siblings, 1 reply; 29+ messages in thread From: Leo Liu @ 2018-09-15 12:58 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 31772, tino.calancha 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#31772: 26.1; (thing-at-point 'list) regression 2018-09-15 12:58 ` Leo Liu @ 2018-09-15 13:28 ` Eli Zaretskii 0 siblings, 0 replies; 29+ messages in thread From: Eli Zaretskii @ 2018-09-15 13:28 UTC (permalink / raw) To: Leo Liu; +Cc: 31772, tino.calancha > 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#31772: 26.1; (thing-at-point 'list) regression 2018-06-11 16:08 ` Leo Liu 2018-06-11 16:25 ` Tino Calancha @ 2018-06-11 16:34 ` Eli Zaretskii 1 sibling, 0 replies; 29+ messages in thread From: Eli Zaretskii @ 2018-06-11 16:34 UTC (permalink / raw) To: Leo Liu; +Cc: 31772, tino.calancha > 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#31772: 26.1; (thing-at-point 'list) regression 2018-06-11 3:17 ` Leo Liu 2018-06-11 15:06 ` Tino Calancha @ 2018-06-11 15:10 ` Eli Zaretskii 2018-06-11 16:36 ` Leo Liu 1 sibling, 1 reply; 29+ messages in thread From: Eli Zaretskii @ 2018-06-11 15:10 UTC (permalink / raw) To: Leo Liu; +Cc: 31772, dgutov, tino.calancha, npostavs > 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#31772: 26.1; (thing-at-point 'list) regression 2018-06-11 15:10 ` Eli Zaretskii @ 2018-06-11 16:36 ` Leo Liu 0 siblings, 0 replies; 29+ messages in thread From: Leo Liu @ 2018-06-11 16:36 UTC (permalink / raw) To: 31772 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#31772: 26.1; (thing-at-point 'list) regression 2018-06-10 15:03 ` Eli Zaretskii 2018-06-10 15:21 ` Tino Calancha @ 2018-06-11 2:32 ` Leo Liu 2018-06-11 14:58 ` Eli Zaretskii 1 sibling, 1 reply; 29+ messages in thread From: Leo Liu @ 2018-06-11 2:32 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 31772, tino.calancha 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#31772: 26.1; (thing-at-point 'list) regression 2018-06-11 2:32 ` Leo Liu @ 2018-06-11 14:58 ` Eli Zaretskii 2018-06-11 17:04 ` Leo Liu 0 siblings, 1 reply; 29+ messages in thread From: Eli Zaretskii @ 2018-06-11 14:58 UTC (permalink / raw) To: Leo Liu; +Cc: 31772, tino.calancha > 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#31772: 26.1; (thing-at-point 'list) regression 2018-06-11 14:58 ` Eli Zaretskii @ 2018-06-11 17:04 ` Leo Liu 0 siblings, 0 replies; 29+ messages in thread From: Leo Liu @ 2018-06-11 17:04 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 31772, tino.calancha 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2018-09-15 13:28 UTC | newest] Thread overview: 29+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-06-10 3:58 bug#31772: 26.1; (thing-at-point 'list) regression Leo Liu 2018-06-10 15:03 ` Eli Zaretskii 2018-06-10 15:21 ` Tino Calancha 2018-06-11 3:17 ` Leo Liu 2018-06-11 15:06 ` Tino Calancha 2018-06-11 16:08 ` Leo Liu 2018-06-11 16:25 ` Tino Calancha 2018-09-06 10:37 ` Leo Liu 2018-09-06 19:01 ` Andreas Röhler 2018-09-07 4:42 ` Leo Liu 2018-09-07 8:17 ` Andreas Röhler 2018-09-08 0:09 ` Leo Liu 2018-09-11 8:31 ` Eli Zaretskii 2018-09-11 10:26 ` Leo Liu 2018-09-11 11:16 ` Eli Zaretskii 2018-09-11 11:52 ` Andreas Röhler 2018-09-11 12:10 ` Eli Zaretskii 2018-09-11 12:36 ` Leo Liu 2018-09-11 12:39 ` Eli Zaretskii 2018-09-14 14:55 ` Leo Liu 2018-09-15 9:06 ` Eli Zaretskii 2018-09-15 12:58 ` Leo Liu 2018-09-15 13:28 ` Eli Zaretskii 2018-06-11 16:34 ` Eli Zaretskii 2018-06-11 15:10 ` Eli Zaretskii 2018-06-11 16:36 ` Leo Liu 2018-06-11 2:32 ` Leo Liu 2018-06-11 14:58 ` Eli Zaretskii 2018-06-11 17:04 ` Leo Liu
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.