Add load_into_document to simdjson::dom::parser
#2103
Conversation
|
Running tests... |
| @@ -452,14 +452,18 @@ namespace parse_api_tests { | |||
| element doc_root1 = parser.parse_into_document(doc, input); | |||
| if(simdjson::to_string(doc_root1) != "[1,2,3]") { return false; } | |||
| //... doc_root1 is a pointer inside doc | |||
| element doc_root2 = parser.parse_into_document(doc, input); | |||
| element doc_root2 = parser.load_into_document(doc, TWITTER_JSON); | |||
There was a problem hiding this comment.
Have you thought about not replacing the existing test, but rather adding a test? I am mildly concerned that your PR reduces our testing of parse_into_document.
This is not a request for change. I'd like to have your thoughts on this.
There was a problem hiding this comment.
I guess it maybe depends if combinations matter. I could copy-paste the original test and make it load_from_document but do we want to cover scenarios of alternating between parse_into_document and load_into_document with the same parser?
I have no strong opinions either way. There's still two parse_into_document calls on the same parser in this test before and after, but it's not two-in-a-row now so I acknowledge there's perhaps a created gap there depending on how you see it.
There was a problem hiding this comment.
@Bo98 I leave this up to your judgment. I am willing to merge this as-is.
There was a problem hiding this comment.
This is probably fine I reckon
|
Merged. |
I've found
parse_into_documentquite useful, but there was previously noloadcounterpart toparse_into_document. It seemed simple enough to add so this PR does exactly that.