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

Unicode escaping #22

Merged
merged 7 commits into from
Jan 11, 2015
Merged

Unicode escaping #22

merged 7 commits into from
Jan 11, 2015

Conversation

Teemperor
Copy link
Contributor

Implements the \uXXXX and \uXXXX\uXXXX unicode escape mechanisms of JSON.

This branch has a problem with code coverage as there is one method which has a untestable case (line 2215).
This code is only reachable if

  1. there is a bug somewhere else that calls the function with the wrong argument
  2. we have access to the private method.

So we need a way to directly access the codePointToUTF8 method from the test header to get the line coverage back to 100%.

Any recommendation how we do that in a nice way?

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.39%) when pulling 6105ce5 on Teemperor:unicode_escaping into 0cd2ecf on nlohmann:master.

@nlohmann
Copy link
Owner

Thanks, I'll have a look.

FYI: In the unit tests, you can access private methods - at least when the code is compiled with the Automake files, because -Dprivate=public is used. (I know this is ugly, but it allows better coverage without adjusting the tested code...)

@Teemperor
Copy link
Contributor Author

Sounds good to me, updated the branch!

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling a409ba9 on Teemperor:unicode_escaping into 0cd2ecf on nlohmann:master.

@Teemperor
Copy link
Contributor Author

Just realized I effectively reverted your commit 0cd2ecf about the code style patch. I'll apply the changes to this patch.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling a866a9d on Teemperor:unicode_escaping into 0cd2ecf on nlohmann:master.

nlohmann added a commit that referenced this pull request Jan 11, 2015
@nlohmann nlohmann merged commit 6533b33 into nlohmann:master Jan 11, 2015
nlohmann added a commit that referenced this pull request Jan 11, 2015
- removed IDE leftovers
- adjusted const’ness of some functions
- style guide
@nlohmann
Copy link
Owner

Thanks a lot! This Unicode business is really a pain!

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

Successfully merging this pull request may close these issues.

3 participants