Rework JuliaSyntax.parse() public API
#107
Conversation
`parse()` and `parseall()` were generally pretty inconvenient to use. This change reworks what I had called `parseall()` to be more similar to `Meta.parse()` and adds `parseall()` and `parseatom()` in analogy to the `Base.Meta` versions of these functions. The lower level function `parse!()` is provided to work with `ParseStream` for cases where more control is required.
|
I'm keen for any reviews on this. @pfitzseb if you have time? :-) |
pfitzseb
left a comment
pfitzseb
left a comment
There was a problem hiding this comment.
Looks good to me. Do you think it's worth adding a 1-arg JuliaSyntax.parse(text) = JuliaSyntax.parse(Expr, text) convenience functions?
src/parser_api.jl
Outdated
| `(tree, diagnostics)`. The `io` position will be set to the next byte of input | ||
| after parsing. |
There was a problem hiding this comment.
"to the first unparsed byte of input" or something like that (is that even correct when the whole io content is consumed?)? "next byte" without any context could be kinda confusing.
There was a problem hiding this comment.
ah this is so hard to word in a way which is clear and correct. How about
"When parse! returns, the stream io is positioned directly after the last byte which was consumed during parsing."
src/parse_stream.jl
Outdated
| ParseStream(ptr::Ptr{UInt8}, len::Integer, index::Integer=1; version=VERSION) | ||
| Construct a `ParseStream` from source `text` which may come in various forms - | ||
| a string, an `IO` object, or a buffer of bytes. In the case that the buffer is |
There was a problem hiding this comment.
I don't think we have an official IO interface. Do you think it'd make sense to list the requirements somewhere (in particular, seekability comes to mind)?
There was a problem hiding this comment.
Yes it makes sense. I thought I'd done this, but it's in the docs for parse!() I'll put it here too
Co-authored-by: Sebastian Pfitzner <pfitzseb@gmail.com>
|
Thanks for taking a look!
I didn't do this because I'm not sure what the right default is. |
Co-authored-by: Sebastian Pfitzner <pfitzseb@gmail.com>
|
I did a bunch of doc cleanups and a couple of minor tweaks. Should be good to go I guess. |
2 participants
JuliaSyntax.parse()andJuliaSyntax.parseall()were generally pretty inconvenient to use.This change reworks what I had called
parseall()to be more similar toMeta.parse()and addsparseall()andparseatom()in analogy to theBase.Metaversions of these functions.The lower level function
parse!()is provided to work withParseStreamfor cases where more control is required.