-
Notifications
You must be signed in to change notification settings - Fork 363
VB.Net Code Element Positions: Fixing issue #333 #444
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
VB.Net Code Element Positions: Fixing issue #333 #444
Conversation
I've also added some updates to handle formatting of VB.Net code (line spacing in particular). The original CodeCleanup methods were using a very minimal |
… (modifiers are not the same)
… (modifiers are not the same)
…does not need to be specified. Replicate changes to CodeItemUsingStatement's RefreshCachesPositionAndName for consistency.
Thank you, I sincerely appreciate the time you have put into these changes and think this is well under way to being accepted. :) I've been reviewing and testing the changes this morning. I confirmed what you saw, there was no difference in C# between the StartPoint property and the GetStartPoint() method but there was a difference in VB. With that knowledge I was more comfortable with the location of the change. I made a couple very minor edits and pushed them into your branch. I also reviewed many (hopefully all) other occurrences of StartPoint/EndPoint outside of BaseCodeItemElement and they were logically fine the way they were. For example calculating complexity only uses the internal component of the method and the attributes are not relevant. For the second stage of your changes (extending VB cleanups) its a great extension of VB capabilities. How much have you tested each component of those changes to make sure they are VB friendly? For example I believe UpdateEndRegionDirectives is only written to support C# region directives. It would be very helpful to test each component of that cleanup (like you have with explicit access modifiers) to make sure they are VB compatible. Also as a general convention I would ask that commented out code be deleted. Thanks again! |
You are welcome! Oh the fun to be had waiting on laundry :) I'm not a heavy user of regions. I did some basic tests and they seemed to act accordingly in the VB code I have. I did test the formatting pretty extensively. This is how I found I needed to remove some of the access modifier stuff since it was breaking Async methods. I believe this is because the access modifiers aren't the same in VB as they are for C# and the modifier text is hard coded to the C# language. So I commented it out for now. I removed the commented code as asked :) I was hesitant on if I really wanted it removed or if I might code up the workaround. As it is now, I think its "good enough" as it works well now in our rather large VB.Net solution. |
…bugs that got merged wrong (git problems)
Awesome, thanks for the tweaks. I did one more sanity pass through the code and removed the UpdateEndRegionDirectives call. It wasn't causing any harm since it was looking for C# directive strings to match on, but it also wasn't doing anything productive. Pull request accepted, merged, and now available in our CI channel here for testing :) http://vsixgallery.com/extension/4c82e17d-927e-42d2-8460-b473ac7df316/ This is a really welcome improvement and I'm very grateful for the time you took to do it. I'll plan to push this out to the public as a v10.4 release in a couple weeks following some time for any reports to come in from users of the CI channel. Thanks again! |
Fix for #333 VB: Drag & Drop Re-ordering does not move attributes.
Fundamentally, the
CodeElement.StartPoint
works differently in VB.Net than it does for C#. In VB.Net, the property does NOT include attributes. It does for C#. Be nice if VB.Net and C# played by the same rules, but here we are.In order to make this work the same in both versions, I modified code to use the
GetStartPoint
method, rather than the property and uses thevsCMPart.vsCMPartWholeWithAttributes
flag to get the entire code segment with attributes. Also did the same forEndPoint
(although I don't think there were problems with it, I did it to be consistent).Things working "wrong" in VB.Net before this change included:
With this change VB.Net now behaves the same as C# (as much as I am able to tell from testing against a variety of source code I am using). C# doesn't appear to be behaving any differently than before.
I know this may be a low level change, but I believe this change mimics what was originally intended (the
StartPoint
/EndPoint
pair needs to include the attributes).