spatialaudio / nbsphinx

:ledger: Sphinx source parser for Jupyter notebooks
https://nbsphinx.readthedocs.io/
MIT License
457 stars 130 forks source link

fix: avoid closing the script tag early by escaping a forward slash #606

Closed maartenbreddels closed 2 years ago

maartenbreddels commented 3 years ago

Same as https://github.com/jupyter/nbconvert/pull/1665

Requires a new release of nbconvert as well

pep8speaks commented 3 years ago

Hello @maartenbreddels! Thanks for opening this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 287:80: E501 line too long (108 > 79 characters)

mgeier commented 2 years ago

Thanks for this PR and sorry for my late response!

I played around with this a bit and it turns out that the </script> tag is case-insensitive and that browsers (at least Firefox and Chromium) also have a problem with </script (with a space at the end).

I've created an extended fix in #611 which should cover more cases. Or am I missing something?

maartenbreddels commented 2 years ago

Yeah, even better, care to upstream this?

(from mobile phone)

Op wo 24 nov. 2021 22:25 schreef Matthias Geier @.***>:

Thanks for this PR and sorry for my late response!

I played around with this a bit and it turns out that the tag is case-insensitive and that browsers (at least Firefox and Chromium) also have a problem with </script (with a space at the end).

I've created an extended fix in #611 https://github.com/spatialaudio/nbsphinx/pull/611 which should cover more cases. Or am I missing something?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/spatialaudio/nbsphinx/pull/606#issuecomment-978241722, or unsubscribe https://github.com/notifications/unsubscribe-auth/AANPEPNEYONCUANKMZ5GD2TUNVJ47ANCNFSM5G3ANPJA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

mgeier commented 2 years ago

Yeah, even better, care to upstream this?

I don't really want to undo your change from https://github.com/jupyter/nbconvert/pull/1665, which isn't wrong and works in most real cases. But if you prefer my solution, you can of course take it and apply it (or parts of it) to your original change.