-
Notifications
You must be signed in to change notification settings - Fork 364
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
Partial Roslyn rewrite, supporting new language features #980
base: dev
Are you sure you want to change the base?
Partial Roslyn rewrite, supporting new language features #980
Conversation
Hey, @codecadwallader sorry to bother you, but have you been able to look at this PR or resolve #979? |
Sorry Timothy, I have not yet the time to review it. I really appreciate you putting it together, we've been meaning to move towards Roslyn for a long time. Have you been able to play with it yourself? I'm curious if there will be any performance implications from mixing the old APIs and new APIs together. |
Very early on, I was able to debug the extension, but after updating Visual Studio 2022 code maid would throw an error during // Error is throw in one of these methods
await RegisterCommandsAsync();
await RegisterEventListenersAsync();
I don't know enough about Roslyn or
I wouldn't bother putting too much effort looking at this. As I haven't been able to properly debug it. It's definitely full of bugs. tbh I'd hoped someone experienced with Roslyn would take pity and take over 😄
If you want to progressively transition then |
I have been having issues with getting the VS debugger to work as well. I recently switched to a new computer and it started working again so I'm curious if there isn't some kind of state that is being maintained in the experimental instance and interfering, or if there's something that has changed across versions that requires a clean install. |
Have you been able to run/debug the roslyn version? I tried with an old computer with VS 2019, I installed VS extension tools and cloned the main branch. I got the same error popup and it didn't work. I'll try using a clean install. What bugs me is that I could definitely debug the extension until I did a VS update, after that even backdating didnt work |
Hey, kept running into #708 and wanted to learn some roslyn and got carried away.
I combined CodeMaidPlus and used a lot of code from
Roslyn.CSharpAddAccessibilityModifiersDiagnosticAnalyzer
to add explicit access modifiers for all the currently codemaid supported types in addition torecords
andrecord struct
. Because the logic is a modifed default VS analyzer/rewriter, cleanup will account for new features likeref struct
,required
andfile
instead of erasing them.I've tried to avoid using
Format(SyntaxNode, SyntaxAnnotation, Workspace)
as it would add line padding between declarations if a change occured, potentially conflicting with the chosen 'Insert blank line before' options. To do this I copiedCSharpSyntaxGenerator.WithAccessibility
and modified it to add trailing trivia to each accesor token.insert blank line padding before
,insert blank line padding between
,insert blank line padding after
andinsert explicit access modifiers
.comments
,region
andrecord
.After updating Visual Studio I experienced the same bug as #979 so I've been unable to debug this extension and instead relied on my unit test 😄
This PR is incomplete and likely contains some obvious mistakes, hopefully someone with roslyn knowlefge can improve on this.
Assuming this PR is successful and roslyn is used, would you consider adding
do
while
to "between chained statements" or "insert blank line padding" before switch expression arms?