💬 Add epigraph, pull-quote and blockquote directive

[agoose77]

The intentions of this PR are twofold:

1. Provide a learning curve for authoring directives in MyST
2. Implement support for epigraphs and pull-quotes

I'll try and document my thought processes as we go.

The merge order should be

1. ENH: add blockquote to supported container contents myst-spec#64
2. 💬 Add epigraph, pull-quote and blockquote directive #961
3. 💬 Add style for pull-quotes myst-theme#328

[agoose77]

Here's what I wrote in Discord about my initial thoughts:

I'm looking to implement support for epigraphs and pull-quotes. Adding a new quote directive (pull-quote) is fairly easy, but adding a new epigraph directive throws up some questions for me:

1. We use transforms to apply custom syntax rules, e.g. GFM admonitions (blockquote -> admonition). Is this "safe" w.r.t third-party mdast utilities? i.e., if a third-party tool expects the non-MyST mdast, does the presence of our nodes in the tree in unexpected places cause issues? I assume that perhaps MDAST is built such that one can extend it, so these tools should ignore unknown content?
2. For nodes with well-constrained children, should we lift the child into an attribute of the parent? e.g. for epigraph, there's an attribution. Sphinx seems to leave that as a node-type, added to children. Should we think about doing the same, or create a new epigraph MDAST node that has an attribution field? My feeling is strongly the former; we want to avoid duplicating existing MDAST nodes in favour of "extending" them through children. I also assume we shouldn't generally add attributes to existing MDAST nodes defined in https://github.com/syntax-tree/

[agoose77]

I then noticed some prior art for epigraphs, in tex-to-myst. So, it looks like we're re-using the caption node instead of attribution node. Hmm, OK, let's try that (see e3e8f30)

[agoose77]

It looks like this falls foul of another transform, https://github.com/executablebooks/mystmd/blob/76c4d7b961579e2d1a3464360424b977f062cebe/packages/myst-transforms/src/containers.ts#L182, which seems to assume a closed set of container node types. It looks like the tex-to-myst epigraph conversion isn't anticipated here. I wonder why, I assume that this transform would apply after tex-to-myst (e.g. via a raw) directive.

[agoose77]

The next step from here probably depends upon whether we want blockquote et al. to support attribution (captions). If we do (which feels like a commonmark regression), we can just make epigraph a synonym of blockquote, and implement the captioning using a transform.

If not, then we need to adjust the container transform.

[agoose77]

I think the simplest solution is to make all quotes support captions, which is what 87e4189 does. Now, pull-quotes and epigraphs can be styled separately should we wish for them to be.

[rowanc1]

Looking good. Have some comments about the change in functionality (I think we should keep it!), adding more tests, changesets, and some files that have been changed and probably don't need to be anymore (package.json, spec-ext).

On the changed functionality: This might have been hashed out in discord/convos, but I am a fan of the list item transform which this PR is planning to remove. One reason I like it is that the attribution doesn't require a new line, and in both cases we are interpreting the final markup using this transform, in one case to turn a paragraph into an attribution caption and in the other to turn a single list item into an attribution. I don't really see the difference, and it is nice that we can do this:

> We know what we are, but know not what we may be.
> - Hamlet act 4, Scene 5

Rather than:

> We know what we are, but know not what we may be.
>
> -- Hamlet act 4, Scene 5

I foresee challenges when people forget that there needs to be an extra new line, which means there is only one paragraph. Some of that can be addressed potentially if we take over the body content in the parsing of the epigraph/pull-quote directives (where we are explicitly expecting an attribution, and can change the parsing).

Regardless of the choice, this PR should update the docs here:

https://github.com/executablebooks/mystjs/blob/main/docs/typography.md#L112

Testing

I would also love to see a few more tests around the regexp, expanding it to endash, I think there is a bug when it isn't the start of the line (e.g. blah --- attribtuion), and ensuring we return false if there is no actual attribution. It would also be good to add some tests for the directive parsing specifically.

There should also be some e2e test (or at least exploratory conformation) that this doesn't clash with the smart typography, which turns -- into – (endash), in which case, expanding the regexp into --- \u2014- meaning three dashes will always fail currently.

The directive docs/references should also be added to include the two new directives:

https://github.com/executablebooks/mystjs/blob/main/docs/directives.md#L2

Before merge, should add changesets!

[rowanc1]

Somewhere along the way we have lost adding the class quote on the figure[kind=quote] node. Adding this back in makes the CSS work again:

I think this actually might be on the theme and putting the figure.kind in at the start of the classes. The bug was introduced here:

jupyter-book/myst-theme#277

[agoose77]

On the changed functionality: This might have been hashed out in discord/convos, but I am a fan of the list item transform which this PR is planning to remove. One reason I like it is that the attribution doesn't require a new line, and in both cases we are interpreting the final markup using this transform, in one case to turn a paragraph into an attribution caption and in the other to turn a single list item into an attribution. I don't really see the difference, and it is nice that we can do this:

I see the convenience of using - attribution syntax for attributions. If it were just a matter of syntax, I'd be easily won in either direction. However, there are three factors that favour the -- variant for me:¹

• Existing implementation: Sphinx, Asciidoctor(? https://github.com/arquillian/continuous-enterprise-development/blob/master/ch01.asciidoc), and others seem to use emdash/--. We are currently explicitly deviating from that
• Convention: non-parsed Markdown seems to reflect the common usage in written prose that prefers -- or emdash —
• Fragility: it's much more common (assertion, no data) to see list items in quotes than -- / — as trailing items. We would break these usages. Though, single list items are probably not too common either.

Some of that can be addressed potentially if we take over the body content in the parsing of the epigraph/pull-quote directives (where we are explicitly expecting an attribution, and can change the parsing).

I had this thought too. We could also introduce an explicit role, e.g.

> We know what we are, but know not what we may be.
> {attrib}`Hamlet act 4, Scene 5`

I'm not saying that I love that, just that it's a member of the phase space ;)

Regardless of the choice, this PR should update the docs here:

Yep, TODO.

Footnotes

1. I had an additional point in mind here, but stepped away from the keyboard and promptly forgot it. So, maybe there are four points! ↩

[rowanc1]

I think only major comment left is picking up the options in the directives (and adding a test!).

Before merge:

• We should also finalize/update the docs in typography.md.
• changeset!

[rowanc1]

On the functionality change, thanks for explaining your thinking - happy on my side to be taken in this direction. :)

We should make an explicit note in the docs updates to remember to add a newline.

Note that updating the docs will take a release, theme release, and then website release to consume the theme (in the live example).

[rowanc1]

Nice to see the consolidation into one directive. What do you think of naming this blockquote (no dash?). Can alias as well? Maybe I am too used to reading html source.

[rowanc1]

I think this should be blockquote (maybe aliased to block-quote?).

[agoose77]

Hmm. I'm on the fence. I like that it reflects symmetry with pull-quote, but of course the internal AST node is blockquote. You make the call!

[rowanc1]

Is this handled in the myst-spec updates?

[agoose77]

No! Actually now I understand where my confusion came from. Let me fix that up.

[agoose77]

@rowanc1 this is ready for handover!

[agoose77]

🎆