Skip to content

Conversation

@tomorrowCoder
Copy link
Contributor

Currently works only for Linux. For windows integration, need to set run time dependencies properly.

@tomorrowCoder
Copy link
Contributor Author

@microsoft-github-policy-service agree

@LakshyAAAgrawal
Copy link
Collaborator

Hi @tomorrowCoder

Thank you so much for all the PRs you have kindly created.

At the moment the unit tests are failing. This is due to some external issue in the unit tests, which I have fixed in the latest commit to main branch.
Could you please merge/rebase on top of main branch to get the unit tests in the CI working again, after which I will review and merge the PR.

Could you please do this for all your PRs?

@tomorrowCoder
Copy link
Contributor Author

@LakshyAAAgrawal I have merged main with my fork. I'm not sure how to trigger unit tests in CI Pipeline. Please let me know if there are further issues I need to look into. Thanks

@tomorrowCoder
Copy link
Contributor Author

Current unit test failure is related to java. I'm not sure if this is observed in main branch

FAILED tests/multilspy/test_multilspy_java.py::test_multilspy_java_example_repo_document_symbols - multilspy.lsp_protocol_handler.server.Error: Internal error. (-32603)

.gitignore Outdated
src/multilspy/language_servers/rust_analyzer/static/
src/multilspy/language_servers/typescript_language_server/static/
src/multilspy/language_servers/dart_language_server/static/
src/multilspy/language_servers/clangd_language_server/clangd/
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would recommend preserving the naming convention. Everything downloaded at runtime should go under static/

assert len(runtime_dependencies) == 1
dependency = runtime_dependencies[0]

clangd_ls_dir = os.path.join(os.path.dirname(__file__), "clangd")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above. Change name to static. You can create a nested directory under it called clangd if required.


async with lsp.start_server():
# Wait for server to be fully initialized
await lsp.server_ready.wait()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally, lsp.start_server should wait for server_ready, and within the context manager, the user should directly be able to call into lsp apis.

code_language = Language.CPP
params = {
"code_language": code_language,
"repo_url": "https://github.com/tomorrowCoder/yaml-cpp/",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you link to the original repository directly instead of the fork?

@LakshyAAAgrawal
Copy link
Collaborator

Current unit test failure is related to java. I'm not sure if this is observed in main branch

Don't worry about the java failures. There is a lot of flakiness in the java tests. I have rerun the CI.

@tomorrowCoder
Copy link
Contributor Author

Addressed review comments

@LakshyAAAgrawal
Copy link
Collaborator

Closes #14

@LakshyAAAgrawal
Copy link
Collaborator

@tomorrowCoder, I have approved the PR for merging. Let me know if you have any changes you would like to make, otherwise LGTM!

@LakshyAAAgrawal LakshyAAAgrawal merged commit ab58821 into microsoft:main Apr 4, 2025
4 checks passed
@LakshyAAAgrawal
Copy link
Collaborator

@tomorrowCoder , thank you very much for your contribution! This has been a long-standing issue of high interest, and I am glad that we finally have a working CPP lang server in multilspy!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants