elbywan / crystalline

A Language Server Protocol implementation for Crystal. 🔮
MIT License
424 stars 21 forks source link

Crystalline is publishing diagnostics for Crystal internal files #15

Closed hugopl closed 3 years ago

hugopl commented 3 years ago

Hi,

The diagnostics are being sent for files in Crystal stdlib and files under project lib directory. This could be skip to save a lot of messages exchange between client and server.

{"jsonrpc":"2.0","method":"textDocument/publishDiagnostics","params":{"uri":"file:///usr/lib/crystal/prelude.cr","diagnostics":[]}}

Also... when a file is saved it send the diagnostics for all files too, I think the spec isn't clear about this but IMO makes more sense to send this diagnostics only for open files... anyway, better look what other LS are doing, so I'm not sure this last paragraph is really an issue.

elbywan commented 3 years ago

Hey @hugopl,

The diagnostics are being sent for files in Crystal stdlib and files under project lib directory. This could be skip to save a lot of messages exchange between client and server.

Also... when a file is saved it send the diagnostics for all files too, I think the spec isn't clear about this but IMO makes more sense to send this diagnostics only for open files...

Yes I understand that this may seem a lot of unnecessary I/O, but it is actually useful 😉.

First, sometimes the Crystal compiler will report errors with the origin coming from a library or an stdlib file - or simply a file that your are not editing and that you may not have opened. It's important to consider that.

In addition, since errors can be caused & solved by editing a file that is different from the one that errors, closing a file and not publishing the diagnostics afterwards (because it is not opened anymore) could make it appear as erroring even though it is not the case anymore. I observed this behaviour while using vscode.

Then I find it practical also to have a view containing a recap of files in your whole project that contain errors, without having to open each and every one of them.

For these reasons I think that it would be better to keep the current logic.

hugopl commented 3 years ago

I understand.... so it makes sense to send errors for all project files, opened or not, including files under project lib dir.

However I still not seeing why would be useful to send diagnostics for files like file:///usr/lib/crystal/prelude.cr. Anyway.... not a big deal, feel free to close the issue if necessary.

elbywan commented 3 years ago

However I still not seeing why would be useful to send diagnostics for files like file:///usr/lib/crystal/prelude.cr.

It's the same logic, errors can come from these files too.

Capture d’écran 2020-12-15 à 17 42 21
hugopl commented 3 years ago

I see.... maybe useful for such a global errors view, not to show in the editor widget itself.