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

Add an option to disable markdown elements #660

Closed
wants to merge 7 commits into from
Closed

Add an option to disable markdown elements #660

wants to merge 7 commits into from

Conversation

Guichaguri
Copy link

A simple change that let you disable any markdown element.
It's better creating an option than using unsafe tricks and hacks

Examples

marked.setOptions({
    disabledElements: ['hr', 'heading', 'list']
});

marked('---'); // Returns "---"
marked('# I am not a heading'); // Returns "# I am not a heading"
marked('* Item'); // Returns "* Item"

A simple change that let you disable any markdown element
@Guichaguri
Copy link
Author

I just found #579.

Differences

  • This one works with GFM.
  • It's an option instead of a new function.
  • Can be applied to a single marked call (marked('---', { disableElements: ['hr'] }))

@Guichaguri Guichaguri closed this Sep 9, 2015
@Guichaguri Guichaguri reopened this Sep 9, 2015
szego and others added 4 commits October 13, 2015 07:35
Previously the script only checked for the disabledElements option in
the Lexer. Now it checks for it in the Lexer and the inlineLexer.
The script now checks for disabledElements in the Render and disables
the appropriate functions.
Disable elements in InlineLexer as well
@Guichaguri
Copy link
Author

Fixed merge conflicts

@joshbruce
Copy link
Member

Thanks for the PR @Guichaguri! Much appreciated. Can you write the tests demonstrating the examples? (Note: I am wondering about the flexibility of our testing harness to actually test all the things we need it to.)

@Guichaguri
Copy link
Author

How should I write it? After a quick look through the test code, it seems like it can only test with predefined options.

@joshbruce
Copy link
Member

@Guichaguri: Let me tag in @UziTech, @matt-, and @Feder1co5oave to help on possibly getting a test written on this.

@UziTech
Copy link
Member

UziTech commented Jan 5, 2018

Ya, the tests aren't very option friendly at the moment. I am working on redoing the tests to allow options to be set with front-matter

stay tuned...

@Feder1co5oave
Copy link
Contributor

Feder1co5oave commented Jan 5, 2018 via email

@UziTech
Copy link
Member

UziTech commented Jan 6, 2018

I submitted pr #1002 to fix the tests and allow options to be specified with front-matter

@Feder1co5oave it looks like you can re-enable all elements by setting the disabledElements option to an empty array (or removing the elements that you want to re-enable)

@Guichaguri
Copy link
Author

Once the test PR gets merged, I'll write tests for all cases, including re-enabling disabled elements.

I should also write documentation for it.

@joshbruce
Copy link
Member

@Guichaguri: Can you also handle @Feder1co5oave's question. I'm not sure I would choose the word "destructive" just because of connotation...maybe "global" - the entire markdown string being parsed??

Which I think is true, yeah?

It would be interesting if it could be open to extension a little bit...like "disable the frist 5 instances of H1" or "disable the first, second, and fifth H1". Having said that, given the developer requests to date, being able to stop Marked from rendering an element in a markdown is the request...not granular control.

Am I missing something in @Feder1co5oave's question.

@Feder1co5oave
Copy link
Contributor

That's what I'm talking about:

> marked('# heading?')
'<h1 id="heading-">heading?</h1>\n'
> marked('# heading?', {disabledElements: ['heading']})
'<p># heading?</p>\n'
> marked('# heading?')
'<p># heading?</p>\n'
> marked('# heading?', {disabledElements: []})
'<p># heading?</p>\n'

@UziTech
Copy link
Member

UziTech commented Jan 7, 2018

I was wrong, I didn't realize the rules were overwritten in the lexer. @Guichaguri maybe a better approach would be to copy the rules to this.rules only if they are not in this.options.disabledElements otherwise set the rule as noop. That way the static rule sets are left alone.

Also should it be called disabledRules instead of disabledElements. I feel like elements refer to tags like h1, h2, etc.

@joshbruce joshbruce added this to the 0.5.0 - Architecture and extensibility milestone Jan 7, 2018
@joshbruce
Copy link
Member

My two cents would be that, yes, "elements" are individual elements (skip h1) while "rules" are all the types (skip headings). Can we have the ability to specify both?

@joshbruce joshbruce removed this from the 0.5.0 - Architecture and extensibility milestone Apr 4, 2018
@joshbruce
Copy link
Member

Closing due to merge conflicts and lack of tests. Should be easy enough to resurrect or continue the spirit of in 1.x or 2.x.

@joshbruce joshbruce closed this Apr 5, 2018
@sungwoncho
Copy link

What can do to help this feature get merged to master?

@joshbruce
Copy link
Member

@sungwoncho: Hello! Right now our focus is on complying with our chosen specifications and that's where we need the most help. This feature, if approved, would not be put in until after 1.0 or later. Therefore, from an over-arching project perspective, if you are still interested in moving toward at least the possibility of having this feature include sooner rather than later, then I recommend helping us with the specification compliance mission of 0.x: #1216

Up to you! Thanks again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants