-
Notifications
You must be signed in to change notification settings - Fork 41
Feature/yaxlib visual studio debugger friendly #227
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
Feature/yaxlib visual studio debugger friendly #227
Conversation
…is serializer in Visual Studio Debugger extension [ "The function evaluation requires all threads to run"](https://stackoverflow.com/questions/4460206/lazyt-the-function-evaluation-requires-all-threads-to-run) 2. Add the DontSerializeDefaultValues option to YAXSerializationOptions.
cache member type by type and SerializerOptions MemberWrapper<Type, IList<MemberWrapper> -> MemberWrapper<<(Type type, SerializerOptions options), IList<MemberWrapper>>
|
Hi Yevhen Thanks for the PR. It includes very nice improvements, while it's really huge and will require some time process. Breaking changes
Few things that I came across when going throw the code for the first time:
Naming conventions
What do you think - does this make sense to you? |
…nment.CurrentManagedThreadId
|
Hi @axunonb, I will try to fix the SonarCloud Quality Gate errors and fill out the XmlDoc as well.
I found the breaking changes |
|
Hi Yevhen, |
|
Hi @axunonb, |
|
Thanks! As you mentioned built-in types like |
|
Hi Yevhen, |
|
Guess, I've already finished this PR. The goal of the two last commits was to make the usage of ITypeInspector easy and do it similarly to Json.NET.ContractResolver or YamlDotNet.TypeInspectorSkeleton |
Yevhen, I'm not happy going back to supporting Net Framework 4.5. Even .NetFramework 4.5.2, 4.6, 4.6.1 have reached end of support last year, and there has never been a request to target for .Net Framework 4.5. |
|
OK. Not a problem. Agree with you. |
| /// <summary> | ||
| /// The member context interface provides information about the attributes of a member and member metadata. | ||
| /// </summary> | ||
| public interface IMemberContext |
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.
IMemberContext should be kept compatible with v4.1.0 so there's no need for a new major version
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.
Done. Please take a look.
| /// <summary> | ||
| /// The member's <see cref="MemberInfo" /> for member serialization, else <see langword="null" />. | ||
| /// </summary> | ||
| MemberInfo MemberInfo { get; } |
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.
Keep, but mark as "Obsolete, will be removed in a future version"
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.
Done. Please take a look.
| /// The member's <see cref="PropertyInfo" /> for property serialization, else <see langword="null" />. | ||
| /// The member's <see cref="IMemberDescriptor" /> for member serialization, else <see langword="null" />. | ||
| /// </summary> | ||
| PropertyInfo? PropertyInfo { get; } |
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.
Keep, but mark as "Obsolete, will be removed in a future version"
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.
Done. Please take a look.
| /// The member's <see cref="IMemberDescriptor" /> for member serialization, else <see langword="null" />. | ||
| /// </summary> | ||
| PropertyInfo? PropertyInfo { get; } | ||
| IMemberDescriptor MemberInfo { get; } |
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.
Maybe rename as MemberDescription
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.
Done as MemberDescriptor. Please take a look.
axunonb
left a comment
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.
Make the PR compatible with v4.1.0 to a avoid a new major version.
| WrappedField.SetValue(obj, value); | ||
| } | ||
|
|
||
| public override string ToString() |
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 be public override string? ToString()?
See https://sonarcloud.io/summary/new_code?id=axunonb_YAXLib&branch=pr%2F227
Maybe cover this method with unit test?
| WrappedProperty.SetValue(obj, value, index); | ||
| } | ||
|
|
||
| public override string ToString() |
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 be public override string? ToString()?
See https://sonarcloud.io/summary/new_code?id=axunonb_YAXLib&branch=pr%2F227
Maybe cover this method with unit test?
axunonb
left a comment
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.
Excellent, thanks a lot!
We could merge this into a version/4.2.0 branch,
and then make DateTime a known type, and also add DateTimeOffset and IpAddress as known types as you suggested.
All together would then be the v4.2.0 release. Okay?
|
Yup, agree with you. I would also add the KeyValuePair to known types. |
Hi YAXLib team.
I want to use your fantastic library in my Object Dumper extension for Visual Studio.
Experimental version of extension
What is done in this PR:
Deprecation:
Improvements:
Get rid of Lazy & ThreadLocal as they do impossible using this serializer in Visual Studio Debugger extension "The function evaluation requires all threads to run"
Add the DoNotSerializeDefaultValues option to YAXSerializationOptions
Add the TypeInspector property to SerializerOptions (allows type name and members customization without attributes)
MemberWrapperCache and UdtWrapperCache: cache the type by type and SerializerOptions.
Fixed failing tests: if choose Get_PooledObject and Parallel_Load_On_Specialized_Pools and run them only, the Parallel_Load_On_Specialized_Pools will fail.