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

Json unicode escape && preserveOrder keys sync #2145

Merged
merged 7 commits into from Feb 8, 2018
Merged

Json unicode escape && preserveOrder keys sync #2145

merged 7 commits into from Feb 8, 2018

Conversation

aleks-f
Copy link
Member

@aleks-f aleks-f commented Feb 7, 2018

@aleks-f aleks-f added this to the Release 1.8.2 milestone Feb 7, 2018
@aleks-f aleks-f self-assigned this Feb 7, 2018
Copy link
Member

@obiltschnig obiltschnig left a comment

Choose a reason for hiding this comment

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

See my inline comment.


explicit Object(bool preserveInsertionOrder = false);
explicit Object(bool preserveInsertionOrder = false, bool escapeUnicode = false);
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should think about introducing an options enum for the various places where we use preserveInsertionOrder and escapeUnicode, and use an int options instead of a potentially growing list of bool args. If we just change the Object constructor to:

Object(int options = 0);

and define:

enum Options
{
    JSON_OPT_PRESERVE_INSERTION_ORDER = 1,
    JSON_OPT_ESCAPE_UNICODE = 2
};

Then the new constructor taking int will be backwards compatible to the one taking bool.

@Burgch
Copy link
Contributor

Burgch commented Feb 8, 2018

This looks great, thanks again!

@aleks-f aleks-f merged commit df5968c into pocoproject:develop Feb 8, 2018
Bjoe pushed a commit to Bjoe/poco that referenced this pull request Feb 19, 2018
* Only escape compulsory characters for JSON by default

* add enable/disable unicode escaping flags to stringify pocoproject#2137

* JSON::Object preserveOrder keys not synced on assignment pocoproject#2142

* fix assignment preserveOrder and add object copy tests

* add object move tests

* fix comment

* enum for JSON::Object options
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants