-
-
Notifications
You must be signed in to change notification settings - Fork 689
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
Cucumber language server, VSCode extension and Monaco editor #1689
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice @aslakhellesoy!
I don't have a ton of context around LSP and VSCode so not much specific feedback, but looks good to me.
One thought: given the issues around long build times etc, and the fact that these modules don't feed into a cucumber implementation, would they live better as their own repo?
if (signal) { | ||
reject(new Error(`Received signal ${signal}`)) | ||
} | ||
// https://github.com/cucumber/cucumber-js/issues/1768 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hopefully we can address this with the new API anyway (I'm finally starting to work on that now).
Interested in thoughts about - in addition to a runCucumber
that does the full test run lifecycle - exposing a few more targeted functions that can help with things like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Being able to invoke Cucumber.js via API would be an improvement. That said, the language server is designed to work with any Cucumber implementation that emits Cucumber Messages (Cucumber-JVM and Cucumber-Ruby), so for those implementations we'll have to spawn a process anyway.
We need to add a setting so users can define/override what command to run to generate the messages.
Yeah, really nice! 👏 Regarding the review, should we focus on something specific? |
Adding more modules to the monorepo increases the serial build, but not the parallel one. Given the very long serial build I've started to question whether we need it at all. If we remove it, adding new modules would not affect build times in CI. The modules I have added here have dependencies on other monorepo modules, and I had to modify a few of them as part of building this. This would have been very cumbersome to do had they not been in the same repo. Bottom line - I think we should address long build times by parallelising our build rather than spltting up in multiple repos. |
Summary
This PR adds four new modules:
@cucumber/language-service
- the services used by@cucumber/language-server
and@cucumber/monaco
@cucumber/language-server
- a language server that can be used by any LSP client@cucumber/vscode
- a Visual Studio Code extension that uses@cucumber/language-server
@cucumber/monaco
- configures monaco editor to use@cucumber/language-service
(for browser-based Gherkin editors)Motivation and Context
Today's developers expect excellent language support in editors and IDEs. The rise of VSCode has popularised language servers as a way to provide this.
There is already a VSCode / language server at https://marketplace.visualstudio.com/items?itemName=alexkrechik.cucumberautocomplete that I considered improving, but I think it's important to be in control over the implementation.
The existing VSCode extension differed significantly from my vision that I didn't consider improving it with pull requests.
How Has This Been Tested?
vscode/javascript
in vscode and press F5 to try the plugin)Screenshots (if appropriate):
Types of changes
Checklist: