PMunch / nimlsp

Language Server Protocol implementation for Nim
MIT License
418 stars 51 forks source link

Port nimsuggestapi from nimlangserver #155

Closed iacore closed 1 year ago

iacore commented 1 year ago

After this patch, nimlsp is easier to build. This only requires nimsuggest on PATH.

I didn't understand anything. I simply fixed all type errors after copying suggestapi.nim from nimlangserver. And then it works.

Thankfully suggestlib.nim (your code) is not far from suggestapi.nim (nimlangserver).

PMunch commented 1 year ago

This is the way NimLSP used to work, before langserver was written. But having nimsuggest marshal Nim types into a format and then NimLSP unmarshal those types into the same structure seemed a bit too silly. If nimsuggest changed then you'd get parsing errors, there is much less help from the type system while developing, and in general the whole thing just felt brittle and needlessly abstracted. That is why NimLSP was rewritten to remove this step and just use nimsuggest directly as a library. This unfortunately leads to a bit more cumbersome build structure, because you now need nimsuggest sources (which live within the compiler). If Nim had better support for this, and choosenim got hooks the situation would be much improved.

Langserver was implemented mostly because people disagreed with this choice. Instead of improving Nimsuggest and the build support systems they decided to implement langserver in the old way NimLSP used to work. They have different pros and cons, so having both isn't a bad thing. The reason suggestlib.nim and suggestapi.nim are so similar is probably because much of the dirty work was copied from NimLSP to langserver to get things up and running faster.

So all in all, this change doesn't really make much sense. If you want a tool which works this way why not just use langserver? This should AFAIK be the biggest difference between the two tools.