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

fix: allow whitespace before closing ">" #147

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sfermigier
Copy link

Don't throw up on correct HTML.

@OmerFI
Copy link

OmerFI commented Jan 10, 2023

Normally, following HTML code is not correct:

image

curlylint right now fails (I think this is the expected behaviour):

image

If your pull request is applied, will the HTML code above pass the check?

@sfermigier
Copy link
Author

I believe whitespace before > is correct. xmllint confirms.

 ~/tmp> cat toto.xml
<toto
>
</toto
>
 ~/tmp> xmllint toto.xml
<?xml version="1.0"?>
<toto>
</toto>
# Or
 ~/tmp> cat toto.xml
<toto >
</toto >
 ~/tmp> xmllint toto.xml
<?xml version="1.0"?>
<toto>
</toto>

@sfermigier
Copy link
Author

sfermigier commented Jan 11, 2023

See also the XML grammar:

https://www.w3.org/TR/xml/#NT-ETag

(S? stands for "whitespace" in the production rule).

@OmerFI
Copy link

OmerFI commented Jan 11, 2023

That makes sense!

make_opening_tag_parser function already skips whitespaces:

return locate(
    P.seq(
        P.string("<"),
        tag_name_parser,
        attributes.skip(whitespace),
        slash,
        P.string(">"),
    )
).combine(_combine_opening_tag)

And the make_closing_tag_parser function should skip whitespaces too!

@OmerFI
Copy link

OmerFI commented Apr 18, 2023

@thibaudcolas Can you review the pr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants