Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat(lsp): add --pipe parameter #76351

Merged
merged 5 commits into from
Dec 14, 2024
Merged

feat(lsp): add --pipe parameter #76351

merged 5 commits into from
Dec 14, 2024

Conversation

616b2f
Copy link
Contributor

@616b2f 616b2f commented Dec 10, 2024

resolves #72871

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Dec 10, 2024
@dotnet-policy-service dotnet-policy-service bot added Community The pull request was submitted by a contributor who is not a Microsoft employee. VSCode labels Dec 10, 2024
@JoeRobich JoeRobich self-requested a review December 10, 2024 08:21
@build-analysis build-analysis bot mentioned this pull request Dec 12, 2024
Open
3 tasks
@616b2f
Copy link
Contributor Author

616b2f commented Dec 12, 2024

Hi @JoeRobich I saw you are going to review the PR, are we need some tests for this? Couldn't find any similar tests in the repo.

I had also some issues to run the tests on linux machine, are they supposed to work only on windows?

Thank you in advance.

@JoeRobich
Copy link
Member

@616b2f When you feel good about your PR please mark it as "Ready for review". Thanks!

are we need some tests for this? Couldn't find any similar tests in the repo.

Taking a look and it appears we would need to do a bit of refactoring here to make this testable. You do not need to do that for this PR.

I had also some issues to run the tests on linux machine, are they supposed to work only on windows?

Unfortunately some of unit tests have dependencies that require Windows as the reuse some VS Editor types. This is something we plan to address as the Language Server needs multi-platform coverage.

@616b2f 616b2f marked this pull request as ready for review December 13, 2024 08:52
@616b2f 616b2f requested a review from a team as a code owner December 13, 2024 08:52
JoeRobich
JoeRobich approved these changes Dec 13, 2024
View reviewed changes
Copy link
Member

@JoeRobich JoeRobich left a comment

Choose a reason for hiding this comment

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

Thanks @616b2f!

@JoeRobich JoeRobich merged commit bf617ef into dotnet:main Dec 14, 2024
25 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Dec 14, 2024
This was referenced Dec 17, 2024
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CyrusNajmabadi CyrusNajmabadi CyrusNajmabadi left review comments

@davidwengier davidwengier davidwengier left review comments

@JoeRobich JoeRobich JoeRobich approved these changes

Assignees
No one assigned
Labels
Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee. untriaged Issues and PRs which have not yet been triaged by a lead VSCode
Projects
None yet
Milestone
Next
Development

Successfully merging this pull request may close these issues.

roslyn-lsp --stdio option (or --pipe with a client-created pipe)
4 participants
@616b2f @JoeRobich @davidwengier @CyrusNajmabadi