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

feature request: ability to nest directives #8

Open
yocontra opened this issue Mar 2, 2021 · 16 comments
Open

feature request: ability to nest directives #8

yocontra opened this issue Mar 2, 2021 · 16 comments
Labels
🙉 open/needs-info This needs some more info 🤞 phase/open Post is being triaged manually 🦋 type/enhancement This is great to have

Comments

@yocontra
Copy link

yocontra commented Mar 2, 2021

👉 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
Copy link
Member

wooorm commented Mar 2, 2021

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
Copy link
Author

yocontra commented Mar 2, 2021

@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
Copy link
Member

wooorm commented Mar 2, 2021

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
Copy link
Author

yocontra commented Mar 2, 2021

@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
Copy link
Member

wooorm commented Mar 2, 2021

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
Copy link
Author

yocontra commented Mar 2, 2021

@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
Copy link
Member

wooorm commented Mar 3, 2021

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
Copy link
Author

yocontra commented Mar 4, 2021

@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
Copy link
Member

wooorm commented Mar 4, 2021

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
Copy link
Member

wooorm commented Apr 11, 2021

Any luck or insights?

@yocontra
Copy link
Author

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

@Madd0g
Copy link

Madd0g commented Nov 11, 2021

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
Copy link
Member

wooorm commented Nov 11, 2021

I wouldn’t call it “best workaround”, because that’s exactly how fenced code also works. It’s intended behavior

@Madd0g
Copy link

Madd0g commented Nov 12, 2021

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
Copy link
Member

wooorm commented Nov 12, 2021

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

@github-actions github-actions bot added the 🤞 phase/open Post is being triaged manually label Dec 30, 2021
@desiprisg
Copy link

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🙉 open/needs-info This needs some more info 🤞 phase/open Post is being triaged manually 🦋 type/enhancement This is great to have
Development

No branches or pull requests

4 participants