Skip to content

parser: use LocatingSlice<&str> instead of &str#560

Merged
GuillaumeGomez merged 4 commits intoaskama-rs:masterfrom
Kijewski:pr-inputstream
Aug 8, 2025
Merged

parser: use LocatingSlice<&str> instead of &str#560
GuillaumeGomez merged 4 commits intoaskama-rs:masterfrom
Kijewski:pr-inputstream

Conversation

@Kijewski
Copy link
Copy Markdown
Member

@Kijewski Kijewski commented Aug 7, 2025

This will enable better span getting in subsequent PRs.

@Kijewski Kijewski added the parser Related to the parser label Aug 7, 2025
Comment thread askama_parser/src/expr.rs
Comment on lines +1090 to -1067
let three_chars = take(3usize).verify_map(|head: &str| {
if let Ok(head) = head.as_bytes().try_into()
&& THREE_CHARS.contains(head)
{
Some(())
} else {
None
}
});
let two_chars = take(2usize).verify_map(|head: &str| {
if let Ok(head) = head.as_bytes().try_into()
&& TWO_CHARS.contains(head)
{
Some(())
} else {
None
}
});
let one_char = any.verify_map(|head: char| {
if let Ok(head) = head.try_into()
&& ONE_CHAR.contains(&head)
{
Some(())
} else {
None
}
});

// need to check long to short
*i = if let Some((head, tail)) = i.split_at_checked(3)
&& let Ok(head) = head.as_bytes().try_into()
&& THREE_CHARS.contains(head)
{
tail
} else if let Some((head, tail)) = i.split_at_checked(2)
&& let Ok(head) = head.as_bytes().try_into()
&& TWO_CHARS.contains(&head)
{
tail
} else if let Some((head, tail)) = i.split_at_checked(1)
&& let [head] = head.as_bytes()
&& ONE_CHAR.contains(head)
{
tail
} else {
return fail(i);
};
Ok(())
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function had to be replaced for the most part. It's logic is still the same.

Comment thread askama_parser/src/lib.rs

type HashSet<T> = std::collections::hash_set::HashSet<T, FxBuildHasher>;

#[cfg(not(windows))]
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only the Path formatting tests are platform specific.

Comment thread askama_parser/src/node.rs
Comment on lines +40 to -57
i: &mut InputStream<'a>,
s: &State<'_, '_>,
) -> ParseResult<'a, Vec<Box<Self>>> {
let start = *i;
let result = match (|i: &mut _| Self::many(i, s)).parse_next(i) {
Ok(result) => result,
Err(err) => {
if let ErrMode::Backtrack(err) | ErrMode::Cut(err) = &err
&& err.message.is_none()
{
*i = start;
if let Some(mut span) = err.span.as_suffix_of(i) {
opt(|i: &mut _| unexpected_tag(i, s)).parse_next(&mut span)?;
}
}
return Err(err);
}
};
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved the logic of this part into its own function, because cut_node works the same. No need to have the same code twice.

| ^^^^^^^^

error: invalid character
error: cannot have multiple characters in a character literal, use `"..."` to write a string
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this could help users that come from Jinja. Being pythonic, strings can be written with '...'.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea!

@Kijewski
Copy link
Copy Markdown
Member Author

Kijewski commented Aug 7, 2025

With this PR, we can use Parser::[with_]span() to get the byte range of a parsed text. This can replace our manual taking of spans, and Literal::subspan() expects a byte range. I use a Stateful<_, ()> stream, so in a subsequent PR the () can be replace by an reference to the source file, which should become part of askama_parser::Span.

For every derived TokenStream we need to include the sources anew, because Literals are thread local, i.e. !Send + !Sync. But we can still cache the parsed syntax tree.

@GuillaumeGomez, do you want to implement these subsequent PRs to make the underlining work? The feature could even allow debug stepping through a template file!

@GuillaumeGomez
Copy link
Copy Markdown
Collaborator

Thanks a lot for this huge improvement!

@GuillaumeGomez, do you want to implement these subsequent PRs to make the underlining work? The feature could even allow debug stepping through a template file!

I can if you don't have time for it, but seems like you already started quite strongly on it. 😆

@GuillaumeGomez GuillaumeGomez merged commit 756717c into askama-rs:master Aug 8, 2025
42 checks passed
@Kijewski Kijewski deleted the pr-inputstream branch August 8, 2025 12:41
@Kijewski Kijewski mentioned this pull request Aug 6, 2025
@Kijewski
Copy link
Copy Markdown
Member Author

Kijewski commented Aug 8, 2025

Will keep working on it. :)

@GuillaumeGomez
Copy link
Copy Markdown
Collaborator

Gonna work on other stuff then. 😉

Can't wait to see what next step will be!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

parser Related to the parser

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants