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 null to pj and use for relocs #15776

Merged
merged 3 commits into from Jan 9, 2020
Merged

Add null to pj and use for relocs #15776

merged 3 commits into from Jan 9, 2020

Conversation

thestr4ng3r
Copy link
Contributor

"N/A" makes no sense in json

@github-actions github-actions bot added the API New API requests, changes, removal label Jan 8, 2020
@XVilka XVilka added this to the 4.2.0 (Arctic World Archive) milestone Jan 8, 2020
libr/core/cbin.c Outdated
if (relname && *relname) {
pj_ks (pj, "name", relname);
} else {
pj_knull (pj, "name");
Copy link
Collaborator

Choose a reason for hiding this comment

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

what about not showing this field just in case there's no name, programatically it will be handled the same when parsing it and it is how its done in other places in r2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It will not be handled the same in almost all parsers. But whether it is better in this case to omit the entire name or use null is a subjective question. If you prefer to have it removed, I can do that.

@@ -28,13 +28,15 @@ R_API PJ *pj_o(PJ *j);
R_API PJ *pj_a(PJ *j);
/* keys, values */
R_API PJ *pj_k(PJ *j, const char *k);
R_API PJ *pj_knull(PJ *j, const char *k);
Copy link
Collaborator

Choose a reason for hiding this comment

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

i dont like to use "null" here wwhen all the other cases have 1 letter to define the type.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

propose a better name

Copy link
Collaborator

@radare radare left a comment

Choose a reason for hiding this comment

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

.

@radare
Copy link
Collaborator

radare commented Jan 8, 2020 via email

@radare
Copy link
Collaborator

radare commented Jan 8, 2020 via email

@thestr4ng3r
Copy link
Contributor Author

I don't know the exact specification of json schema, but I am pretty sure it is possible to model both optional fields and nullable fields with it.

@radare radare merged commit 4b02315 into master Jan 9, 2020
@thestr4ng3r thestr4ng3r deleted the pj-null branch January 9, 2020 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API New API requests, changes, removal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants