Skip to content

fix: allow whitespace before closing ">"#147

Open
sfermigier wants to merge 1 commit intothibaudcolas:mainfrom
sfermigier:main
Open

fix: allow whitespace before closing ">"#147
sfermigier wants to merge 1 commit intothibaudcolas:mainfrom
sfermigier:main

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.

2 participants