Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hey :)
I did another PR focusing on execution speed and Errors.
JsonPathParserError
error crate with more custom things to get rid of the plain-String errors. This way its easier for libraries to handle errors and catch them.json_path_instance
returned aBox<dyn>
which means that the type is determined on runtime, and this is compiled via a vtable. As all posibilities are known i just created a simple enum containing those and implementing thePath
trait. This allows the compiler to further optimize the crate.There are a few drawbacks of this change we should consider:
Pairs
in Error directly, because the lifetimes made problems withTryFrom
trait, so for now its just the String representation of thePairs
str_to_bool
function can never fail, IMO the path is onlyPath::boolean
if the str is eithertrue
orfalse
as described in the .pest file, but I have never worked with that parse, is this assumption true?parse_logic_or
andparse_logic_and
functions, i did a check on the len of that iterator. IMO as soon as there is 1 entry, the result cannot beNone
is this correct ? Otherwise I would have introduced a logical bug, but this way we only prepare the Error message on demand`.So this change needs some good code-review before we just merge it, i don't want to introduce some logic bugs.
On the other hand, the compilation time for a
JsonPathInst
is quite a bit faster now, from over 70 us to under 15us.I was also thinking about
JsonPath
andJsonPathInst
, maybe its possible to just use theJsonPath
in the future, and also move the 3find
methods into this struct. This would definitly be something for another PR, but tell me what you think about it ? Because this PR contains some breaking changes, so we should prob wait with a new release for a bit after merging this.