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

Add JsonKey.includeFromJson/includeToJson - allow explicit control of serialization #1256

Merged
merged 5 commits into from
Nov 30, 2022

Conversation

kevmoo
Copy link
Collaborator

@kevmoo kevmoo commented Nov 22, 2022

...and deprecate JsonKey.ignore

Fixes #24
Fixes #274
Fixes #537
Fixes #569
Fixes #797
Fixes #1100
Fixes #1244

@ImadBouirmane
Copy link

Useful, I will use it, can I have a simplify description about this new improvement on Json abilities in our codes !

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Deprecate JsonKey.ignore

Fixes #24
Fixes #274
Fixes #537
Fixes #569
Fixes #797
Fixes #1100
Fixes #1244
@kevmoo kevmoo force-pushed the i569_all_the_control branch from c7bc099 to a2a3629 Compare November 22, 2022 16:05
@ValentinVignal
Copy link

Is there any reason you use this FieldUsage enum with 4 values instead of 2 boolean parameters (ex: ignoreInFromJson and ignoreInToJson) ?

@kevmoo
Copy link
Collaborator Author

kevmoo commented Nov 22, 2022

@ValentinVignal great question! It comes down to terseness. I believe it's easier to document and explain...at least a bit. I realize it's the same state-space. Happy to discuss further, though.

@kevmoo kevmoo changed the title [DRAFT] Add JsonKey.usage - allow explicit control of serialization Add JsonKey.usage - allow explicit control of serialization Nov 30, 2022
@kevmoo kevmoo marked this pull request as ready for review November 30, 2022 03:34
/// super classes, sorted first by their location in the inheritance hierarchy
/// (super first) and then by their location in the source file.
Iterable<FieldElement> createSortedFieldSet(ClassElement element) {
List<FieldElement> createSortedFieldSet(ClassElement element) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just a small implementation tweak...

@@ -85,28 +86,44 @@ class GeneratorHelper extends HelperCore with EncodeHelper, DecodeHelper {
final createResult = createFactory(accessibleFields, unavailableReasons);
yield createResult.output;

accessibleFieldSet = accessibleFields.entries
final fieldsToUse = accessibleFields.entries
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Needs to be something sortable...

for (var candidate in sortedFields.where((element) =>
jsonKeyFor(element).explicitYesToJson &&
!fieldsToUse.contains(element))) {
fieldsToUse.add(candidate);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Need to add candidates BACK even if they are not used in the factory if they are forced to be used for toJSON

Copy link
Collaborator

Choose a reason for hiding this comment

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

make these comments actual comments?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done!

}

fieldsToUse.sort(
(a, b) => sortedFields.indexOf(a).compareTo(sortedFields.indexOf(b)));
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Need the fields to maintain the original source ordering

@kevmoo kevmoo changed the title Add JsonKey.usage - allow explicit control of serialization Add JsonKey.includeFromJson/includeToJson - allow explicit control of serialization Nov 30, 2022
@kevmoo kevmoo merged commit 776df7c into master Nov 30, 2022
@kevmoo kevmoo deleted the i569_all_the_control branch November 30, 2022 17:24
@feduke-nukem
Copy link

I wonder, how I am able to use these fixes? @kevmoo

@kevmoo
Copy link
Collaborator Author

kevmoo commented Jan 19, 2023

@feduke-nukem ...what do you mean?

@feduke-nukem
Copy link

feduke-nukem commented Jan 19, 2023

@feduke-nukem ...what do you mean?

I am not able to find JsonKey.writeOnly and others mentoned here in the latest version of package.
Am I misunderstanding something?

@kevmoo
Copy link
Collaborator Author

kevmoo commented Jan 19, 2023

@feduke-nukem – that's because I haven't published it yet! Ha! Let me see if I can do that today

@feduke-nukem
Copy link

feduke-nukem commented Jan 19, 2023

@feduke-nukem – that's because I haven't published it yet! Ha! Let me see if I can do that today

That will be really nice 😁

@kevmoo
Copy link
Collaborator Author

kevmoo commented Jan 19, 2023

@feduke-nukem – that's because I haven't published it yet! Ha! Let me see if I can do that today

That will be really nice 😁

Published!

@feduke-nukem
Copy link

@feduke-nukem – that's because I haven't published it yet! Ha! Let me see if I can do that today

That will be really nice 😁

Published!

Thanks!


bool get explicitYesToJson => includeToJson == true;

bool get explicitNoToJson => includeFromJson == false;
Copy link

Choose a reason for hiding this comment

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

Shouldn't this be includeToJson == false?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yup! Great find! #1281

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants