Better support for optional arguments #12
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…attribute to ensure the mapping is done properly
- Set NullValueHandling to NullValueHandling.Ignore, so that null values are not serialized - If .NET primitive types (like bool or long) are optional, use nulltable types, so that they can be ignored when serializing
Member
|
Wow, this one seemed to have gotten lost. Is it still worth merging this one? I think so, but I'm afraid it might break the other pull requests here? Any thoughts? |
Contributor
Author
|
Well, in the end it's mainly the code-generating logic that has changed. This is a pretty important one for us, so it would be good if you can accept this. I'd suggest to do this before #15, and have #15 re-generate the C# code based on these changes. That way, ChromeDevTools is fully up to date once more :) I'll try to take some time later today to fix issues with the .NET Core tooling - VS2017 has been released so that needs some updating. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The debugger protocol defines optional arguments (like the
executionContextId argument on the runScript method.When this property is present in the JSON, even with the default value of
0, the Chrome debugger will try to interpret this value. This caused some issues for me - I don't know which value to provide for theexecutionContextIdand Chrome wasn't happy with the default value of0, but if I removed theexecutionContextIdproperty from the generated JSON, all was well.This PR changes the code generation of optional properties of type
longandboolto use nullable typeslong?andbool?, and adds a[JsonProperty(NullValueHandling = NullValueHandling.Ignore)]attribute to all operational properties, ensuring they are absent from the generated JSON unless explicitely set.This PR builds on #10 (which now has passing unit tests) so you'd probably want to review that PR first, and then evaluate this one.
If you have questions/feedback, let me know!