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.

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

amend cursor position decrement logic & create language service command kernel test #3281

Merged
merged 4 commits into from
Oct 19, 2023

Conversation

cartercanedy
Copy link
Contributor

Changes:

  • Refactored the logic around cursor position decrement to skip conditionals altogether if cursor position is at pos 0
  • Added a test to catch a kernel crash on submission of a language service command where the cursor position is set to 0 in a 0-length buffer

@colombod
Copy link
Member

Thank you for this contribution

@colombod colombod merged commit 87f66a3 into dotnet:main Oct 19, 2023
4 checks passed
@colombod colombod added the Area-Language Services IntelliSense, LSP, and related label Oct 19, 2023
@shyamnamboodiripad
Copy link
Contributor

Thanks for the contribution @cartercanedy! Could you please clarify why the test was excluded from netfx? I see that some other tests in the file were already excluded - but I don't remember why.

@cartercanedy
Copy link
Contributor Author

It looks like the test case is available for both netcore and netfx through Microsoft.DotNet.Interactive.Tests Microsoft.DotNet.Interactive.VisualStudio.Tests. The test uses a CSharpKernel nested in a CompositeKernel, which is unavailable in the latter.

@shyamnamboodiripad
Copy link
Contributor

Ah indeed - that makes sense. I was wondering if there was some other reason why the test fails on netfx. Thanks for clarifying.

@cartercanedy cartercanedy deleted the create-tests branch February 7, 2024 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Language Services IntelliSense, LSP, and related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants