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

Dynamic class name #23

Closed
Tobbe opened this issue May 15, 2018 · 15 comments
Closed

Dynamic class name #23

Tobbe opened this issue May 15, 2018 · 15 comments

Comments

@Tobbe
Copy link

Tobbe commented May 15, 2018

Is it possible to have a generic container that applies a dynamic class name?

Kind of like the info-string for fenced code blocks. I.e. if you write ```java you get a <code> with the class name language-java, but if you write ```ruby the class name will be language-ruby.

In a similar manner I would like to be able to do ::: foo and get <div class="foo"> and ::: bar and get <div class="bar">. Is this possible? It's important that all use the same opening/closing sequence (:::) and that I don't have to write anything more than just those three colons and the classname.

@Tobbe
Copy link
Author

Tobbe commented May 15, 2018

I think I solved it myself. I wrote a custom render function, like this:

md.use(markdownItContainer, 'dynamic', {
    validate: function() { return true; },
    render: function(tokens, idx) {
        var token = tokens[idx];

        if (token.nesting === 1) {
            return '<div class="' + token.info.trim() + '">';
        } else {
            return '</div>';
        }
    },
});

Do you see any problems with an implementation like that?

Usage:

Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vivamus metus tortor, dignissim a libero sed, rhoncus feugiat neque.

::: foo
In fermentum felis non dapibus hendrerit. Proin et urna viverra, tempor purus a, accumsan nulla.
:::

::: bar baz
Vestibulum ornare eros nisl, in facilisis ante tempor non. Vestibulum sit amet arcu ultricies, sollicitudin urna ut, facilisis enim.
:::

Mauris non efficitur justo, quis consectetur risus. Duis eget pellentesque enim. Integer sagittis, turpis vel semper tincidunt, nunc ante dapibus nibh, sit amet ullamcorper ex metus vitae quam. Aliquam id libero in nulla commodo porttitor sed varius diam. 

Output:

<p>Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vivamus metus tortor, dignissim a libero sed, rhoncus feugiat neque.</p>

<div class="foo">
<p>In fermentum felis non dapibus hendrerit. Proin et urna viverra, tempor purus a, accumsan nulla.</p>
</div>

<div class="bar baz">
<p>Vestibulum ornare eros nisl, in facilisis ante tempor non. Vestibulum sit amet arcu ultricies, sollicitudin urna ut, facilisis enim.</p>
</div>

<p>Mauris non efficitur justo, quis consectetur risus. Duis eget pellentesque enim. Integer sagittis, turpis vel semper tincidunt, nunc ante dapibus nibh, sit amet ullamcorper ex metus vitae quam. Aliquam id libero in nulla commodo porttitor sed varius diam. </p>

@puzrin
Copy link
Member

puzrin commented May 15, 2018

Yes, your renderer modifications is ok. You can also skip validate property if you don't need it.

@puzrin puzrin closed this as completed May 15, 2018
@mb21
Copy link

mb21 commented Oct 12, 2018

Actually, you need the validate function. Otherwise it will work only with ::: dynamic.

I think the behaviour described in this issue would be a great default. It's also how pandoc behaves by default now. So this:

::: warning
*here be dragons*
:::

would work with just:

var md = require('markdown-it')().use( require('markdown-it-container') );

instead of the current:

var md = require('markdown-it')().use( require('markdown-it-container'), 'warning' );

@puzrin
Copy link
Member

puzrin commented Oct 12, 2018

I don't agree. You mix params (infostring) and container (generated tag) type. Those are ortogonal (independent). Type is mandatory.

@mb21
Copy link

mb21 commented Oct 12, 2018

Not sure I understood that. I'm suggesting that if no container name is provided to the plugin, this:

::: x
content
:::

is converted to:

<div class="x">
content
</div>

for any string x.

Are you saying, that you don't agree that this is a sensible default?

But you agree that it is pandoc's default behaviour?

@puzrin
Copy link
Member

puzrin commented Oct 12, 2018

My comment was about

var md = require('markdown-it')().use( require('markdown-it-container') );

instead of

var md = require('markdown-it')().use( require('markdown-it-container'), 'warning' );

You can set last param any name you like, but it should not be missed.

@mb21
Copy link

mb21 commented Oct 12, 2018

But what if I want my users to be able to use any class name? Which is in fact what pandoc users can do.

@mb21
Copy link

mb21 commented Oct 12, 2018

If you're saying that's a bad default, maybe this could be done by explicitly setting an option, like:

.use( require('markdown-it-container'), 'div', {
  anyClass: true
})

@puzrin
Copy link
Member

puzrin commented Oct 12, 2018

I think, you misunderstand the intent of this module. Intent is to be universal (modifyable), for any use case. But example shows only a small subset.

If you wish to have default class - just modify renderer to add it, when infostring is empty.

@mb21
Copy link

mb21 commented Oct 12, 2018

I'm not sure whether we're talking past each other here. I don't want to have a default class. I want what's pandoc's behaviour, which is the same what the OP of this issue achieved.

If the only way to do this is with a custom render function, that's fine. I just thought it would be nice to do this without that somewhat ugly workaround.

@puzrin
Copy link
Member

puzrin commented Oct 12, 2018

For example, in nodeca we use 2 different container types: quote & spoiler. Those have different layouts, validators and so on. And default renderer will not help anyhow.

infobox is just a one of many use cases. If we start to modify API for single use case, that will be not very good IMHO.

Anyway, you are free to create a new issue, with focus on pandoc similarity, and we can discuss your arguments more detailed. But note, i'm very conservative about extending API, and will ask valuable reasons.

@mb21
Copy link

mb21 commented Oct 12, 2018

I see that you consider the default use-case a different one, makes sense as well. Don't think it's worth arguing about defaults. Everyone has a different opinion ;-) Coming from pandoc, I just was expecting something else. That's why I suggested changing it if name is not set (or maybe set to boolean false), it could do the pandoc behaviour. But I'm actually fine with using the above code snippet. Just thought I'd give some feedback. Thanks again for markdown-it!

@puzrin
Copy link
Member

puzrin commented Oct 12, 2018

No problem. Just consider this module not as final ready-to-use solution. It helps you to solve very complicated task - how to properly parse block with embedded content. Everything else should be tuned, because very opinion-based.

@cekvenich
Copy link

Tobbe : thanks, I'm using your code at metabake.org for nested css blocks

@flaviotordini
Copy link

Thanks @Tobbe for the code! It should have been the default behaviour.

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

No branches or pull requests

5 participants