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

67 method with a default enum parameter wich type is byte does not compile #70

Conversation

ChristianSauer
Copy link
Collaborator

I worked on this on friday but did not push it.
It conflicts with your idea @simonmckenzie . What is your opinion?

I also suspect some more issues with enums, e.g. usage in params and so on. Will try to to test this out now

@simonmckenzie
Copy link
Contributor

It generates better code than my version, and with less code, so I'm all for this approach!

@ChristianSauer
Copy link
Collaborator Author

Ok, will get it merge ready

@simonmckenzie
Copy link
Contributor

Something I wasn't sure about is what will happen if the enum type is in an external DLL - will the enum variable names still resolve? I'm not entirely sure how much of the externally-referenced code will be present.

It'd also be good to see a test with a flags enum and a combined default value, more to understand what it generates than out of any fear it would break.

@@ -15,7 +15,8 @@ private static string InheritDoc(ISymbol source) =>
private static readonly SymbolDisplayFormat FullyQualifiedDisplayFormat =
new(
genericsOptions: SymbolDisplayGenericsOptions.IncludeTypeParameters,
memberOptions: SymbolDisplayMemberOptions.IncludeParameters,
memberOptions: SymbolDisplayMemberOptions.IncludeParameters
| SymbolDisplayMemberOptions.IncludeContainingType,
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't believe this is a member option! 🤦‍♀️ I didn't even consider that a member option could control default parameter rendering!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I found that also quite surprising. I played around until it worked ;).
Not ideal, but nothing of that is documented...

@ChristianSauer ChristianSauer merged commit 8e451ff into master Jan 28, 2025
2 checks passed
@ChristianSauer ChristianSauer deleted the 67-method-with-a-default-enum-parameter-wich-type-is-byte-does-not-compile branch January 28, 2025 08:22
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.

2 participants