micromark / micromark-extension-directive

micromark extension to support generic directives (`:cite[smith04]`)
https://unifiedjs.com
MIT License
29 stars 16 forks source link

feature request: ability to nest directives #8

Open yocontra opened 3 years ago

yocontra commented 3 years ago
šŸ‘‰ Note: for people landing here: it is possible to nest directives, but nested directives have to use more colons (this issue is about a different way to do it!)

Subject of the feature

Something like this:

:::box{flex}
  :::box
    ### Test h3
    Test content
  :::

  ::embed{url="http://example.com"}
:::

Problem

Currently container directives can't live inside of other container directives - the parser will close the parent container when it sees the first :::, and continue rendering the rest of the content as if it was not in the container.

Expected behavior

The parser should recognize when containers are nested and wait until the parent container is closed, instead of closing when it sees a child container close.

wooorm commented 3 years ago

Hi there!

You canā€™t do that with code (```) either. But you can put code in code by using more ticks:

````markdown
```js
console.log(1)

As directives are somewhat underspecified, but it is the one ralying cry folks are going for in markdown + extension land, I decided to stick as close to other markdown features as possible, and use the same mechanism: use more colons outside than inside.

Does this do the trick for you? Or would you still push for indentation?

yocontra commented 3 years ago

@wooorm Sorry I should have specified in more detail - I see in the readme that you can add additional colons, the "feature" would be instead of doing that the parser keeps track of what is open and closed, allowing you to nest containers without having to manually balance the colons. As you're walking through the AST and see a close, you close the last opened tag instead of the current behavior which is to close the top parent container.

wooorm commented 3 years ago

Ahh. Interesting. I guess for directives the names are required but for fenced code they aren't. So that might work indeed! šŸ¤”

Closing may be hard though. I guess xml/html use that explicit name too there to prevent errors. I mean, taking your idea to 10 levels of nesting, it's going to be pretty hard to keep track of what's open/closed...

yocontra commented 3 years ago

@wooorm Not using any indentation or whitespace just for clarity on how this is basically the same as a stack in bytecode:

:::box
One
:::box
Two
:::box
Three
:::box
Four
:::
:::
:::
:::

Each :::name is pushing a new "open" container to the stack, and each ::: is popping one off the stack (and closing it). Implementation could basically be as simple as an array open = [].

Assuming box renders a plain div, results in:

<div>
One
<div>
Two
<div>
Three
<div>
Four
</div>
</div>
</div>
</div>

I can't think of any cases where this wouldn't work - it isn't the most readable (and where HTML/XML does better) but if people use tabs it really looks pretty decent. The current method of nesting is really cumbersome and less readable IMO, every time you add a new child you need to add another : to every parent above it and in all of the closing tags.

wooorm commented 3 years ago

but if people use tabs it really looks pretty decent

Downside there, which is a different point, is that a tab will result in indented code? šŸ˜¬

I can't think of any cases where this wouldn't work

Some proposals allow the same number of closing colons, with arbitrary characers (hidden stuff) after them. So there the One box would be closed by L3.

every time you add a new child you need to add another : to every parent above it and in all of the closing tags.

Yeah, this is indeed annoying, and it might be something to share at the CM proposal thread. But: why do you need so many levels of nesting šŸ˜…

yocontra commented 3 years ago

@wooorm We have a box directive which is used to make layouts (:::box{column}, :::box{w="30%"}, :::box{align="center"} and so on) - realistically you would never be more than 4 levels deep in a normal layout, but even with just one level adding a child requires updating the parent which makes it hard to create the UI for this and also hard for people who are manually writing it.

I think it would be an OK solution if you only needed to add an additional semicolon to the child instead of modifying the parent every time you add a child, but as you said it is really a discussion for the spec. I guess the purpose of this ticket is not so much to figure out the spec (which as you said earlier is not really anything final yet) but offer more options for parsing, possibly a flag to turn on this behavior.

wooorm commented 3 years ago

While I see that changing the outer container when adding an inner container is not great, Iā€™m not interested personally on adding something that a) differs from the spec, b) will be very hard to implements. I could be persuaded to allow it under a flag, but that still leaves b).

Perhaps easier to implement and acceptable for your problem would be the inverse of the current behavior. I think thatā€™s what you meant with ā€œI think it would be an OK solution if you only needed to add an additional semicolon to the child instead of modifying the parent every time you add a childā€, right?:

:::outer
::::med
:::::inner
:::::
::::
:::
yocontra commented 3 years ago

@wooorm Yep that is what I meant - I can send a PR for either (or both), I think both should be relatively easy to implement. Just let me know what you think and I can put it on my TODO.

wooorm commented 3 years ago

Sweet! Let me know how youā€™re faring and any Qs you have on micromark extensions. I havenā€™t gotten around to documenting all the inner workings and extension points. There are some inner not-so-pretty things in extensions that I didnā€™t envision when making micromark last fall, that Iā€™d hope to clean in a couple months. Just an FYI!

wooorm commented 3 years ago

Any luck or insights?

yocontra commented 3 years ago

@wooorm Got hit with a big rush at work and haven't had time to work on it - will update when I do

Madd0g commented 2 years ago

is the current best workaround to have more colons in the parent?

for me this parses as expected, but if the parent uses ::: instead of 4 it breaks:

::::parent

:::child
::inner[1]
:::

:::child
::inner[2]
:::

::::
wooorm commented 2 years ago

I wouldnā€™t call it ā€œbest workaroundā€, because thatā€™s exactly how fenced code also works. Itā€™s intended behavior

Madd0g commented 2 years ago

I saw your first comment but hadn't realized it works with directives too. I saw someone's directives example have like 6 colons, tried putting more than 3 and it worked. I thought it only supported 1-3.

Happy to hear it's the correct way of doing it. Thanks

wooorm commented 2 years ago

ah, I didnā€™t think of that, but understandable now I read it back. You can read more about the syntax in the readme: https://github.com/micromark/micromark-extension-directive#syntax, which mentions this behavior and shows an example

desiprisg commented 1 year ago

While I see that changing the outer container when adding an inner container is not great, Iā€™m not interested personally on adding something that a) differs from the spec, b) will be very hard to implements. I could be persuaded to allow it under a flag, but that still leaves b).

Perhaps easier to implement and acceptable for your problem would be the inverse of the current behavior. I think thatā€™s what you meant with ā€œI think it would be an OK solution if you only needed to add an additional semicolon to the child instead of modifying the parent every time you add a childā€, right?:

:::outer
::::med
:::::inner
:::::
::::
:::

This would make editing much easier. Is there anything that made the inverse a better option at the time of the proposal?