add option to not parse beyond end of structure #435
Conversation
|
this is fine, just needs a test or two to exercise it |
| if cfg.DontParseBeyondEnd { | ||
| return nil | ||
| } |
There was a problem hiding this comment.
@petar IIUC this is perhaps a nice config option, but only slightly helps fix the problem you're likely currently worrying about (i.e. streaming decodes of multiple dag-json objects for Reframe using Edelweiss).
Since the specific application format (in this case Reframe's HTTP+dag-json transport) has its own way of dealing with concatenating dag-json blobs (in this case appending \n) you're going to need some custom code anyhow to parse the \n.
At that point is it so different from just using Unmarshal(na, json.NewDecoder(r), cfg) directly and then trying to slurp up one more \n before continuing instead of using Decode?
There was a problem hiding this comment.
The issue with this proposal is that json.NewDecoder won't decode bytes and other IPLD-specific objects. Right?
The solution to Edelweiss' problem could be composed of two steps:
- this PR, which makes sure nothing is slurped after the JSON object, and
- a manual reader.ReadChar, following every invocation of ipld.StreamingUnmarshal(dagjson.Decoder, ...) in the Edelweiss-generated code for reading multiple
\n-separated results.
There was a problem hiding this comment.
I just wanted to flag more concretely that this PR just allows for using:
DecodeOptions{ ParseLinks : true, ParseBytes: true, DontParseBeyondEnd : true }.Decode(na, r)
rather than:
Unmarshal(na, json.NewDecoder(r), DecodeOptions{ ParseLinks : true, ParseBytes: true }).
Is this config mostly about helping discoverability so people know how to do this (i.e. is the Unmarshal code path not obvious enough)?
There was a problem hiding this comment.
It turns out that Unmarshal is deprecated.
|
@rvagg should be good now. |
Resolves #374.