nim-lang / vscode-nim

A VS Code plugin for the Nim language
Other
78 stars 8 forks source link

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

Open nickysn opened 11 months ago

nickysn commented 11 months ago

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 commented 11 months ago

We can also postpone this one after the first release.

nickysn commented 11 months ago

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 commented 11 months ago

ok

nickysn commented 11 months ago

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 commented 11 months ago

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

nickysn commented 11 months ago

Pull request: https://github.com/nim-lang/vscode-nim/pull/12

lewisl commented 2 months ago

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

nickysn commented 2 months ago

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 commented 2 months ago

I appreciate the goal to move forward and that the lsp api is the way forward. The problem is that I have seen repeated crashes and disconnections of the language server with the nim-lang vscode plugin. I have seen none with the older plugin. So, something is going wrong and there seems little attention to multiple users reporting these problems. Happy to wait; in the meantime I’ll use the unmaintained plugin with few crashes. In time the new solution will be clearly better.

From: Nikolay Nikolov @.> Reply-To: nim-lang/vscode-nim @.> Date: Tuesday, September 10, 2024 at 10:15 AM To: nim-lang/vscode-nim @.> Cc: Lewis Levin @.>, Comment @.***> Subject: Re: [nim-lang/vscode-nim] Let's get rid of the direct nimsuggest integration (i.e. support only nimlangserver) (Issue #7)

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.

— Reply to this email directly, view it on GitHubhttps://github.com/nim-lang/vscode-nim/issues/7#issuecomment-2341525152, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAIYWLKK6IHWUQ3ERRBQPFLZV4SJ5AVCNFSM6AAAAABN7FQJGWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNBRGUZDKMJVGI. You are receiving this because you commented.Message ID: @.***>

Araq commented 2 months ago

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 commented 2 months ago

Thanks.

All things in time. I sent a bunch of error messages in. I didn’t really understand what caused them; hard to repro. Just keep your VS Code open to several Nim files at a time and the problems will eventually occur.

From: Andreas Rumpf @.> Reply-To: nim-lang/vscode-nim @.> Date: Tuesday, September 10, 2024 at 1:13 PM To: nim-lang/vscode-nim @.> Cc: Lewis Levin @.>, Comment @.***> Subject: Re: [nim-lang/vscode-nim] Let's get rid of the direct nimsuggest integration (i.e. support only nimlangserver) (Issue #7)

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. ;-)

— Reply to this email directly, view it on GitHubhttps://github.com/nim-lang/vscode-nim/issues/7#issuecomment-2341928448, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAIYWLL6VGC6DH3HKG3XFQDZV5HHFAVCNFSM6AAAAABN7FQJGWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNBRHEZDQNBUHA. You are receiving this because you commented.Message ID: @.***>

nickysn commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

I am willing to confess I might have a bad configuration after multiple attempts.

What should I look at and provide?

From: Andreas Rumpf @.> Reply-To: nim-lang/vscode-nim @.> Date: Tuesday, September 10, 2024 at 1:47 PM To: nim-lang/vscode-nim @.> Cc: Lewis Levin @.>, Comment @.***> Subject: Re: [nim-lang/vscode-nim] Let's get rid of the direct nimsuggest integration (i.e. support only nimlangserver) (Issue #7)

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.

— Reply to this email directly, view it on GitHubhttps://github.com/nim-lang/vscode-nim/issues/7#issuecomment-2341981671, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAIYWLLWWHHJZKPV5UDEWLLZV5LF5AVCNFSM6AAAAABN7FQJGWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNBRHE4DCNRXGE. You are receiving this because you commented.Message ID: @.***>

jmgomez commented 2 months ago

A good start is to continue the conversation that you started here: https://github.com/nim-lang/langserver/issues/228

lewisl commented 2 months ago

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.