🔀 MERGE: Improve optional syntaxes #273

chrisjsewell · 2020-12-15T18:07:11Z

Plugins are now split off into mdit_py_plugins and more plugins are added:

myst_enable_extensions=["colon_fence"] replaces myst_admonition_enable and myst_figure_enable (which are deprecated) to work exactly like standard back-tick code fences and directives, i.e. you can now use any directive as:

:::{name} argument
:option: value

body
:::

myst_enable_extensions = ["dollarmath", ...] now replaces and deprecates individual enable configuration variables: admonition_enable, figure_enable, dmath_enable, amsmath_enable, deflist_enable, html_img_enable

myst_enable_extensions=["linkify", "substitution"] are also big additions

Principally this is all explained in https://myst-parser--273.org.readthedocs.build/en/273/using/syntax-optional.html

codecov · 2020-12-15T18:09:31Z

Codecov Report

Merging #273 (f227428) into master (ebed2a5) will decrease coverage by 0.75%.
The diff coverage is 79.54%.

@@            Coverage Diff             @@
##           master     #273      +/-   ##
==========================================
- Coverage   91.99%   91.23%   -0.76%     
==========================================
  Files          12       13       +1     
  Lines        1399     1484      +85     
==========================================
+ Hits         1287     1354      +67     
- Misses        112      130      +18

Flag	Coverage Δ
pytests	`91.23% <79.54%> (-0.76%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
myst_parser/mathjax.py	`96.87% <ø> (ø)`
myst_parser/parse_directives.py	`87.50% <50.00%> (-0.97%)`	⬇️
myst_parser/__init__.py	`84.61% <72.72%> (-5.04%)`	⬇️
myst_parser/docutils_renderer.py	`92.98% <78.33%> (+0.92%)`	⬆️
myst_parser/directives.py	`79.24% <79.24%> (ø)`
myst_parser/main.py	`87.38% <84.00%> (-6.02%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ebed2a5...f227428. Read the comment docs.

chrisjsewell · 2020-12-17T15:59:41Z

@choldgraf, @mmcky, @AakashGfude this adds/changes some key extensions and adds deprecations, so you should review

chrisjsewell · 2020-12-17T16:22:49Z

As explained at

MyST-Parser/myst_parser/docutils_renderer.py

Line 887 in a21b9bb

def render_substitution(self, token, inline: bool):

In would be nice to use these like Jinja substitutions, with filters, e.g. {{ key | lower }},
however, it is not trivial how to handle cyclic substitution referencing with this; you need to somehow identify all the keys used.

Also, error messaging could be improved

chrisjsewell · 2020-12-17T16:33:13Z

Another open question is inline images; within table cells using:

---
substitutions:
   key3: "![](img/fun-fish.png)"
---

| col1     | col2     |
| -------- | -------- |
| {{key3}} | {{key3}} |

works,

but this does not:

---
substitutions:
  key3: |
    ```{image} img/fun-fish.png
    :alt: fishy
    :width: 200px
    :align: center
    ```
---

it just shows the directive as a literal 🤷

and similar but different with:

---
substitutions:
  key3: <img src="img/fun-fish.png" alt="fishy" width="200px">
---

choldgraf · 2020-12-17T16:43:01Z

docs/using/syntax-optional.md

@@ -1,32 +1,202 @@
-(syntax-optional)=
+---
+substitutions:


using front-matter YAML for this is pretty cool, IMO

choldgraf · 2020-12-17T16:45:19Z

docs/using/syntax-optional.md


+```md
+Inline: {{ key1 }}


One concern that I have with using double-curlies {{ is that it might clash with users that are combining Jinja + MyST Markdown. I could imagine someone, for example, programmatically generating a markdown file with a Jinja template, and then interpolating substitution variables into that file as well. In that case there wouldn't be a way to accommodate both tools, right? I don't know how common this is so not sure if it's worth changing the {{ (since that is so common for interpolation it makes sense to use it), but wanted to raise this

These substitutions are literally combining Jinja with MyST Markdown now (as of abe0bed), i.e. you can do {{ key1 | capitalize }}. So it definitely makes sense that they use {{ and this approach should be able to replace most of these use cases.
I could easily make the delimiters configurable in the markdown-it plugin, or conversely these delimiters are configurable by the any other Jinja based tool.

ah interesting - that's pretty neat / powerful. So the use-case I had in mind when I made this comment was something like the Carpentries. They have a set of "content templates" that (I think) use Jinja interpolation to generate the actual course material for a specific Carpentry course. The generated material itself might also use the interpolation that we describe here (e.g. define a variable in the conf.py and re-use it throughout the book, or something like that.

So a couple quick thoughts:

I suppose at the Jinja interpolation side, they could interpolate-in variables that had {{ in the pasted-in content. E.g. they could have the variable {{ mykey }} and mykey would equal {{ my-other-variable-in-myst-header }}, that's ultimately what'd get pasted into the MyST Markdown.

Alternatively, it sounds like you're saying you could configure Jinja to treat a different delimeter (like || or something) to do the interpolation? I didn't realize that was possible!

Sorry if it seems like I am being nit-picky here, I don't think that this is a common use-case, but I am trying to make sure we don't set ourselves up for an anti-pattern here.

Yeh you can choose whatever you want for the delimiters: https://jinja.palletsprojects.com/en/2.11.x/api/#jinja2.Environment

myst_sub_delimiters now added (and explained in docs)

choldgraf · 2020-12-17T16:46:45Z

docs/using/syntax-optional.md

+
+(syntax/colon_fence)=
+
+## Code fences using colons


I think this makes sense because having the ::: behave similarly-but-not-quite the same was adding confusion. That said, I think we should make sure to have good "best practice" guides for people - as someone noted in another issue, we're now adding so much functionality or flexibility that it can be hard for people to figure out the "right" way to go about doing things. If we are violating the "there should be one, and ideally only one way to do something" principle, then we should make sure to document why there are multiple pathways, and in what situations you'd choose one over the other.

As I explained there, I think this is generally already done, but happy to review if someone wants to try altering the Jupyter book documentation.
In the actual documentation here, I think its pretty clear, whereby there is one file/page for (core) syntax; docs/using/syntax.md and one for optional/extended syntax; docs/using/syntax-optional.md.
There are then one "core" way an other optional ways, that pepoole can ignore if they want.

The main thing really, is if you care about what the source files look like. If not, just use core, but if you want it to be most readable outside of sphinx (e.g. in notebooks) then use things like ::: blocks and HTML images

I think that here it's totally fine - the myst parser docs are more "low level" and thus I think should focus on explaining all the features in full over streamlining for a particular workflow. For Jupyter Book perhaps we'll want to re-visit the "introductory" parts a bit more, since that's a more "opinionated" distribution and will probably attract more beginner-level readers that are more likely to get tripped up. I don't think this is actionable on this PR at all though 👍

docs/using/syntax-optional.md

choldgraf · 2020-12-17T16:48:43Z

docs/using/syntax-optional.md


 ```md
-:::{admonition,warning} This *is* also **Markdown**
+:::{admonition} This *is* also **Markdown**


just to be clear - we are now deprecating this original admonition,warning syntax, right?

yes its deprecated. By deprecated though, I mean entered a deprecation cycle; after this PR, the code will still be parsed and create the same HTML, but a (sphinx) warning will be issued of the deprecation.
Then it will be actually removed in the version after next

This is the same for all the deprecated sphinx configuration options

so people should still be able to make their sites/books as before, but they will get a number of warnings, if they were using these features (with messages pointing them towards how to fix)

makes sense to me - thanks for the explanation 👍

choldgraf · 2020-12-17T16:49:59Z

docs/using/syntax-optional.md

+## Admonition directives
+
+:::{important}
+`myst_admonition_enable` is deprecated and replaced by `myst_enable_extensions = ["colon_fence"]` (see above).


just a note that I think adding a list of extensions to enable is a much better pattern than having a unique variable for each 👍

One extra thought I had - what if we used a single configuration variable like myst_config and then had keys / values for our various configurations, rather than using a different variable for each that started with myst_?

meh, I don't think that is necessary or wanted: I believe it would make it difficult in jupyter-book for people to override the defaults of specific aspects of myst-parser, because they would then have to specify the whole dictionary

makes sense, I think there are pros/cons just from a "readability" perspective

e.g. I think that for a medium-large number of config over-rides:

myst: extensions: - a - b otheroption: otherval

is more readable than

myst_extensions: - a - b myst_otheroption: otherval

but anyway discussing this I think should be left to the future and need not block this PR

choldgraf · 2020-12-17T16:53:57Z

I will try and play around with this tomorrow or over the weekend, but took a quick pass and had a few comments in there. In general this is looking great, I like the new config style and love the new features that we're adding in here. Will try to wrap my head around it a bit more.

chrisjsewell · 2020-12-18T00:50:51Z

Another open question is inline images;

with 08460b5, inline HTML images are now correctly parsed (and can be inserted in tables)

In would be nice to use these like Jinja substitutions, with filters

implemented in abe0bed!

So now you can do e.g. {{ env.docname | upper }} (see updated documentation)

choldgraf · 2020-12-18T10:42:09Z

docs/using/syntax-optional.md

+```
+
+{{ env.docname | upper }}
+{{ "a" + "b" }}


This makes me wonder if there is a way to (in the future, not this PR) incorporate glue-like functionality into this syntax. I think {{ mykey }} would be the natural short-hand for {glue:}`mykey`, don't you think? Would using this syntax both for "markdown substitutions" and for glue syntax be possible? (again, I do not think we should do it in this PR, just checking whether it would be possible)

Oh yeh this was exactly in my thinking

nice - I think using Jinja for the md substitution is super cool :-)

Plugins are now split off into `mdit_py_plugins`

This replaces `myst_admonition_enable` and `myst_figure_enable`(which are dprecated) to work exactly like standard back-tick code fences and directives.

Add replacements, smartquotes, linkify and substitution. Additionally `myst_enable_extensions` now replaces and deprecates individual enable configuration variables: `admonition_enable`, `figure_enable`, `dmath_enable`, `amsmath_enable`, `deflist_enable`, `html_img_enable`

To configure substitution delimiters.

AakashGfude · 2020-12-19T14:13:29Z

Great work @chrisjsewell . Will read through this, and get familiar with the changes. Sorry, did not get time to review this earlier.
Had a question though, now, that this is merged, I am getting this warning on building docs with pdflatex builder.
https://github.com/executablebooks/jupyter-book/pull/1125/checks?check_run_id=1581634526 . What is the resolution to this?

chrisjsewell · 2020-12-19T15:34:12Z

just change the reference to myst-parser:syntax/optional

AakashGfude · 2020-12-20T01:39:28Z

just change the reference to myst-parser:syntax/optional

too easy. Thanks.

chrisjsewell requested review from choldgraf, mmcky and AakashGfude December 17, 2020 15:58

chrisjsewell linked an issue Dec 17, 2020 that may be closed by this pull request

Add substitution syntax(?) #172

Closed

chrisjsewell mentioned this pull request Dec 17, 2020

Add substitution syntax(?) #172

Closed

chrisjsewell linked an issue Dec 17, 2020 that may be closed by this pull request

Add more :::{name} optional syntaxes #213

Closed

3 tasks

chrisjsewell mentioned this pull request Dec 17, 2020

Allow for :::admonition syntax (without curly brackets) #271

Closed

chrisjsewell linked an issue Dec 17, 2020 that may be closed by this pull request

Enable plugins with a single configuration value #264

Closed

This was referenced Dec 17, 2020

Allow for admonition titles without specifying "admonition" when using ::: syntax #259

Closed

Support for all-pages-on-site epilog and prolog #270

Closed

support prolog (& epilog) content #190

Open

choldgraf reviewed Dec 17, 2020

View reviewed changes

docs/using/syntax-optional.md Show resolved Hide resolved

choldgraf reviewed Dec 17, 2020

View reviewed changes

choldgraf reviewed Dec 18, 2020

View reviewed changes

chrisjsewell added 6 commits December 18, 2020 21:22

⬆️ UPGRADE: markdown-it-py v0.6

061cfe8

Plugins are now split off into `mdit_py_plugins`

✨🗑 NEW: Add myst_colon_fence_enable

5013b60

This replaces `myst_admonition_enable` and `myst_figure_enable`(which are dprecated) to work exactly like standard back-tick code fences and directives.

👌 IMPROVE: Render inline HTML images

0162ffb

👌 IMPROVE: Parse substitution references with Jinja2

34669b7

👌 IMPROVE: add myst_sub_delimiters

f227428

To configure substitution delimiters.

chrisjsewell force-pushed the update-mdit branch from d4e4a03 to f227428 Compare December 18, 2020 21:24

chrisjsewell changed the title ~~⬆️ UPGRADE: markdown-it-py v0.6~~ 🔀 MERGE: Improve optional syntaxes Dec 18, 2020

chrisjsewell merged commit 4b7f571 into master Dec 18, 2020

chrisjsewell deleted the update-mdit branch December 18, 2020 21:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🔀 MERGE: Improve optional syntaxes #273

🔀 MERGE: Improve optional syntaxes #273

chrisjsewell commented Dec 15, 2020 •

edited

Loading

codecov bot commented Dec 15, 2020 •

edited

Loading

chrisjsewell commented Dec 17, 2020

chrisjsewell commented Dec 17, 2020 •

edited

Loading

chrisjsewell commented Dec 17, 2020 •

edited

Loading

choldgraf Dec 17, 2020

choldgraf Dec 17, 2020

chrisjsewell Dec 18, 2020

choldgraf Dec 18, 2020

chrisjsewell Dec 18, 2020

chrisjsewell Dec 18, 2020

choldgraf Dec 17, 2020 •

edited

Loading

chrisjsewell Dec 18, 2020

choldgraf Dec 18, 2020

choldgraf Dec 17, 2020

chrisjsewell Dec 18, 2020

chrisjsewell Dec 18, 2020

choldgraf Dec 18, 2020

choldgraf Dec 17, 2020

chrisjsewell Dec 18, 2020

choldgraf Dec 18, 2020 •

edited

Loading

choldgraf commented Dec 17, 2020

chrisjsewell commented Dec 18, 2020

choldgraf Dec 18, 2020 •

edited

Loading

chrisjsewell Dec 18, 2020

choldgraf Dec 18, 2020 •

edited

Loading

AakashGfude commented Dec 19, 2020

chrisjsewell commented Dec 19, 2020

AakashGfude commented Dec 20, 2020

🔀 MERGE: Improve optional syntaxes #273

🔀 MERGE: Improve optional syntaxes #273

Conversation

chrisjsewell commented Dec 15, 2020 • edited Loading

codecov bot commented Dec 15, 2020 • edited Loading

Codecov Report

chrisjsewell commented Dec 17, 2020

chrisjsewell commented Dec 17, 2020 • edited Loading

chrisjsewell commented Dec 17, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

choldgraf Dec 17, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

choldgraf Dec 18, 2020 • edited Loading

Choose a reason for hiding this comment

choldgraf commented Dec 17, 2020

chrisjsewell commented Dec 18, 2020

choldgraf Dec 18, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

choldgraf Dec 18, 2020 • edited Loading

Choose a reason for hiding this comment

AakashGfude commented Dec 19, 2020

chrisjsewell commented Dec 19, 2020

AakashGfude commented Dec 20, 2020

chrisjsewell commented Dec 15, 2020 •

edited

Loading

codecov bot commented Dec 15, 2020 •

edited

Loading

chrisjsewell commented Dec 17, 2020 •

edited

Loading

chrisjsewell commented Dec 17, 2020 •

edited

Loading

choldgraf Dec 17, 2020 •

edited

Loading

choldgraf Dec 18, 2020 •

edited

Loading

choldgraf Dec 18, 2020 •

edited

Loading

choldgraf Dec 18, 2020 •

edited

Loading