GNU bug report logs -
#29193
26.0.90; Using (thing-at-point 'sexp) in flymake-diag-region might be suboptimal
Previous Next
Reported by: Dmitry Gutov <dgutov <at> yandex.ru>
Date: Tue, 7 Nov 2017 15:29:01 UTC
Severity: minor
Found in version 26.0.90
Done: Dmitry Gutov <dgutov <at> yandex.ru>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 29193 in the body.
You can then email your comments to 29193 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#29193
; Package
emacs
.
(Tue, 07 Nov 2017 15:29:01 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Dmitry Gutov <dgutov <at> yandex.ru>
:
New bug report received and forwarded. Copy sent to
bug-gnu-emacs <at> gnu.org
.
(Tue, 07 Nov 2017 15:29:01 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
Example:
Rubocop can report warning when 'end' is at wrong column. It just
reports the beginning column, of course.
In ruby-mode, (thing-at-point 'sexp) signals an error at this position.
I'm not sure exactly whether it's a problem in ruby-mode.
But Flycheck uses (thing-at-poing 'symbol) for the same purpose, and the
whole 'end' token gets highlighted (which is probably what we expect).
In contrast, Flymake only highlights its first character ('e').
++
In GNU Emacs 26.0.90 (build 5, x86_64-pc-linux-gnu, GTK+ Version 3.22.11)
of 2017-11-07 built on zappa
Repository revision: ca2d94ba61dee678f85bfc7299d167e7219e6d8f
Windowing system distributor 'The X.Org Foundation', version 11.0.11903000
System Description: Ubuntu 17.04
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#29193
; Package
emacs
.
(Sat, 12 Dec 2020 11:40:01 GMT)
Full text and
rfc822 format available.
Message #8 received at 29193 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dgutov <at> yandex.ru> writes:
> Rubocop can report warning when 'end' is at wrong column. It just
> reports the beginning column, of course.
>
> In ruby-mode, (thing-at-point 'sexp) signals an error at this position.
> I'm not sure exactly whether it's a problem in ruby-mode.
>
> But Flycheck uses (thing-at-poing 'symbol) for the same purpose, and the
> whole 'end' token gets highlighted (which is probably what we expect).
>
> In contrast, Flymake only highlights its first character ('e').
I had a look at the current `flymake-diag-region', and it does not use
(thing-at-point 'sexp) at present. It does use (end-of-thing 'sexp),
though.
So is this problem still present? There was no recipe for reproduction,
so it's not immediately clear.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#29193
; Package
emacs
.
(Sat, 12 Dec 2020 23:57:02 GMT)
Full text and
rfc822 format available.
Message #11 received at 29193 <at> debbugs.gnu.org (full text, mbox):
On 12.12.2020 13:39, Lars Ingebrigtsen wrote:
> Dmitry Gutov <dgutov <at> yandex.ru> writes:
>
>> Rubocop can report warning when 'end' is at wrong column. It just
>> reports the beginning column, of course.
>>
>> In ruby-mode, (thing-at-point 'sexp) signals an error at this position.
>> I'm not sure exactly whether it's a problem in ruby-mode.
>>
>> But Flycheck uses (thing-at-poing 'symbol) for the same purpose, and the
>> whole 'end' token gets highlighted (which is probably what we expect).
>>
>> In contrast, Flymake only highlights its first character ('e').
>
> I had a look at the current `flymake-diag-region', and it does not use
> (thing-at-point 'sexp) at present. It does use (end-of-thing 'sexp),
> though.
The behavior is the same: when 'end' is misindented, only 'e' in 'end'
is highlighted.
Whereas the message is:
Layout/EndAlignment: ‘end‘ at 23, 3 is not aligned with ‘class‘ at 5, 4.
That highlighting is not too hard to interpret, but we could do better,
probably.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#29193
; Package
emacs
.
(Sun, 13 Dec 2020 12:53:01 GMT)
Full text and
rfc822 format available.
Message #14 received at 29193 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dgutov <at> yandex.ru> writes:
>> I had a look at the current `flymake-diag-region', and it does not
>> use
>> (thing-at-point 'sexp) at present. It does use (end-of-thing 'sexp),
>> though.
>
> The behavior is the same: when 'end' is misindented, only 'e' in 'end'
> is highlighted.
>
> Whereas the message is:
>
> Layout/EndAlignment: ‘end‘ at 23, 3 is not aligned with ‘class‘ at 5, 4.
>
> That highlighting is not too hard to interpret, but we could do
> better, probably.
OK, adding João to the Cc's.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#29193
; Package
emacs
.
(Sun, 13 Dec 2020 13:20:01 GMT)
Full text and
rfc822 format available.
Message #17 received at 29193 <at> debbugs.gnu.org (full text, mbox):
Lars Ingebrigtsen <larsi <at> gnus.org> writes:
> OK, adding João to the Cc's.
Not sure why 'symbol and 'sexp don't reference the same "thing", or at
least stuff consistent in terms of nesting. In my testing
(thing-at-point 'sexp) at the "e" got me nothing, but at the "n" or "d"
got me the whole sexp. Similar strange situation with "begin". "b"
sees the whole sexp, any of "egin" doesn't. This seems to resist
different indentations.
In the end flymake-diag-region tries its best to guess a region from
limited line/col stuff and asks thingatpt.el for help, which in turn
will probably ask the major-mode for syntactic navigation.
So I wouldn't say the problem is in flymake.el, but in whom is
untimately providing the (broken) goods. Then again maybe this untested
patch would work and not break much stuff for other backends...
diff --git a/lisp/progmodes/flymake.el b/lisp/progmodes/flymake.el
index efa7b2ffbf..6c3e0a1981 100644
--- a/lisp/progmodes/flymake.el
+++ b/lisp/progmodes/flymake.el
@@ -437,7 +437,8 @@ flymake-diag-region
(if (and col (cl-plusp col))
(let* ((beg (progn (forward-char (1- col))
(point)))
- (sexp-end (ignore-errors (end-of-thing 'sexp)))
+ (sexp-end (or (ignore-errors (end-of-thing 'sexp))
+ (ignore-errors (end-of-thing 'symbol))))
(end (or (and sexp-end
(not (= sexp-end beg))
sexp-end)
Other than that, I don't have much to add. I don't have the resources
to debug ruby-mode and/or install rubocop right now. Good luck.
João
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#29193
; Package
emacs
.
(Sun, 13 Dec 2020 20:56:01 GMT)
Full text and
rfc822 format available.
Message #20 received at 29193 <at> debbugs.gnu.org (full text, mbox):
On 13.12.2020 15:19, João Távora wrote:
> - (sexp-end (ignore-errors (end-of-thing 'sexp)))
> + (sexp-end (or (ignore-errors (end-of-thing 'sexp))
> + (ignore-errors (end-of-thing 'symbol))))
That seems to fix it, thank you.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#29193
; Package
emacs
.
(Mon, 14 Dec 2020 11:04:01 GMT)
Full text and
rfc822 format available.
Message #23 received at 29193 <at> debbugs.gnu.org (full text, mbox):
On Sun, Dec 13, 2020 at 8:55 PM Dmitry Gutov <dgutov <at> yandex.ru> wrote:
>
> On 13.12.2020 15:19, João Távora wrote:
> > - (sexp-end (ignore-errors (end-of-thing 'sexp)))
> > + (sexp-end (or (ignore-errors (end-of-thing 'sexp))
> > + (ignore-errors (end-of-thing 'symbol))))
>
> That seems to fix it, thank you.
Feel free to push if you can't come up with anything better in ruby-mode.
João
Reply sent
to
Dmitry Gutov <dgutov <at> yandex.ru>
:
You have taken responsibility.
(Wed, 16 Dec 2020 00:57:01 GMT)
Full text and
rfc822 format available.
Notification sent
to
Dmitry Gutov <dgutov <at> yandex.ru>
:
bug acknowledged by developer.
(Wed, 16 Dec 2020 00:57:02 GMT)
Full text and
rfc822 format available.
Message #28 received at 29193-done <at> debbugs.gnu.org (full text, mbox):
On 14.12.2020 13:03, João Távora wrote:
> Feel free to push if you can't come up with anything better in ruby-mode.
Pushed, and closing, thanks.
Regarding "doing the fix in the proper place", the alternative would be
changing the implementation of thing-at-point--end-of-sexp.
But I'm not sure it's a well-defined function. It clearly special-cases
characters with class "closing paren". And there's no clear counterpart
for 'end' (even though it's a sexp closer in ruby-mode's syntax).
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#29193
; Package
emacs
.
(Thu, 17 Dec 2020 23:26:01 GMT)
Full text and
rfc822 format available.
Message #31 received at 29193 <at> debbugs.gnu.org (full text, mbox):
fda9a2 causes a test failure.
Ref eg: https://hydra.nixos.org/build/132986117
(Are people trying to run foo-tests.el before pushing changes to foo.el?)
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#29193
; Package
emacs
.
(Fri, 18 Dec 2020 02:07:02 GMT)
Full text and
rfc822 format available.
Message #34 received at 29193 <at> debbugs.gnu.org (full text, mbox):
On 18.12.2020 01:25, Glenn Morris wrote:
> fda9a2 causes a test failure.
>
> Ref eg:https://hydra.nixos.org/build/132986117
Thanks! It was a curious bug: one would expect that some highlighting's
boundaries might change. What changed, however, is one's _type_.
I've pushed a fix, and I'll let the Flymake's maintainer sort out at his
leisure whether flymake-diag-region can be relied on not to change match
data, as several of its clients seem to assume.
> (Are people trying to run foo-tests.el before pushing changes to foo.el?)
Some of us are spoiled enough to expect an email from the CI when some
breakage occurs.
(Sorry.)
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#29193
; Package
emacs
.
(Fri, 18 Dec 2020 11:43:02 GMT)
Full text and
rfc822 format available.
Message #37 received at 29193 <at> debbugs.gnu.org (full text, mbox):
On Fri, Dec 18, 2020 at 2:07 AM Dmitry Gutov <dgutov <at> yandex.ru> wrote:
>
> On 18.12.2020 01:25, Glenn Morris wrote:
> > fda9a2 causes a test failure.
> >
> > Ref eg:https://hydra.nixos.org/build/132986117
>
> Thanks! It was a curious bug: one would expect that some highlighting's
> boundaries might change. What changed, however, is one's _type_.
>
> I've pushed a fix, and I'll let the Flymake's maintainer sort out at his
> leisure whether flymake-diag-region can be relied on not to change match
> data, as several of its clients seem to assume.
I think this is rather a thingatpt.el issue, which makes no mention of
match-data destruction. Regardless, it's decent enough to do that in
flymake-diag-region so I pushed save-match-data a bit higher.
Thanks,
João
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#29193
; Package
emacs
.
(Fri, 18 Dec 2020 15:23:01 GMT)
Full text and
rfc822 format available.
Message #40 received at 29193 <at> debbugs.gnu.org (full text, mbox):
On 18.12.2020 13:42, João Távora wrote:
> I think this is rather a thingatpt.el issue, which makes no mention of
> match-data destruction. Regardless, it's decent enough to do that in
> flymake-diag-region so I pushed save-match-data a bit higher.
By default we assume that any function can destroy match-data (and only
a certain set doesn't).
But it's an easy mistake to make, especially given the fairly opaque
implementation of (end-of-thing 'symbol).
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#29193
; Package
emacs
.
(Fri, 18 Dec 2020 15:27:01 GMT)
Full text and
rfc822 format available.
Message #43 received at 29193 <at> debbugs.gnu.org (full text, mbox):
On Fri, Dec 18, 2020 at 3:22 PM Dmitry Gutov <dgutov <at> yandex.ru> wrote:
>
> On 18.12.2020 13:42, João Távora wrote:
>
> > I think this is rather a thingatpt.el issue, which makes no mention of
> > match-data destruction. Regardless, it's decent enough to do that in
> > flymake-diag-region so I pushed save-match-data a bit higher.
>
> By default we assume that any function can destroy match-data (and only
> a certain set doesn't).
By that reasoning the problem should be solved in whoever called
flymake-diag-region and wrongly assumed it would keep match data.
Doesn't a seem very friendly API tho (and no idea who it was, anyway).
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#29193
; Package
emacs
.
(Fri, 18 Dec 2020 15:30:02 GMT)
Full text and
rfc822 format available.
Message #46 received at 29193 <at> debbugs.gnu.org (full text, mbox):
On 18.12.2020 17:26, João Távora wrote:
> On Fri, Dec 18, 2020 at 3:22 PM Dmitry Gutov <dgutov <at> yandex.ru> wrote:
>>
>> On 18.12.2020 13:42, João Távora wrote:
>>
>>> I think this is rather a thingatpt.el issue, which makes no mention of
>>> match-data destruction. Regardless, it's decent enough to do that in
>>> flymake-diag-region so I pushed save-match-data a bit higher.
>>
>> By default we assume that any function can destroy match-data (and only
>> a certain set doesn't).
>
> By that reasoning the problem should be solved in whoever called
> flymake-diag-region and wrongly assumed it would keep match data.
Yes.
> Doesn't a seem very friendly API tho (and no idea who it was, anyway).
Also true. So we can document that flymake-diag-region does indeed
preserve match data.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#29193
; Package
emacs
.
(Fri, 18 Dec 2020 15:40:01 GMT)
Full text and
rfc822 format available.
Message #49 received at 29193 <at> debbugs.gnu.org (full text, mbox):
On Fri, Dec 18, 2020 at 3:29 PM Dmitry Gutov <dgutov <at> yandex.ru> wrote:
> > Doesn't a seem very friendly API tho (and no idea who it was, anyway).
> Also true. So we can document that flymake-diag-region does indeed
> preserve match data.
Done
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org
.
(Sat, 16 Jan 2021 12:24:08 GMT)
Full text and
rfc822 format available.
This bug report was last modified 3 years and 73 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.