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

catch exceptions for json_pointer : ..../+99 #1990

Merged
merged 4 commits into from
Apr 13, 2020

Conversation

dota17
Copy link
Contributor

@dota17 dota17 commented Mar 17, 2020

From RFC 6901, Section 4:

The ABNF syntax for array indices is:
array-index = %x30 / ( %x31-39 *(%x30-39) )
; "0", or digits without a leading "0"

The array index must not begin with signed char either.
Currently, the path .../+99 works, and it maybe not right.

@dota17 dota17 requested a review from nlohmann as a code owner March 17, 2020 13:00
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 99.714% when pulling bfc003c on dota17:json_pointer into 54656ef on nlohmann:develop.

Copy link
Owner

@nlohmann nlohmann left a comment

Choose a reason for hiding this comment

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

Good catch!

Could you please review the lines that became uncovered (https://coveralls.io/builds/29418441/source?filename=include%2Fnlohmann%2Fdetail%2Fjson_pointer.hpp#L418) and check if you could add a respective test?

@dota17
Copy link
Contributor Author

dota17 commented Mar 20, 2020

@nlohmann
OK, i will add some testcases for this.

@RobBeyer
Copy link

@dota17
Commit is fantastic. Addresses all _json_pointer array_index() & contains() issues raised and cleans up/DRYs code in the process. Cheers!

@dota17
Copy link
Contributor Author

dota17 commented Apr 7, 2020

Sorry, i had been busy for the past few days.
And since the #2009 is fixed, i can get the report of coverall and i will add testcases for the following days.

@dota17
Copy link
Contributor Author

dota17 commented Apr 7, 2020

@nlohmann
please restart those jobs -The job exceeded the maximum time limit for jobs, and has been terminated.

Copy link
Owner

@nlohmann nlohmann left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@nlohmann nlohmann self-assigned this Apr 13, 2020
@nlohmann nlohmann added this to the Release 3.7.4 milestone Apr 13, 2020
@nlohmann nlohmann merged commit b7be613 into nlohmann:develop Apr 13, 2020
@nlohmann
Copy link
Owner

Thanks!

@nlohmann
Copy link
Owner


🔖 Release item

This issue/PR will be part of the next release of the library. This template helps preparing the release notes.

Type

  • ✨ New Feature
  • 🐛 Bug Fix
  • ⚡️ Improvement
  • 🔨 Further Change
  • 🔥 Deprecated function

Description


@dota17 dota17 deleted the json_pointer branch April 14, 2020 01:53
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.

4 participants