Skip to content
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

dynamic attributes cause parse error #23

Open
davidszotten opened this issue Sep 7, 2020 · 3 comments
Open

dynamic attributes cause parse error #23

davidszotten opened this issue Sep 7, 2020 · 3 comments
Labels
bug Something isn't working parser This issue relates to Curlylint’s templates parser

Comments

@davidszotten
Copy link

Describe the bug

template:

<div {{ name }}="value"></div>
0:15      Parse error: expected one of '>', '\\s+', '{#', '{%', '{{' at 0:15      parse_error

This seems like a reasonable (albeit slightly unusual) template.

Which terms did you search for in the documentation and issue tracker?

looked at all open and closed issues

(Write your answer here if relevant.)

Environment

$ curlylint --version
curlylint, version 0.12.0
@davidszotten davidszotten added the bug Something isn't working label Sep 7, 2020
@thibaudcolas
Copy link
Owner

Hey @davidszotten, thank you for the detailed report! That’s definitely a reasonable template, and a case that the parser currently doesn’t support. I think the logic for "template syntax in opening tags" simply needs implementing.

If you or anyone else is interested in helping with this – well, this might make for a tough first contribution, but as a starter I’d love to have unit tests written for this. The parser is in need of much better tests, so we can make additions like this without risking regressions.

Here is the code for future reference:

attr_name_start_char = P.regex(r"[:a-zA-Z]")
attr_name_char = attr_name_start_char | P.regex(r"[0-9A-Z-_.]")
attr_name = attr_name_start_char + attr_name_char.many().concat()
def make_attribute_parser(jinja):
attribute_value = make_attribute_value_parser(jinja)
return (
locate(
P.seq(
interpolated(attr_name),
whitespace.then(
P.seq(
P.string("=").skip(whitespace).tag("equal"),
interpolated(attribute_value).tag("value"),
).map(dict)
).optional(),
)
)
.combine(_combine_attribute)
.desc("attribute")
)
def make_attributes_parser(config, jinja):
attribute = make_attribute_parser(jinja)
jinja_attr = make_jinja_parser(
config,
interpolated(
whitespace.then((attribute | jinja).sep_by(whitespace)).skip(
whitespace
)
),
)
attrs = interpolated(
(
whitespace.then(jinja_attr) | mandatory_whitespace.then(attribute)
).many()
)
return attrs

@thibaudcolas thibaudcolas added the parser This issue relates to Curlylint’s templates parser label Sep 8, 2020
@colmjude
Copy link

colmjude commented Sep 29, 2020

Another example of a parse_error for reference.

Template

<a class="govuk-button"href="/cookies">Set cookie preferences</a>
0:23    Parse error: expected one of '>', '\\s+', '{#', '{%', '{{' at 0:23      parse_error

Throwing an error makes sense, there is a missing space between the attributes but parse_error does seem like the right error and message.

Environment

$ curlylint --version
curlylint, version 0.12.0

@thibaudcolas
Copy link
Owner

👍 good to know about @colmjude. I think that one would be much simpler to address – by the looks of it the problem is that the parser expects attributes to always be separated by whitespace, which as you say feels like a reasonable thing to enforce, but it probably shouldn’t be a (cryptic) parse error:

def make_attribute_parser(jinja):
attribute_value = make_attribute_value_parser(jinja)
return (
locate(
P.seq(
interpolated(attr_name),
whitespace.then(
P.seq(
P.string("=").skip(whitespace).tag("equal"),
interpolated(attribute_value).tag("value"),
).map(dict)
).optional(),

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working parser This issue relates to Curlylint’s templates parser
Projects
None yet
Development

No branches or pull requests

3 participants