Use Parser::[with_]span() to get byte range and use byte range for proc_macro::Span#571
Use Parser::[with_]span() to get byte range and use byte range for proc_macro::Span#571Kijewski merged 6 commits intoaskama-rs:masterfrom
Parser::[with_]span() to get byte range and use byte range for proc_macro::Span#571Conversation
This comment was marked as resolved.
This comment was marked as resolved.
5d3968f to
370214f
Compare
|
I copied the unescape implementation of literal-escaper, because it is (IMHO needlessly) msrv 1.89, and version 0.0.5 does not sound very stable. :) Now, on nightly, the correct subspan in |
389f10d to
b1da00e
Compare
589c19a to
657e26b
Compare
c294554 to
1e38a92
Compare
|
@GuillaumeGomez, I think it's ready to be reviewed. The spans for |
| @@ -0,0 +1,185 @@ | |||
| // The content of this file was copied and adapted from the project [`rustc-literal-escaper`] in | |||
There was a problem hiding this comment.
What is missing from rustc-literal-escaper that you needed to copy some parts of it? If some sub-parts can be useful, I can make them public.
There was a problem hiding this comment.
Nothing was missing, rustc-literal-escaper works fine! The part of rustc-literal-escaper we need is quite small. We copied other small function like path_clean::clean() and pathdiff::diff_paths(), too, instead of adding a dependency.
I don't like the use of NonZero<u8> instead of NonZeroU8, because it needlessly rises the MSRV to 1.89, and the version number 0.0.5 did not instill me with confidence. :) A 0.0.x version is effectively a pinned version, so we would have to release a new Askama version when a new rustc-literal-escaper is released, or downstream users would not get updates.
There was a problem hiding this comment.
More than 100 lines of code seems a bit too much to me but considering how big the PR is, but we can rediscuss it later on, for now I just want this PR to be merged. :)
There was a problem hiding this comment.
Feel free to replace the copied code with a dependency once rustc-literal-escaper 0.1.0 was released! :)
|
Went over the code, that's quite impressive. O.o So at this point, I'm pretty sure I missed most of the important bits. Can you update the first comment to add a high-level overview of the changes here please for posterity? |
|
Thanks! Feel free to merge. |
|
Thank you! :) |
This PR includes the following changes:
&stras a span, we use the byte range in the input string. With this change we don't have to track the lifetime of the input string, so<'a>got removed fromWithSpanandSpan. When tacking a subspan of a Literal, we need the byte range, not the text.LocatingStream::span(). Because of that change, almost every instance ofparser.with_taken()was replaced withparser.with_span().Literal::subspan()is not the bytes in the unescaped string, but in the stored representation. E.g. in"a\u{62}c"thebis not at1..2, but at2..8. We use literal-escaper to manually interpret/unescape the input string, because withLitStr::value()we would not be able to tell"abc","a\u{62}c", andr###"abc"###apart.Stateand/or nestingLevelto recursive calls in the parser, we use winnow'sStatefulStream. This means that most instances of(|i: &mut _| p(i, s)).parse_next(i)?could be replaced withp.parse_next(i)?.