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 the more obscure backslash escape tests #8

Closed
1 of 4 tasks
chrisjsewell opened this issue Mar 31, 2020 · 9 comments · Fixed by #171
Closed
1 of 4 tasks

Fix the more obscure backslash escape tests #8

chrisjsewell opened this issue Mar 31, 2020 · 9 comments · Fixed by #171
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@chrisjsewell
Copy link
Member

chrisjsewell commented Mar 31, 2020

These are the final tests that need to be fixed; one that is directly applicable to CommonMark compliance, and the others are for additional compliance with markdown-it. They are all to do with how \ escapes are treated for some of the more obscure cases in link titles / code fence languages, e.g. [](<\&quot;> "\&amp;\&ouml;")

@chrisjsewell chrisjsewell added the bug Something isn't working label Mar 31, 2020
@chrisjsewell
Copy link
Member Author

Note the portability issue lies in the fact that markdown-it uses the external JS dependencies mdurl and punycode for their functions, which are obviously not directly useable for python.

@chrisjsewell
Copy link
Member Author

For the commonmark spec:

Backslash-escapes do not work inside autolinks:

<http://example.com/\[\>
.
<p><a href="http://example.com/%5C%5B%5C">http://example.com/\[\</a></p>

However, we get:

<p><a href="http://example.com/%5B%5C">http://example.com/\[\</a></p>

@chrisjsewell
Copy link
Member Author

@hukkinj1 and @tsutsu3, given your recent work, perhaps you have an idea of how to solve these?
It doesn't seem that anyone has really come across issues with these, but it would be nice to solve if possible before a v1.0.0 release

@hukkin
Copy link
Member

hukkin commented Dec 15, 2020

👍 not picking this up right now, but I'll leave a comment here if I do later. Do these tests exist in JS markdown-it? Without the skips? If yes, then the JS tests could be useful for debugging.

@chrisjsewell
Copy link
Member Author

Yep the test fixtures are copied verbatim from the markdown-it repo: tests/test_port/fixtures, which are all formatted as inputs and expected outputs.

You "just" need to remove the skips and fix the code so they pass, without breaking any of the other tests.

I've tried before, but gave up after it felt like I was going round in circles lol: fixing one test but making another fail

@tsutsu3
Copy link
Contributor

tsutsu3 commented Dec 15, 2020

I tried it briefly before, but it didn't work. It was harder than I thought it would be. 😭

@chrisjsewell
Copy link
Member Author

It was harder than I thought it would be

same here!

@hukkin
Copy link
Member

hukkin commented Dec 16, 2020

I was able to remove one of these skips here #97 admittedly without knowing what I was doing 😄 Hoping that I can trust our tests that I didnt break anything else...

@chrisjsewell
Copy link
Member Author

thanks! yeh the tests are quite comprehensive, including the full commonmark specification.
But obviously if you can think of any other test cases, feel free to add them

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
Development

Successfully merging a pull request may close this issue.

3 participants