-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Fix layering of C# formatting options #79083
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
Fix layering of C# formatting options #79083
Conversation
| #else | ||
| public enum BinaryOperatorSpacingOptions | ||
| #endif | ||
| internal enum BinaryOperatorSpacingOptionsInternal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was causing no end of pain.
src/Workspaces/CSharp/Portable/Formatting/CSharpFormattingOptions.cs
Outdated
Show resolved
Hide resolved
| } | ||
| } | ||
|
|
||
| public enum LabelPositionOptions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thse are not new apis. they just moved from the shared location her.e we don't want it in teh shared location as that adds public types to multiple assemblies, which is bad (esp. for code that then references both assemblies.
| #else | ||
| public enum LabelPositionOptions | ||
| #endif | ||
| internal enum LabelPositionOptionsInternal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these are now internal and renamed (so no conflicts anymore). all shared code now uses the internal option.
...rkspaces/SharedUtilitiesAndExtensions/Compiler/Core/Options/EditorConfigValueSerializer`1.cs
Show resolved
Hide resolved
|
@JoeRobich @tmat ptal. |
| where TToEnum : struct, Enum | ||
| { | ||
| // Ensure that this is only called for enums that are actually compatible with each other. | ||
| Contract.ThrowIfTrue(typeof(TFromEnum).GetEnumUnderlyingType() != typeof(int)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this just be typeof(TFromEnum).GetEnumUnderlyingType() != typeof(TToEnum).GetEnumUnderlyingType()?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this has changed. i previously took a dependency on int in teh impl code. now i have a TEnumUnderlyingType that i can use instead.
And i do confirm that everything is cromulent between all 3 type parameters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's move convert out of formatting options.
This was done. PTAL @tmat |
| where TToEnum : struct | ||
| where TUnderlyingEnumType : struct | ||
| { | ||
| return Unsafe.As<TUnderlyingEnumType, TToEnum>(ref Unsafe.As<TFromEnum, TUnderlyingEnumType>(ref value)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
witness, the power of .net. note that this should get fully elided to nothing in the jit :)
No description provided.