From: Theodor Thornhill via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
To: "João Távora" <joaotavora@gmail.com>
Cc: 50244-done@debbugs.gnu.org,
Philipp Stephani <p.stephani2@gmail.com>,
Dmitry Gutov <dgutov@yandex.ru>
Subject: bug#50244: 28.0.50; Support project-wide diagnostics reports in flymake.el
Date: Sat, 23 Oct 2021 22:50:00 +0200 [thread overview]
Message-ID: <m15ytnqxdz.fsf@Frende-MacBook.lan> (raw)
In-Reply-To: <CALDnm50qm0mp8Pv8rv7827QW3Z_cNs3_AS5_-MnUmEjBJCu=hg@mail.gmail.com>
João Távora <joaotavora@gmail.com> writes:
> On Sat, Oct 23, 2021 at 8:22 PM Theodor Thornhill <theo@thornhill.no> wrote:
>> (sent this mail some days ago, but the bug was archived, so not sure if
>> it worked. Trying again. If this is only noise, then I'm sorry, but
>> hopefully ok considering i found a typo in the old patch :))
>
> No problem, and yes I got the other one. It's just I've been again
> ompletely utterly busy so it'll be a while. I took the Flymake thing out
> of my brain cache and it's going to take a while to get it back.
>
Didn't mean to rush things, we do have time :)
>> I finally did get to test this out a little, and I seem to have hit some
>> small snags I can't really understand. I'd love some pointers as to how
>> to continue!
>> (the patch I currently use is attached at the bottom)
>> Ok, so what happens is:
>>
>> - There is no backend set on the list-only-diagnostics as for now, and I
>> cannot for the life of me see how to set them, as it doesn't look like
>> eglot itself sets it. Is there some reflection going on here that I
>> have missed?
>
> No, I don't think so. Maybe I simply decided that no backend was
> necessary? Is it? If it is, we might to add some mechanism so
> that it is assigned.
>
I don't think a backend really is necessary anymore. I've been checking
with more servers lately, and it seems to work better with some than
with others. And especially elmls seems a little wonky here.
>> - The function in question is riddled with fallbacks and failure
>> checking. Are we sure we need all that? I see most of the commits
>> relating to the range checking is from 2018, so they may not be
>> relevant anymore?
>
> I'm not sure I follow here. I wouldn't delete any checks until I'm
> sure they are doing nothing. 2018 or not, I do remember a lot of
> servers providing very strange diagnostics and invalid ranges.
>
Yeah, I didn't remove any checks yet, and likely will not. I'm merely
guessing that the lsp scene has settled a bit lately.
>> - The diff as supplied appends the list-only-diagnostics just fine to
>> the flymake buffer, but I believe since the backend isn't set, eglot
>> doesn't report the proper diagnostics when I visit the file. I have
>> to make a bogus edit to trigger the eglot-flymake-backend function.
>> (This could just as well be some unrelated eglot/elmls issue...)
>
> Ah, this rings a bell. You're talking about visiting a file by clicking a
> "list-only diagnostic" in the list, right? And then you wnat Flymake to
> resume checking and replace them with "real" ones. Yes, might be
> eglot/elmls issue, but usually there's not much we can do: some servers
> are like that.
>
Yes, you are probably correct here.
> To keep things consistent we could remove the relevant list-only
> diagnostics and re-report them via the normal mechanism if the server
> is taking too long.
>
I'll look into this, and see if I can find a solution for this.
> Do you understand this idea? The user wouldn't, in principle, be able to
> tell that the freshly annotated diagnostics weren't collected just now
> when he visited the buffer, but much much earlier. The project-wide
> listing should, in principle, also remain consistent.
>
> ...though probably unsorted. If I remember correctly consistent sorting
> is still a problem.
>
Yes sorting is a little off, and I'm looking into that as well. I think
we need to sort on filename, line and col in that order to get it
correct, right?
>> - What would be the best way to get the proper line and column? Now I
>> simply use the range, but considering there's a lot of error handling
>> there I guess that is a little risky? I did have a version earlier
>> using with-temp-buffer and finding the correct positions, but that
>> seemed more complicated than needed.
>
> Risky for what? I don't follow
>
I'm thinking that if I avoid doing the same error handling as in the
normal case (which requires a live buffer), chances are that the range
may be wrong. But since I didn't yet see such a case, I'm thinking some
of the error handling is redundant. I don't have enough proof yet to
remove them, but there may be some problems that I didn't yet discover
with the patch I sent.
> If you used with-temp-buffer, I guess you visited the file. That would
> go against the idea of list-only-diagnostics, which are supposed
> to be for unvisited files (if the file were visited you'd have Eglot/Flymake
> actually checking). So using the LSP range and converting it to
> flymake-make-diagnostic's protocol seems ok.
>
Great! Yes, I didn't want to visit the buffer, but that seemed as a good
way to go at the time, considering that it at least won't clutter the
bufferlist with every file in a project on load :)
>> I'd love to get your thoughts on this :)
>
> Hope I could help and sorry if the thoughts were a little sparse.
> As I said, this is out of my cache and will be for some time.
>
No, this is great. It confirmes some of my hunches, so that is good at
least.
> Do consider patches/changes to flymake.el as well. This new
> mechanism is very new, it's possible it needs some tweaks.
>
I think most of what _I_ need is covered, in some form or other. I'll
prepare a patchset for both eglot and maybe flymake sometime soon, and
you can look at it when you have some free time and energy.
Theo
next prev parent reply other threads:[~2021-10-23 20:50 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-29 0:53 bug#50244: 28.0.50; Support project-wide diagnostics reports in flymake.el João Távora
2021-08-29 23:27 ` Dmitry Gutov
2021-08-30 7:00 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-08-30 8:46 ` João Távora
2021-09-11 1:08 ` João Távora
2021-09-13 0:08 ` Dmitry Gutov
2021-09-13 6:48 ` João Távora
2021-09-13 18:03 ` João Távora
2021-09-13 19:47 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-09-13 20:04 ` João Távora
2021-09-13 20:21 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-09-14 8:50 ` João Távora
2021-09-14 9:21 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-09-14 11:34 ` João Távora
2021-09-14 12:22 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-10-23 19:22 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-10-23 20:22 ` João Távora
2021-10-23 20:50 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors [this message]
2021-09-13 20:11 ` Dmitry Gutov
2021-09-14 8:20 ` João Távora
2021-09-16 22:27 ` Dmitry Gutov
2021-09-16 23:37 ` João Távora
2021-09-13 20:26 ` Dmitry Gutov
2021-09-13 20:53 ` João Távora
2021-09-13 23:35 ` Dmitry Gutov
2021-09-14 8:43 ` João Távora
2021-09-16 22:19 ` Dmitry Gutov
2021-09-16 23:36 ` João Távora
2021-09-18 1:19 ` Dmitry Gutov
2021-09-18 9:59 ` João Távora
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=m15ytnqxdz.fsf@Frende-MacBook.lan \
--to=bug-gnu-emacs@gnu.org \
--cc=50244-done@debbugs.gnu.org \
--cc=dgutov@yandex.ru \
--cc=joaotavora@gmail.com \
--cc=p.stephani2@gmail.com \
--cc=theo@thornhill.no \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).