fix(startup): fix start standalone outside vscode#74
fix(startup): fix start standalone outside vscode#74kieran-ryan merged 1 commit intocucumber:mainfrom
Conversation
|
Can you provide a guide on how to install this branch locally for my neovim config? Seems like it takes too long for the maintainer to merge it |
|
This also fixes #79 |
@pprotas |
|
@binhtran432k after installing your fork from NPM I can get the language server to run without crashing immediately, but upon trying to use it with Neovim it logs the following: I added some code to log |
Resolved by using Node 16 |
|
hi all, thank you for your work! I tried to use your lsp-version in my Lazyvim by your instructions:
return {
{
"neovim/nvim-lspconfig",
opts = {
servers = {
cucumber_language_server = {
-- currently using a forked ls.
mason = false,
root_dir = require("lspconfig.util").root_pattern("pom.xml"),
settings = {
cucumber = {
features = { "**/*.feature" },
glue = { "**/*Steps.java" },
},
},
},
},
},
},
}Now, after loading my Java repo, or a JS repo...LSP seems to be active but formatting and finding stepDefs does not work. Haven't tried Vscode. LspLog: I don't really know what this means. At least it seems we are a few steps further. |
I have the same problem myself, I also get Undefined step everywhere |
|
Using the branch {
cmd = { "~/.nvm/versions/node/v16.20.2/bin/cucumber-language-server", "--stdio" },
settings = {
cucumber = {
cucumber = {
features = { "**/*.feature" },
glue = { "**/*Step*.java" },
}
}
},
on_attach = function(client, bufnr)
print("attached to cucumber file")
vim.keymap.set('n', "gd", vim.lsp.buf.definition, { buffer = 0 })
vim.keymap.set('n', "gn", vim.diagnostic.goto_next, { buffer = 0 })
vim.keymap.set('n', "gb", vim.diagnostic.goto_prev, { buffer = 0 })
end
} |
e6c05cd to
514f3da
Compare
There was a problem hiding this comment.
Firstly, thanks @binhtran432k for making an excellent contribution to the project - and welcome to our Slack community also; secondly thanks all for your patience on this - a review has been pending for a considerable time.
I'd like to support to get this reviewed and merged. However, I'm unfamiliar with Neovim - so you might have to bear with me while I get to grips with spinning up the Language Server with it, hopefully end of next week.
To close this out, some items we may need to achieve are as follows:
- Tested in VSCode - see contributing guidelines
- Tested on Windows - particularly the file path change
- Consider whether unit tests can be written covering the file path change - ideally tests that fail with the previous state and pass with the changed state
- Ensure changing from
file:///to a path would not have any impact if we were to enable the language server to work in browser-based contexts in future - Consider whether we should include a lightweight
UsageorInstallationsection in theREADMEwith a subsection for using Neovim - explaining how to complete installation - perhaps this is an area you could assist @harleyreevesmartin - as a newer user looking to contribute and as I observe you are a Neovim user
514f3da to
1bdc6c0
Compare
1bdc6c0 to
96bf033
Compare
|
I managed to get it working with a little hacking using Since So if anyone using //cucumber-language-server/node_modules/.bin/cucumber-language-server
#!/change/this/path/to/v16.17.0/bin/node
/* eslint-disable @typescript-eslint/no-var-requires */
require('source-map-support').install()
const url = require('url')
const { startNodeServer, NodeFiles } = require('@cucumber/language-server/node')
const wasmBaseUrl = url.pathToFileURL(
`file:///${__dirname}/../node_modules/@cucumber/language-service/dist`
)
startNodeServer(wasmBaseUrl.href, (rootUri) => new NodeFiles(rootUri))Another detail I had to add was to double up the require("lspconfig").cucumber_language_server.setup({
settings = {
cucumber = {
features = { "**/*.feature" },
glue = { "**/*.steps.ts" },
parameterTypes = {},
},
features = { "**/*.feature" },
glue = { "**/*.steps.ts" },
parameterTypes = {},
},
})
|
96bf033 to
9733fec
Compare
Hi @kumpan-david, thank you again for your help. Unfortunately, a significant refactor implemented after v1.2.0 makes reverting to that version impractical. However, I'm glad to report that this fix works with the latest version. I appreciate your understanding. |
|
Hi @kieran-ryan, I've pushed new commits that accomplish the following: Testing:
Unit Testing:
External VSCode Usage:
Please review the changes and let me know if you have any questions or feedback. |
|
@binhtran432k from a fellow neovim user, thank you so much! This issue has been bugging me for months! |
|
Any chance this can get merged anytime soon? |
There was a problem hiding this comment.
Excellent work @binhtran432k - appreciate the changes. Just two small comments to clarify the README documentation and to reuse some test code, and then we can merge and go for release. Well done! 👏
Also if could confirm unit tests are executing with node v18 - I had some issues but might be local.
test(standalone): test startup success docs: add external vscode usage session
9733fec to
7cda2e9
Compare
|
@kieran-ryan, Thank you for your review. I have just completed the requested changes. |
|
Thanks everyone 🙏 this is a life saver |
|
v1.5.0 is now available 🚀 |
🤔 What's changed?
⚡️ What's your motivation?
Fixes #72, #79, #90, #96
🏷️ What kind of change is this?
♻️ Anything particular you want feedback on?
First, The
url.pathToFileURLwas introduced by #68 to fix #66 but it did not fix the problem and #66 was closed by using another product.After that, the #73 change how
language-serverworked. It refactorsrc/wasm/startWasmServer.tsintosrc/wasm/startEmbeddedServer.tsandsrc/wasm/startStandaloneServer.ts. ThestartEmbeddedServeris used for VS Code with browser support and thestartStandaloneServeris used as fallback for user outside of VS Code but it was not tested properly.As a result, the language server is rendered unusable outside of VS Code, and this PR aims to rectify this issue.
📋 Checklist:
This text was originally generated from a template, then edited by hand. You can modify the template here.