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

Renderer HTML escaping #636

Closed

Conversation

FredrikAppelros
Copy link

Move escaping of HTML from InlineLexer to Renderer. This will
modularize marked further and allow using it with custom renderers that
do not intend to output HTML.

Move escaping of HTML from `InlineLexer` to `Renderer`. This will
modularize marked further and allow using it with custom renderers that
do not intend to output HTML.
@FredrikAppelros
Copy link
Author

This indirectly addresses the issues #269, #529 and #625. More importantly, it makes marked a little bit more modular. You can now use the parser and lexer components with a different renderer that doesn't output HTML.

For the fun of it I also made a small branch which directly fixes the issues mentioned above here, but I personally don't understand why you would want to disable the escaping if it is to be rendered in a browser anyway. 😕

@adam-lynch
Copy link

👍

This should be probably also contain changes to the readme, right?

A reason why someone mightn't like it: users overriding renderer methods will inadvertently lose escaping. Why this doesn't matter: this is already the case when overriding link rendering; https://github.com/chjj/marked/blob/18fb6a639a4a77dd59650879bcad10d833c40067/lib/marked.js#L872.

I personally don't understand why you would want to disable the escaping if it is to be rendered in a browser anyway.

It mightn't be rendered in a browser. We need it for desktop notifications, etc. as I said in #529.

@FredrikAppelros
Copy link
Author

Yes, this change will affect existing custom renderers that relies on the escaping being done within the lexer. Not sure if it really requires any changes to the README as there was no mention of it being escaped in the first place.

@diega
Copy link

diega commented Apr 6, 2016

Hi!
Is there any chance for this to be reviewed and/or merged?

Regards

@mercmobily
Copy link

+1 Please. It has been more than a year...

@joshbruce
Copy link
Member

See #746

We are planning some changes eventually to the Marked library architecture. Adding reference to that issue. Closing due to merge conflicts, stale, and missing tests.

@joshbruce joshbruce closed this Jan 21, 2018
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.

5 participants