New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tests to ensure parser requires whitespaces #379

Open
Keats opened this Issue Jan 17, 2019 · 1 comment

Comments

1 participant
@Keats
Copy link
Owner

Keats commented Jan 17, 2019

Seen on pest gitter:

Armin Ronacher
@mitsuhiko
Jan 10 17:12
is there something like a word boundary thing I can use in pest?
foo { "partial" ~ identifier } also matches on "partialfoo" instead of what i want: "partial foo"
(i thought i would steal from tera's grammar for the solution but even tera parses {% foritem in seq %} as an alias for {% for item in seq %})

.... 

Dragoș Tiselice
@dragostis
Jan 10 22:01
@mitsuhiko, WHITESPACE needs to be present in such cases, i.e. "partial" ~ WHITESPACE ~ identifier. This is because ~ considers WHITESPACE as completely optional. #333 will make this more flexible in that you will be able to force ~ to contain at least one whitespace.

...

yeah. i get that
my issue was actually that when i switched to ${} i forgot to add ! to inner expressions
that was not entirely obvious
ident = !{ XID_START ~ XID_CONTINUE* } partial_block = ${ "partial" ~ WHITESPACE ~ ident ~ WHITESPACE? ~ object_def_body } object_def_body = !{ "{" ~ fields ~ "}" }

Full discussion is https://gitter.im/pest-parser/pest?at=5c376ef8317e2407cd0e3651
In short it means that {%- ifname %} will be considered valid

This is not intended and should be fixed on the v1 branch.
The first step is to have failing tests for all this kind of missing whitespace issue and then fix the grammar. It could be a nice occasion to clean up the grammar at the same time, there are enough tests that it should be fairly safe to do big changes.

@Keats

This comment has been minimized.

Copy link
Owner

Keats commented Jan 25, 2019

Here is the related test:

// https://github.com/Keats/tera/issues/379
#[test]
fn lex_requires_whitespace_between_things() {
    let inputs = vec![
        "{% filterupper %}hey{% endfilter %}",
        "{% fora in b %}{{a}}{% endfor %}",
        "{% for a inb %}{{a}}{% endfor %}",
        "{% ifi18n %}世界{% else %}world{% endif %}",
        "{% ifi18n %}世界{% eliftrue %}world{% endif %}",
        "{% blockhey %}{%endblock%}",
        "{% macrohey() %}{%endmacro%}",
        "{% setident = 1 %}",
        "{% set_globalident = 1 %}",
        "{% extends'base.html' %}",
        "{% import 'macros/image.html' asimage %}",
        "{% import'macros/image.html' as image %}",
    ];

    for i in inputs {
        let res = TeraParser::parse(Rule::template, i);
        println!("{:?}", i);
        assert!(res.is_err());
    }
}

if someone wants to get started on that.

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