Skip to content

Conversation

@alsi-lawr
Copy link
Contributor

Resolves #266

Copy link
Owner

@razzmatazz razzmatazz left a comment

Choose a reason for hiding this comment

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

Nice! Though ideally I want to have tests added to ensure that something has actually been fixed here and prevent regressions.

@alsi-lawr
Copy link
Contributor Author

alsi-lawr commented Sep 17, 2025

Nice! Though ideally I want to have tests added to ensure that something has actually been fixed here and prevent regressions.

Absolutely, I'll add some tests. I was testing locally swapping out the mason-installed csharp-ls with my build to make sure it was now working. I noticed that there were 3 separate changes to the function call from .NET 8 -> .NET 9.0.100 -> .NET 9.0.300, where the Async function was dropped in the first change and the function arguments were changed in the second change, so I tried to cover all 3 and tested all 3 were working by setting the dotnet version. I'm not sure how I'd replicate that kind of test in a unit test though, but I'll give it a crack.

@alsi-lawr alsi-lawr changed the title feat: support dotnet 9+ extract interface code action draft: feat: support dotnet 9+ extract interface code action Sep 17, 2025
@alsi-lawr alsi-lawr marked this pull request as draft September 17, 2025 11:23
@alsi-lawr alsi-lawr changed the title draft: feat: support dotnet 9+ extract interface code action feat: support dotnet 9+ extract interface code action Sep 17, 2025
@alsi-lawr alsi-lawr marked this pull request as ready for review September 17, 2025 16:19
@alsi-lawr
Copy link
Contributor Author

@razzmatazz I've added some tests (and parallelized the test execution), also moved the task helper to an extension function Task.fromResult

@razzmatazz
Copy link
Owner

There is a lot of whitespace/formatting changes in this PR which makes it a bit hard to read..
What tool did you use to reformat the files?
Ideally this would be done in a separate PR + a linter added to github actions to avoid churn later..

@alsi-lawr
Copy link
Contributor Author

There is a lot of whitespace/formatting changes in this PR which makes it a bit hard to read.. What tool did you use to reformat the files? Ideally this would be done in a separate PR + a linter added to github actions to avoid churn later..

Yeah, really sorry for that, I didn't turn off my formatter (fantomas) and by the time I noticed it was too late. I can try to detangle it and create a new PR somehow

@alsi-lawr alsi-lawr marked this pull request as draft September 18, 2025 10:04
@razzmatazz
Copy link
Owner

There is a lot of whitespace/formatting changes in this PR which makes it a bit hard to read.. What tool did you use to reformat the files? Ideally this would be done in a separate PR + a linter added to github actions to avoid churn later..

Yeah, really sorry for that, I didn't turn off my formatter (fantomas) and by the time I noticed it was too late. I can try to detangle it and create a new PR somehow

Sorry for poking holes in your PR :(

I added a linter step in #271 w/ fantomas 7.0.3 (which has caused yet more reformatting).

@alsi-lawr alsi-lawr marked this pull request as ready for review September 19, 2025 11:03
@alsi-lawr
Copy link
Contributor Author

@razzmatazz I believe the PR is in a good state to be reviewed now, no more formatting changes clogging up the diffs

@razzmatazz razzmatazz merged commit 81e313f into razzmatazz:main Sep 19, 2025
2 checks passed
@razzmatazz
Copy link
Owner

Thank you! This is an outstanding PR.

@razzmatazz
Copy link
Owner

This has been released as part of 0.20.0: https://www.nuget.org/packages/csharp-ls/0.20.0

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.

GetExtractInterfaceOptionsAsync has been replaced with GetExtractInterfaceOptions, breaking support for the "Extract Interface" code action

3 participants