Feat/dart #68
Feat/dart #68
Conversation
There was a problem hiding this comment.
Everything looks great! However, the unittests are currently failing possibly because dart was not found in the environment. We will need it to work in the CI. You can look at https://github.com/pratham1002/multilspy/blob/69373e057e75585f58b27c767b6f460f9954825e/.github/workflows/test-workflow.yaml#L15-L19 for reference.
I would prefer to the extent possible, that the lsp executable is downloaded in the setup_runtime_dependencies, and if and only if we determine that that will be infeasible/undesirable, then this route (of adding sections to set it up separately in the CI) can be taken.
What are your thoughts?
There was a problem hiding this comment.
This is good to start with now, but can we maybe add tests for any other commonly used lsp features as well?
src/multilspy/language_servers/dart_language_server/initialize_params.json
Outdated
Show resolved
Hide resolved
src/multilspy/language_servers/dart_language_server/dart_language_server.py
Outdated
Show resolved
Hide resolved
|
Also please add the name "dart" in relevant places in the README! This is indeed a major contribution! |
There was a problem hiding this comment.
Do we also want to add a workspace configuration file https://github.com/dart-lang/sdk/blob/main/pkg/analysis_server/tool/lsp_spec/README.md#client-workspace-configuration? Could be useful, but can leave out for now if it is too much work.
There was a problem hiding this comment.
to be honest i do not know what that configuration is for, so i wouldnt be sure what to add exactly
would u mind sending me some docs or adding it yourself if possible?
|
Closes #53 |
|
LGTM! Thanks a lot for your contribution. As next steps, I would request to create a few issues for things that this PR doesn't add, which would be desirable for Dart support (support in non-linux environments to automatically download and run the lsp, configuration options, etc.). I will merge this now unless you had any changes. |
|
sound good to me, i dont have any other changes for now but ill make sure to add a few more tests during the week if you could help me with the initialization options thatd be great thanks for all ur comments, ill open a new pr if i make any other changes |
No description provided.