Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Let's get rid of the direct nimsuggest integration (i.e. support only nimlangserver) #7

Open
nickysn opened this issue Dec 11, 2023 · 16 comments

Comments

@nickysn
Copy link
Collaborator

nickysn commented Dec 11, 2023

Rationale:

  1. Removing the direct nimsuggest integration makes the VS code extension easier to maintain. Its functionality is redundant and the code is very Visual Studio Code-specific, while nimlangserver implements the same features in a way that is standardized in the LSP protocol and easier to integrate with other IDEs. It makes no sense to continue maintaining the nimsuggest backend (which contains lots of code), when VS Code offers an LSP client as well.
  2. Using nimlangserver offers extra functionality (like inlay hints). The ability to use nimsuggest directly hides this functionality from the user.
  3. The direct nimsuggest integration makes compilation of the extension more difficult and dependant on a specific nimsuggest version (as it requires copying files from the nimsuggest directory from the Nim compiler sources). In comparison, nimlangserver is capable of using multiple nimsuggest versions (it has nimsuggest version detection now).
@jmgomez
Copy link
Collaborator

jmgomez commented Dec 12, 2023

We can also postpone this one after the first release.

@nickysn
Copy link
Collaborator Author

nickysn commented Dec 12, 2023

I think we should disable it, without removing the code, before the release, though. That would be much less work and would prevent people, who couldn't get nimlangserver working from complaining about removing functionality, when we actually remove the nimsuggest integration in a future update.

@jmgomez
Copy link
Collaborator

jmgomez commented Dec 12, 2023

ok

@nickysn
Copy link
Collaborator Author

nickysn commented Dec 12, 2023

Or we can change the default of "nim.enableNimsuggest" to false and mark both "nim.enableNimsuggest" and ""nim.provider" config options as deprecated, by adding a "deprecationMessage": https://code.visualstudio.com/api/references/contribution-points#contributes.configuration

@jmgomez
Copy link
Collaborator

jmgomez commented Dec 12, 2023

all right, feel free to do it and if Im not missing anything else we should be good to go now

@nickysn
Copy link
Collaborator Author

nickysn commented Dec 12, 2023

Pull request: #12

@lewisl
Copy link

lewisl commented Sep 10, 2024

Let's not. nimlangserver is even less reliable than nimsuggest. No thanks. Does nimlangserver even have a maintainer?

@nickysn
Copy link
Collaborator Author

nickysn commented Sep 10, 2024

Let's not. nimlangserver is even less reliable than nimsuggest. No thanks.

Nimlangserver is just a thin wrapper around nimsuggest, so it should offer the same stability as nimsuggest, with the added bonus that crashing nimsuggest doesn't crash the language server.

Besides, the developer who made the original nimsuggest integration doesn't seem to be working on the extension anymore. The original extension is here, you can stay with it, but it seems to be abandoned:

https://github.com/saem/vscode-nim

None of the current developers of this extension want to maintain the direct nimsuggest integration.

Does nimlangserver even have a maintainer?

Yes, it does. On the contrary, the original direct nimsuggest integration has no maintainer. If you have any issues with nimlangserver, please report them as bugs.

@lewisl
Copy link

lewisl commented Sep 10, 2024 via email

@Araq
Copy link
Member

Araq commented Sep 10, 2024

Nimlangserver is just a thin wrapper around nimsuggest, so it should offer the same stability as nimsuggest, with the added bonus that crashing nimsuggest doesn't crash the language server.

I've no opinion on whether to remove the nimsuggest support or not.

I just want to point out that in general a wrapper around a service can indeed make things worse. ;-)

@lewisl
Copy link

lewisl commented Sep 10, 2024 via email

@nickysn
Copy link
Collaborator Author

nickysn commented Sep 10, 2024

Nimlangserver is just a thin wrapper around nimsuggest, so it should offer the same stability as nimsuggest, with the added bonus that crashing nimsuggest doesn't crash the language server.

I've no opinion on whether to remove the nimsuggest support or not.

I have. The nimsuggest integration has no maintainer and is a lot of code that is tightly coupled with VS Code's JavaScript interface. It makes the plugin more difficult to build (requires nimsuggest interface, which is part of the Nim compiler). It is difficult to extend (I don't know how to add e.g. inlay hints to it) and is basically a dead end, due to lack of maintainers. Removing the direct nimsuggest integration would make the plugin much easier to maintain. And people who use it might as well use Saem's extension. It's the same thing, as far as direct nimsuggest integration is concerned. None of the improvements that we have implemented in the LSP server will ever be available in the direct nimsuggest interface.

It's much better if we address the issues that people have with the LSP server in order to move forward.

Another alternative is if someone steps up and starts maintaining the direct nimsuggest integration, but so far, no such people have appeared.

I just want to point out that in general a wrapper around a service can indeed make things worse. ;-)

Yes, it can. It's usually caused by things like bad configuration (unfortunately, the README.md contained broken configuration that caused the server to crash - it's fixed now, but many people tried it before the fix and gave up on using the LSP server) or installation issues (nimble problems or missing dlls). We're working to address these issues. Any new issues and problems that are reported will be looked at and addressed. The claim that the nim language server is not maintained is false or maybe outdated.

@Araq
Copy link
Member

Araq commented Sep 10, 2024

Yes, it can. It's usually caused by things like bad configuration ...

For the Nim language server, maybe, but I really meant "in general", like for example: The wrapper introduces new memory leaks or even memory corruptions or it has a bug that causes some wrong interprocess message translation.

@lewisl
Copy link

lewisl commented Sep 11, 2024 via email

@jmgomez
Copy link
Collaborator

jmgomez commented Sep 11, 2024

A good start is to continue the conversation that you started here: nim-lang/langserver#228

@lewisl
Copy link

lewisl commented Sep 12, 2024

I am going to apologize. Part of what I said was wrong and my fault and part was still correct.
I have 2 machines with 2 different installs: 1 built from source and 1 installed from brew. That made me over-react. I apologize; I was wrong. No install of nim was deleted by Nimble.

There are actual issues:

  1. choosenim can't build for arm64 architecture. Still a problem.
  2. nimble will install nim if it doesn't find it where it expects it. This is less clear and doesn't always happen. But, it can happen. But, it won't remove an installation from some other directory: I was wrong about this.
  3. The ecosystem is uneven and the hardest thing to get right with very few paid contributors, only a handful of corporate/institutional users, and few academic supporters. Not saying more support isn't deserved, but it's lacking. Only a few "new" languages have achieved it; most haven't and probably won't. Julia is the high water mark of philanthropic, corporate and academic support: very hard to replicate all they had going for them (and there were early growing pains to be sure...). Go is another example with massive support from Google (not saying anything good or bad about the language)--just that Google very likely helped bootstrap adoption. Many other languages have similar struggles. Nim has dramatically improved the core language and std library over time--not all "new" languages can say that.

Probably need to work towards having some "blessed" packages accessible from Nimble with some way to flag this. It's in the language's interest to have a handful of very useful, high quality packages available even if they originated from an independent source.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants