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

Simplified Admonition Titles #12

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Simplified Admonition Titles #12

wants to merge 3 commits into from

Conversation

rowanc1
Copy link
Member

@rowanc1 rowanc1 commented Feb 24, 2023

📖 Read the MEP Here

We propose a simplified syntax to allow arguments for all named admonitions. The syntax allows an argument for any title of a named admonition like {tip} or {note}, to allow users to easily set the color and icon of an admonition while also having a custom title. The proposed syntax is:

```{note} Custom Note Title
Body of the styled admonition.
```

Coauthored with @mmcky.

See also:

TODO:

  • Merge cross references, turn that to active MEP0002!
  • Another pass on the document, cleaning up any ideas
  • Get images into a local folder
  • discuss MEP codemods and how to do this, update that section

@chrisjsewell
Copy link
Contributor

chrisjsewell commented Feb 24, 2023

So, to be frank, I feel it will be impossible to make this change in https://github.com/executablebooks/MyST-Parser:
it would break too much, without any deprecation route.

As discussed in jupyter-book/myst-spec#57, I do think it would be possible with div though, i.e.

:::{note} Custom Note Title
Body of the styled admonition.
:::

@chrisjsewell
Copy link
Contributor

With div it would be possible, because the current colon_fence is a syntax extension:
We could detect if a user has set colon_fence, and if so issue a warning, saying they should change to div, and providing a URL link to a page explaining the difference

I would also suggest with div, we may want to think about (a) no longer using :a: b to denote options, and instead using block attributes, and (b) introducing a leaf div

So instead of

:::{name}
:class: class
:name: name
:key: value

Content
:::

it could be

{#name .class key=value}
:::{name}
Content
:::

or even

{#name .class key=value}
::{name} Content

the key thing here obviously being terseness of the syntax

@rowanc1
Copy link
Member Author

rowanc1 commented Feb 24, 2023

Ok, I didn't understand the different code paths between a fence and a code block on the python/docutils side. For JS there isn't an issue at all.

I think that really high-grades the colon-fence MEP. Happy to have the colon fence be the documented/only way to do this, the only reason I used backticks here is because that isn't in the myst-spec at all yet.

Let's use #13 to track that, and then move onto #10.

@chrisjsewell
Copy link
Contributor

Yep, as mentioned in executablebooks/MyST-Parser#713

I feel the key difference here is that:

  • ``` fence should be for enclosing non-myst content
  • ::: div should be for enclosing myst content

@chrisjsewell
Copy link
Contributor

(Happy to jump on a chat to discuss this with you guys 😄 )

@chrisjsewell
Copy link
Contributor

chrisjsewell commented Feb 24, 2023

Note, one thing that is maybe problematic about {#name .class key=value} though, is if you need a flag, like

:::{name}
:flag:
:::

e.g. this is not currently valid attribute syntax {flag}, so at resent you would have to do something like {flag=true}

@mmcky
Copy link

mmcky commented Feb 26, 2023

Yep, as mentioned in executablebooks/MyST-Parser#713

I feel the key difference here is that:

  • ``` fence should be for enclosing non-myst content
  • ::: div should be for enclosing myst content

hey @chrisjsewell @rowanc1 I am not sure I fully understand this point / discussion.

It would be great to link up at some point to fully explore this.

My concern here is that from a user perspective -- they really shouldn't need to know the difference between myst and non-myst content in my mind. To me we have an entry point into directives which represents -- do something special with whatever is in here.

@chrisjsewell
Copy link
Contributor

My concern here is that from a user perspective -- they really shouldn't need to know the difference between myst and non-myst content in my mind. To me we have an entry point into directives which represents -- do something special with whatever is in here.

Except, I would suggest, many people do. ::: containers were heavily requested, in particular as a way to have admonitions (or really any directive containing Markdown) render correctly in "non-myst" environments (like here on GitHub).
That, plus it makes it a lot more friendly for static anlysers (like syntax highlighters and LSPs) to assess the source text, without having knowledge of every possible directive

If we do a quick GitHub search: https://github.com/search?q=%5C%22colon_fence%5C%22&type=code
there are 1200 hits

By contrast, and I'm happy to be proved otherwise, I've had very few people ask for what this MEP proposes;
it's more of a nice to have.

and yeh to re-iterate, this MEP is essentially suggesting changing the whole semantics of ``` directive parsing, to improve one use-case.
Plus, in a way that would likely break many myst-parser projects and, more critically, it would break them in a way that is really difficult/impossible to detect, warn about and provide deprecation pathways

@tavin
Copy link

tavin commented Feb 27, 2023

As a random end user stumbling across this let me remark that @chrisjsewell's direction sounds good to me. I had the wish already that anything in a colon fence would be rendered as markdown, period, and never as preformatted code.

@rowanc1 rowanc1 changed the base branch from chrisjsewell-mep-process-proposal to main March 2, 2023 23:16
@rowanc1 rowanc1 force-pushed the admonition-title branch from 4e6e83b to deb4060 Compare March 2, 2023 23:16
@rowanc1 rowanc1 force-pushed the admonition-title branch from f0715ae to 548bb07 Compare March 4, 2023 16:38
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.

4 participants