-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Code Quality: Improved DependencyProperty generator #16025
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
Conversation
7bcf657
to
ff7cc61
Compare
acd9112
to
09d923c
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Thanks for your help. 🤔 Since we've made it to the eve of the partial properties, maybe we can wait for C#13 to come out before making adjustments? That way we can implement it in this syntax: [DependencyProperty(PropertyChangedCallback, etc.)]
public partial object? MyProperty { get; private set; } = new Object(); This way is much easier and more readable. And perhaps CommunityToolkit would like to implement a better version? |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
If you wish to do so, then yes. You can get it through ((TypeDeclarationSyntax)generatorAttributeSyntaxContext.TargetNode).AttributeLists[0].Attributes[0].ArgumentList.Arguments[0].Expression or ((AttributeSyntax)attributeData.ApplicationSyntaxReference).ArgumentList.Arguments[0].Expression However the arguments of the attributes only allow constants, which I don't think is very useful, so I used |
This comment was marked as resolved.
This comment was marked as resolved.
Oh sorry, I didn't test it. It should be ((AttributeSyntax)attributeData.ApplicationSyntaxReference.GetSyntax()).ArgumentList.Arguments[0].Expression |
Worked thanks!! |
d0445e4
to
f2e8349
Compare
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.
Looks good to me
That didn't work; somehow emitted property type string like "IconType" and caused an invalid cast exception. Reverted to your original approach, I think we can wait for .NET partial properties to achieve this as you suggested. Again, ready. |
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.
LGTM
@0x5bfa are you ready for me to merge this? |
YES!! |
Fixed |
Resolved / Related Issues
The primary achievement is to reduce lines of code and verbosity when declaring DependencyProperties.
Steps used to test these changes
None