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

Use a HTML-comment to escape code #893

Merged
merged 1 commit into from
May 1, 2016

Conversation

zeitgeist87
Copy link
Collaborator

If an element does not contain any code, but a single HTML-comment, then the comment is used as code.

<code class="language-markup"><!-- <p>Example</p> --><code>

It works very well, but I feel silly for even proposing this. It seems semantically wrong to abuse a comment like this. I created this to contribute to the discussion around #887.

@LeaVerou
Copy link
Member

No matter what we do for this, it will be a hack. Which is ok, as long as we keep it out of the core and in the unescaped markup plugin. Wanna move the commit to that? :)

@LeaVerou
Copy link
Member

Just please amend the documentation to mention this, and make sure to only apply it for block code (it's much more likely that inline code will actually contain a single comment, for block code I think it's kinda rare).
Nice hack!

@zeitgeist87 zeitgeist87 force-pushed the comment-escape branch 2 times, most recently from d04fb66 to 95fb2b0 Compare February 15, 2016 22:19
@zeitgeist87
Copy link
Collaborator Author

I've updated the PR. Unfortunately some small changes to prism-core.js are still necessary. I had to move the check for empty code below the before-highlight hook.

@LeaVerou
Copy link
Member

I’m a bit worried that this could be a breaking change for third party plugins. Perhaps we should add a new hook before the check?

@zeitgeist87
Copy link
Collaborator Author

I’m a bit worried that this could be a breaking change for third party plugins.

I think the chances of that happening are quite low, because env.code contains an empty string at that point and not something unexpected like undefined or null. Any plugin should be able to handle that. It seems to me, the if-statement is mainly a performance improvement that allows Prism to return early if there is nothing to highlight. In my opinion there is no reason to add another hook.

@zeitgeist87
Copy link
Collaborator Author

On the other hand, a new hook would guarantee, that the unescape-markup plugin runs before any other plugin that needs to change env.code.

@zeitgeist87
Copy link
Collaborator Author

@LeaVerou I've resolved the merge conflicts for this PR. Do you think it is ready to be merged?

@LeaVerou LeaVerou merged commit f40aff3 into PrismJS:gh-pages May 1, 2016
@LeaVerou
Copy link
Member

LeaVerou commented May 1, 2016

Sure, ok! Thanks!

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