Shopify / vscode-shopify-ruby

An opinionated and auto-configured set of extensions for Ruby development
MIT License
176 stars 13 forks source link

Feature request: built-in support for syntax_suggest #575

Closed swrobel closed 8 months ago

swrobel commented 9 months ago

syntax_suggest is a great feature of recent Rubies. Editor integration for it would be a welcome addition!

EDIT: Here's an example of a plugin for solargraph that does the same as what I'm suggesting here

vinistock commented 9 months ago

Thank you for the feature suggestion!

The new Ruby parser Prism comes with error tolerance and the Ruby LSP uses that information to surface them in the editor.

The Prism team has mostly been focusing on compatibility with Ruby for now, so there's a lot to improve in terms of how the errors are reported and the correctness of the errors.

That said, when efforts are put into Prism to enhance the error tolerance information, is there any advantage to using Syntax suggest?

My understanding is that it shows syntax errors with suggestions on how to fix it, which will eventually be handled by Prism itself. If this is the case, then there's nothing for us to do, all of the work is on the Prism side.

swrobel commented 9 months ago

I don't really know anything about Prism, other than that it's brand new, or whether this extension is using it (I don't see any references to it in the repo).

You are correct about the purpose of syntax_suggest. It uses Prism under the hood (if available), and is ready to be used now to improve Dx, which I think is a major advantage over Prism, based on my understanding of your summary of the state of its syntax error feedback.

vinistock commented 8 months ago

Well, this repo is just the extension pack that includes a set of other extensions. It doesn't have any features other than the setting recommendations.

The https://github.com/Shopify/vscode-ruby-lsp repo is the extension that spawns the language server, which then uses Prism as the parser.

Yeah, I think adding syntax suggest is not really worth the effort. Prism is already able to show syntax errors, although not always super precise. As soon as the precision of the errors are improved in Prism, the Ruby LSP will automatically show then in the right place.

If we were to add syntax suggest, then we'd just end up removing it once Prism errors are more precise. In addition to that, Prism errors are reported as an outcome of parsing, which makes reporting syntax errors very performant. With syntax suggest, we'd need to run something else on top of parsing, which would slow down the LSP.