♻️ REFACTOR: Directive plugins #31

chrisjsewell · 2021-05-29T07:38:49Z

Note for development I have put the code in the src/directives2 folder, but eventually this will replace src/directives. The admonition plugin is functionally complete (~~except option conversion/validation~~) and now I will start looking at the figure and math ones.

So this PR essentially re-writes the whole of the directive plugin code/logic.
This is necessary since, among other things, for the current code:

It doesn't handle parsing options
It doesn't handle nested admonitions (in fact the parsing completely expects if you have any)
It doesn't parse admonition titles as Markdown
There is no obvious way to map from Sphinx <-> these plugins

The new logic is designed to closely mirror sphinx/myst-parser and follows as such:

The src/directives/base.ts:pluginDirectiveBase is defined.

All specific directive plugins will rely on this being loaded first.
It simply walks through the token stream and converts all code fences, whose token.info matches the directive regex (i.e. looks like {name} argument), to a directive_base type token, changes the token.info to just the name and stores the argument in token.meta.arg. See tests/directiveBase.spec.ts for an example.

Having this as a separate plugin separates out the logic of identifying a directive, from how it is processed/rendered.
It means that it is possible, in the future, to change our directive syntax without affecting any of the other directive plugins. Or even people can use different plugins to generate these directive_base tokens.

src/directives/parseStructure.ts:parseStructure is defined.

This is a function converts the first line and body of a directive_base token to the structure of a directive: args (list of strings), options (dict), body (string), dependent on the particular directive specification (e.g. how many arguments it requires and whether it has a body). The code is directly adapted from myst_parser/parse_directives.py.

This code is generic to all directives, and will be called in the individual plugins.

The src/directives2/admonition.ts:pluginDirectiveAdmonition is defined.

This acts on directive_base tokens of specific names (admonition, note, etc); first calling parseStructure, performing a nested (block level only) parse on the body string of the directive, then replacing the original token with the following list of tokens:

open_admonition (tag = aside class = 'admonition' + type of admonition + any set in the class option of the directive)
open_admonition_title (tag = div, class = 'admonition-title')
inline (containing the title text)
close_admonition_title
open_admonition_body(tag = div, class = 'admonition-body')
list of body tokens output from the nested parsing
close_admonition_body
close_admonition

Outputting this "structure" of tokens means that you don't actually require a specific render method for admonitions, since the default markdown-it renderer will deal with it. Where possible this how plugins should be implemented; generating a token structure that directly mirrors the HTML structure and requires no/minimal custom rendering.

See for tests/directiveAdmonition.spec.ts for more examples, but this is what the parse looks like:

````{admonition} arg string
:class: class1 class2

content
```{admonition} arg string2
content2
```
````

is parsed to tokens:

[
'{"type":"open_admonition","tag":"aside","attrs":[["class","admonition' +
    '"]],"map":[0,6],"nesting":1,"level":0,"children":null,"content":"","markup":"","info":"","meta":null,"block":false,"hidden":false}',
'{"type":"open_admonition_title","tag":"div","attrs":[["class","admonition-title"]],"map":null,"nesting":1,"level":0,"children":null,"content":"","markup":"","info":"","meta":null,"block":false,"hidden":false}',
'{"type":"inline","tag":"","attrs":null,"map":[0,0],"nesting":0,"level":0,"children":[{"type":"text","tag":"","attrs":null,"map":null,"nesting":0,"level":0,"children":null,"content":"arg string","markup":"","info":"","meta":null,"block":false,"hidden":false}],"content":"arg string","markup":"","info":"","meta":null,"block":false,"hidden":false}',
'{"type":"close_admonition_title","tag":"div","attrs":null,"map":null,"nesting":-1,"level":0,"children":null,"content":"","markup":"","info":"","meta":null,"block":false,"hidden":false}',
'{"type":"open_admonition_body","tag":"div","attrs":[["class","admonition-body"]],"map":[1,5],"nesting":1,"level":0,"children":null,"content":"","markup":"","info":"","meta":null,"block":false,"hidden":false}',
'{"type":"paragraph_open","tag":"p","attrs":null,"map":[1,2],"nesting":1,"level":0,"children":null,"content":"","markup":"","info":"","meta":null,"block":true,"hidden":false}',
'{"type":"inline","tag":"","attrs":null,"map":[1,2],"nesting":0,"level":1,"children":[{"type":"text","tag":"","attrs":null,"map":null,"nesting":0,"level":0,"children":null,"content":"content","markup":"","info":"","meta":null,"block":false,"hidden":false}],"content":"content","markup":"","info":"","meta":null,"block":true,"hidden":false}',
'{"type":"paragraph_close","tag":"p","attrs":null,"map":null,"nesting":-1,"level":0,"children":null,"content":"","markup":"","info":"","meta":null,"block":true,"hidden":false}',
'{"type":"open_admonition","tag":"aside","attrs":[["class","admonition' +
    '"]],"map":[2,5],"nesting":1,"level":0,"children":null,"content":"","markup":"","info":"","meta":null,"block":false,"hidden":false}',
'{"type":"open_admonition_title","tag":"div","attrs":[["class","admonition-title"]],"map":null,"nesting":1,"level":0,"children":null,"content":"","markup":"","info":"","meta":null,"block":false,"hidden":false}',
'{"type":"inline","tag":"","attrs":null,"map":[2,2],"nesting":0,"level":0,"children":[{"type":"text","tag":"","attrs":null,"map":null,"nesting":0,"level":0,"children":null,"content":"arg string2","markup":"","info":"","meta":null,"block":false,"hidden":false}],"content":"arg string2","markup":"","info":"","meta":null,"block":false,"hidden":false}',
'{"type":"close_admonition_title","tag":"div","attrs":null,"map":null,"nesting":-1,"level":0,"children":null,"content":"","markup":"","info":"","meta":null,"block":false,"hidden":false}',
'{"type":"open_admonition_body","tag":"div","attrs":[["class","admonition-body"]],"map":[3,4],"nesting":1,"level":0,"children":null,"content":"","markup":"","info":"","meta":null,"block":false,"hidden":false}',
'{"type":"paragraph_open","tag":"p","attrs":null,"map":[3,4],"nesting":1,"level":0,"children":null,"content":"","markup":"","info":"","meta":null,"block":true,"hidden":false}',
'{"type":"inline","tag":"","attrs":null,"map":[3,4],"nesting":0,"level":1,"children":[{"type":"text","tag":"","attrs":null,"map":null,"nesting":0,"level":0,"children":null,"content":"content2","markup":"","info":"","meta":null,"block":false,"hidden":false}],"content":"content2","markup":"","info":"","meta":null,"block":true,"hidden":false}',
'{"type":"paragraph_close","tag":"p","attrs":null,"map":null,"nesting":-1,"level":0,"children":null,"content":"","markup":"","info":"","meta":null,"block":true,"hidden":false}',
'{"type":"close_admonition_body","tag":"div","attrs":null,"map":null,"nesting":1,"level":0,"children":null,"content":"","markup":"","info":"","meta":null,"block":false,"hidden":false}',
'{"type":"close_admonition","tag":"aside","attrs":null,"map":null,"nesting":-1,"level":0,"children":null,"content":"","markup":"","info":"","meta":null,"block":false,"hidden":false}',
'{"type":"close_admonition_body","tag":"div","attrs":null,"map":null,"nesting":1,"level":0,"children":null,"content":"","markup":"","info":"","meta":null,"block":false,"hidden":false}',
'{"type":"close_admonition","tag":"aside","attrs":null,"map":null,"nesting":-1,"level":0,"children":null,"content":"","markup":"","info":"","meta":null,"block":false,"hidden":false}'
]

which is rendered to:

<aside class="admonition class1 class2"><div class="admonition-title">arg string</div><div class="admonition-body"><p>content</p>
<aside class="admonition"><div class="admonition-title">arg string2</div><div class="admonition-body"><p>content2</p>
<div></aside><div></aside>

codecov · 2021-05-29T07:39:45Z

Codecov Report

Merging #31 (f9f24ab) into main (c517f5c) will decrease coverage by 1.06%.
The diff coverage is 92.47%.

❗ Current head f9f24ab differs from pull request most recent head 32b8893. Consider uploading reports for the commit 32b8893 to get more accurate results

@@            Coverage Diff             @@
##             main      #31      +/-   ##
==========================================
- Coverage   97.12%   96.05%   -1.07%     
==========================================
  Files          24       28       +4     
  Lines         626      812     +186     
  Branches      135      181      +46     
==========================================
+ Hits          608      780     +172     
- Misses         18       32      +14

Impacted Files	Coverage Δ
src/directives/types.ts	`100.00% <ø> (ø)`
src/utils.ts	`96.55% <ø> (ø)`
src/directives2/base.ts	`80.00% <80.00%> (ø)`
src/directives2/parseStructure.ts	`90.21% <90.21%> (ø)`
src/directives2/admonition.ts	`98.55% <98.55%> (ø)`
src/directives2/optionConverters.ts	`100.00% <100.00%> (ø)`

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 c517f5c...32b8893. Read the comment docs.

chrisjsewell · 2021-05-29T07:40:00Z

cc'ing @choldgraf @rowanc1 , @damianavila and @agoose77 for an initial look

chrisjsewell · 2021-05-29T12:14:15Z

I have now also made the HTML tags for open_admonition, open_admonition_title and open_admonition_body a setting of the plugin. The use case for this, is that you can set main as details and title as summary, and voila you've got collapsible admonitions 🎉
See e.g. https://github.com/executablebooks/sphinx-panels/blob/master/sphinx_panels/scss/panels/_dropdown.scss, how you can style these as an accordion

rowanc1

Standardizing on the tokens coming out makes sense. As this comes in I will need to update downstream packages to compensate for these changes.

It looks like this is removing the reliance on markdown-it-container, which may have been giving some problems with nesting of directives. Once this replaces, we should remove that library from dependencies.

This will also require some changes to the numbering (figure/equation/references) that I have done so far. Thinking that some of that may not be wanted/needed by consumers if they are only interested in the structure of the outputs (at the AST level).

Additionally the docs will need to change:
https://executablebooks.github.io/markdown-it-myst/overview.html#creating-a-new-directive

On a broader level, are you planning other large structural changes? Can we discuss those in an issue if you are?

src/directives2/base.ts

rowanc1 · 2021-05-29T20:06:53Z

src/directives2/base.ts

+    if (logError) {
+      console.error('unexpected "directive_base" token', token)
+    }
+    return `<pre><code>\n${token.info}: ${token.meta.arg}\n\n${token.content}\n</code></pre>\n`


Probably worth using escapeHtml here?

import { escapeHtml } from 'markdown-it/lib/common/utils';

rowanc1 · 2021-05-29T20:11:25Z

src/directives2/parseStructure.ts

+  const totalArgs =
+    (fullSpec.required_arguments || 0) + (fullSpec.optional_arguments || 0)
+  if (args.length < (fullSpec.required_arguments || 0)) {
+    throw new DirectiveParsingError(


Is there a strategy for collecting errors in the python implementation?

Would be nice for this to succeed in parsing, but show warnings to the user.

yes they are logged as warnings, the parsing succeeds but the directive is not rendered. there is already a TODO in the admonition plugin, where the error is caught and the token discarded (presently without warning)

rowanc1 · 2021-05-29T20:13:32Z

tests/directiveAdmonition.spec.ts

+      const tokens = mdit.parse(basicDirective, {})
+      // console.log(tokens.map(token => JSON.stringify(token)))
+      expect(tokens.map(token => JSON.stringify(token))).toEqual([
+        '{"type":"open_admonition","tag":"aside","attrs":[["class","admonition' +


Makes sense to standardize at this level - hadn't been thinking about that.

Will each directive have its own type?

yes indeed; the directives/roles in the first instance should be building up the token stream (i.e. AST), this allows for other plugins to potentially modify it "later".

rowanc1 · 2021-05-29T20:19:45Z

src/directives2/admonition.ts

+
+const defaultTags = {
+  main: 'aside',
+  title: 'div',


I was using header (semantic html) for this, and dropping the then unnecessary class name.

yep I can change this to header; I was having a look if there was any good tag for the body, but didn't see one? I'm not convinced though thats these can fully replace class names, for specificity

rowanc1 · 2021-05-29T20:25:04Z

src/directives2/admonition.ts

+      adToken.attrSet('class', classes.join(' '))
+      newTokens.push(adToken)
+      const adTokenTitle = new Token('open_admonition_title', tags.title, 1)
+      adTokenTitle.attrSet('class', 'admonition-title')


Are these classes part of the rendering or the tokens? It seems that the information about classes should be left to the renderer which can decide that an open_admonition_title should have a specific css class?

as mentioned there is no special renderer, its all handled by markdown-it's default one

rowanc1 · 2021-05-29T20:27:43Z

src/directives2/parseStructure.ts

+  required_arguments: 0,
+  optional_arguments: 0,
+  final_argument_whitespace: false,
+  has_content: false,


Directive defaults have no content by default?

rowanc1 · 2021-05-29T20:55:11Z

src/directives2/base.ts

+  regex = DIRECTIVE_PATTERN,
+  logError = true
+): void {
+  // TODO also convert colon-fences


What is your strategy on this? Do the fences already come from markdown-it core identifying them?

Its a separate plugin: https://github.com/executablebooks/mdit-py-plugins/blob/master/mdit_py_plugins/colon_fence.py

chrisjsewell · 2021-05-29T21:39:32Z

Thanks for the comments 👍, here some responses:

Standardizing on the tokens coming out makes sense. As this comes in I will need to update downstream packages to compensate for these changes.

Yes indeed, and that will be covered by semantic versioning. If you haven't already, I would suggest pinning to 0.0.x

It looks like this is removing the reliance on markdown-it-container, which may have been giving some problems with nesting of directives. Once this replaces, we should remove that library from dependencies.

Indeed, this was removed a long time ago in the Python code

This will also require some changes to the numbering (figure/equation/references) that I have done so far. Thinking that some of that may not be wanted/needed by consumers if they are only interested in the structure of the outputs (at the AST level).

As mentioned in my initial comment, I will be rewriting all of them

Additionally the docs will need to change:
executablebooks.github.io/markdown-it-myst/overview.html#creating-a-new-directive

👍

On a broader level, are you planning other large structural changes? Can we discuss those in an issue if you are?

As mentioned I will be rewriting all the directives. I haven't had a look at the roles yet, but quite possibly aspects of that as well. Again the key point is to have them aligned with the sphinx/myst-parser code (plus make best use of markdown-it). If you haven't already I would strongly suggest you look through the myst-parser code and also sphinx, particularly: https://www.sphinx-doc.org/en/master/extdev/appapi.html, it will be necessary to understand that before understanding why certain decisions have been taken here

Co-authored-by: Rowan Cockett <rowanc1@gmail.com>

chrisjsewell · 2021-07-07T09:44:51Z

@rowanc1 so obviously this PR has essentially been superseded by https://github.com/executablebooks/markdown-it-docutils.
But, as discussed, I've updated the PR with the stuff I was playing around with for a "full" sphinx-like implementation; principally in src/appComponents.ts, and src/app.ts, to have a record of that.
and also docs/design.md should eventually have a home somewhere

rowanc1 · 2022-02-03T23:35:24Z

Closing this! See #34.

Add arxiv to frontmatter

♻️ REFACTOR: Directive plugins

45e1157

chrisjsewell mentioned this pull request May 29, 2021

Sharing assets between myst-parser and markdown-it-myst executablebooks/meta#185

Open

make HTML tags a variable setting

30e2bd4

chrisjsewell added 3 commits May 29, 2021 14:55

ensure propagation of line mappings

a193124

add directive option conversion

9a070eb

fix title parsing

6b642c5

rowanc1 reviewed May 29, 2021

View reviewed changes

chrisjsewell and others added 2 commits May 29, 2021 23:39

Update src/directives2/base.ts

e0dd145

Co-authored-by: Rowan Cockett <rowanc1@gmail.com>

Initial addition of design documentation

f9f24ab

damianavila mentioned this pull request Jun 7, 2021

Team Sync - Jun 07, 2021 2i2c-org/team-compass#114

Closed

chrisjsewell mentioned this pull request Jun 18, 2021

♻️ REFACTOR: Move to markdown-it + mdformat renderer executablebooks/rst-to-myst#18

Merged

update with sphinxy stuff

32b8893

chrisjsewell mentioned this pull request Nov 12, 2021

Ensure good design and learn from docutils/sphinx executablebooks/markdown-it-docutils#18

Open

rowanc1 closed this Feb 3, 2022

rowanc1 deleted the directives branch February 3, 2022 23:35

rowanc1 pushed a commit that referenced this pull request Oct 28, 2022

Merge pull request #31 from curvenote/feat/arxiv

1edfbab

Add arxiv to frontmatter

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

♻️ REFACTOR: Directive plugins #31

♻️ REFACTOR: Directive plugins #31

chrisjsewell commented May 29, 2021 •

edited

Loading

codecov bot commented May 29, 2021 •

edited

Loading

chrisjsewell commented May 29, 2021 •

edited

Loading

chrisjsewell commented May 29, 2021 •

edited

Loading

rowanc1 left a comment

rowanc1 May 29, 2021

rowanc1 May 29, 2021

chrisjsewell May 29, 2021

rowanc1 May 29, 2021

chrisjsewell May 29, 2021

rowanc1 May 29, 2021

chrisjsewell May 30, 2021

rowanc1 May 29, 2021 •

edited

Loading

chrisjsewell May 29, 2021

rowanc1 May 29, 2021

chrisjsewell May 29, 2021

rowanc1 May 29, 2021

chrisjsewell May 29, 2021

chrisjsewell commented May 29, 2021

chrisjsewell commented Jul 7, 2021

rowanc1 commented Feb 3, 2022

♻️ REFACTOR: Directive plugins #31

♻️ REFACTOR: Directive plugins #31

Conversation

chrisjsewell commented May 29, 2021 • edited Loading

codecov bot commented May 29, 2021 • edited Loading

Codecov Report

chrisjsewell commented May 29, 2021 • edited Loading

chrisjsewell commented May 29, 2021 • edited Loading

rowanc1 left a 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

rowanc1 May 29, 2021 • 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

chrisjsewell commented May 29, 2021

chrisjsewell commented Jul 7, 2021

rowanc1 commented Feb 3, 2022

chrisjsewell commented May 29, 2021 •

edited

Loading

codecov bot commented May 29, 2021 •

edited

Loading

chrisjsewell commented May 29, 2021 •

edited

Loading

chrisjsewell commented May 29, 2021 •

edited

Loading

rowanc1 May 29, 2021 •

edited

Loading