-
Notifications
You must be signed in to change notification settings - Fork 3
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
Inline Options for Roles and Directives #28
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: Rowan Cockett <[email protected]>
ce47cdf
to
4b5b761
Compare
Co-authored-by: Rowan Cockett <[email protected]>
@choldgraf @rowanc1 @fwkoch this is now set to active! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small comment, I'm supportive of this change and excited to see it unlock new functionality for roles and directives (especially for HTML outputs). I like the fact that it doesn't break any existing syntax (since only a subset of invalid syntax will now be valid), and that it follows patterns that have become common across adjacent markdown syntax ecosystems. +1 from me
meps/mep-0003.md
Outdated
|
||
### Inline Attribute Syntax | ||
|
||
The internal inline syntax for specifying content will follow existing standards in [Pandoc](https://pandoc.org/chunkedhtml-demo/8.18-divs-and-spans.html)/[djot](https://htmlpreview.github.io/?https://github.com/jgm/djot/blob/master/doc/syntax.html#inline-attributes), with the addition of the role/directive name that already exists in MyST Markdown. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
except by placing it in front it goes against these standards, and also somewhat breaks djot's 1st design goal
It should be possible to parse djot markup in linear time, with no backtracking.
now every time you encounter a {
you have to do a complex parse to work out if it is a role. It will also mean people will have to escape {
in certain circumstances where you can use them currently in plain text
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sentence could be cleaned up, we are referring to internal inline syntax
i.e. the things inside of the braces. MyST syntax is already significantly different than djot/pandoc here, and this proposal is not a large step by any means in aligning the syntax.
I don't understand how placing it in front changes the complexity of where we are now with myst; can you clarify that? With this proposal, we simply allow more options inside of the braces than we did before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand how placing it in front changes the complexity of where we are now with myst
this is how you check if you have reached a role during parsing:
https://github.com/jupyter-book/mystmd/blob/d1f8768eb4dbada0a370765ba696fbb3c00ba66f/packages/markdown-it-myst/src/roles.ts#L31-L32
how will you do this now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, so you are saying that we will no longer be able to parse this with a regexp because it is complex, so the inline code has to be parsed and then could reject. The implementation we have experimented with so far is using regexp still, but that will fail in more complex situations, so we would have to parse to do that properly.
Hadn't thought about the performance implications of this, thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're in a slightly tricky part of the problem space because of MyST Markdown's history.
This MEP (roles and directives)
In my thinking, roles and directives are related to spans and divs (notwithstanding the fact that we have span
roles and div
directives). This MEP is designed to add configuration to roles (and directives) in a way that differs from djot et al.'s inline attributes.
For example,
{cite style=author}`hello_world_1984`
vs
*Hello world*{.red}
There is some overlap — adding labels and IDs to roles feels natural. We could more strictly separate the features, such that this MEP only addresses options:
{cite style=author}`hello_world_1984`
and prevents IDs, classes, etc. i.e. forbids
{cite style=author .red}`hello_world_1984`
This would then allow us to strictly talk about those attribute sets in a different proposal. But, I don't immediately see the merit of this — it feels like boilerplate and worse readability.
So, given that we already have roles, and these roles would benefit from accepting configuration (options), this MEP extends the role definition syntax to include the ability to set configuration and annotation using inline attributes.
A future MEP for annotations
This does, of course, raise the question of what an MEP that allows for annotation of inline elements should look like. It's worth zooming out here to think about MyST vs djot:
Djot
Djot's inline attribute syntax allows the author to set ID, class, and arbitrary key-value pairs. It appears that the intention for these key-value pairs is to be carried as custom data for transforms to inspect (i.e. an attributes
bundle). As such, Djot's approach feels quite Quarto/Pandoc-like in that extensibility is handled through annotations and transforms.
MyST
MyST meanwhile handles extensibility through custom roles and directives, and transforms. In this way, setting arbitrary key-value annotations feels less useful. Given that MyST has a well-defined AST, it feels like we want to set existing schema-defined attributes rather than arbitrary key-value pairs.
% Role with ID
{delete #old}`See this old text`
% Role with option
{eval quote=true}`"Hello" + "World"`
% Inline attribute
![](){width=128px}
Something that comes to mind here is that setting e.g. image widths from {width=128px}
feels fragile — how do we validate this?
I am still thinking about this.
where `props.myProp` will be `true`. We might consider extending this syntax such that: | ||
|
||
```` | ||
```{code linenos} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
flag values are not part of pandoc/djot: jgm/djot#257
This is a WIP draft for adding "inline attributes" to MyST.
Spec Issue: jupyter-book/myst-spec#68