review request: Ivy theme

[gvwilson]

I've created an Ivy theme called mccole in https://github.com/gvwilson/theme/, and would be very grateful for feedback. In particular, there are several places in the extensions where I check to see if the node argument is empty, and if so, bail out; this tells me that I'm triggering on the wrong event, but I don't know which one(s) to watch for instead.

Similarly, the theme creates lookup tables under ivy.site.config["mccole"] for its own use; in several places one extension checks to see if another extension has added information there yet or not, and if not, bails out - my instinct is that shouldn't be necessary if I listen for another event.

Finally:

1. I tried to get attribute lists working with tables, but the Markdown parser doesn't seem to recognize them (see the example at the top of lib/mccole/extensions/tables.py). If there's a way to do this, I'd be very happy to rewrite the extension.
2. I'm using a regular expression to find headings in pages rather than running the Markdown parser and searching the token stream (partly because I suspect it's faster, but also because I'm not familiar with Markdown's parsing cycle). If there's an easier way to do this, I'd be happy to rewrite to use it.

And thanks for documenting Ibis and shortcodes - they were easy to figure out.

p.s. it's my intention to use this theme (with better CSS) for https://teachtogether.tech/, https://stjs.tech/, and https://buildtogether.tech/ - all of the features are driven by those books' needs. I'm speaking with the https://pagedjs.org/ folks later this week to see about HTML-to-PDF generation as well.

[dmulholl]

This looks really interesting.

Can you link me to the places where you're having to check if the node argument is empty? I just spotted one inside a shortcode handler -- it should be a straight up bug if that ever gets called with an empty node argument. It shouldn't even be possible as shortcodes are only processed at a single point inside a Node method and the parser gets passed the method's self argument as the context object. I would definitely be interested in tracking the cause of that down if it's happening!

I'm not an expert on the Markdown library I'm afraid as I rarely use it myself. When I first built Ivy that was the most popular Markdown library available but I think there's quite a selection now, including multiple Python implementations of CommonMark:

https://github.com/commonmark/commonmark-spec/wiki/List-of-CommonMark-Implementations

You might find one of them suits your project better. If you register your own rendering engine handler for .md files, that will override the default handler so you can use any Markdown library you like.

[gvwilson]

Here are two places where I had to insert an if not node check:

• https://github.com/gvwilson/theme/blob/main/lib/mccole/extensions/contents.py#L31
• https://github.com/gvwilson/theme/blob/main/lib/mccole/extensions/excerpts.py#L70

My homebrew precursor to Ivy+this theme used markdown-it-py as a Markdown parser; it generates a token stream, but it doesn't handle attribute lists, which I want to use so that I can add IDs to headings. I'm OK using regular expressions to match headings for now - it keeps the build nice and quick.

[dmulholl]

That's very mysterious, it should be impossible for a shortcode handler to ever get called with an empty node argument. Let me know if you can narrow down the triggering conditions.

[gvwilson]

Sure - if you grab the current state of https://github.com/gvwilson/theme (main branch) and run ivy build in the root directory, it creates output in docs. If you set a breakpoint or add a print statement in contents.py or excerpts.py in lib/mccole/extensions right before the check for an empty node, you'll see that it's [] the first time the function is called (hence the check).

[dmulholl]

I've spotted the issue here:

https://github.com/gvwilson/theme/blob/091c9f5dc9865c3fd3c718d05b56a1007f7e36b5/lib/mccole/extensions/figures.py#L76

It's a weakness in the shortcodes library itself which expects to be run in either 'global registration' mode or 'per-parser registration' mode but not both simultaneously.

Your custom shortcodes.Parser instance is inheriting the full set of globally-registered shortcodes, including all the ones that expect to be passed a Node instance when parsed from Ivy.

I've added an inherit_globals flag to the parser in Shortcodes v5.2.0 that lets you create a clean-slate parser instance:

parser = shortcodes.Parser(inherit_globals=False)

The global registration decorators were always a trade-off leaning heavily on the convenience side of the equation -- classic case of global state causing problems!

[gvwilson]

Thanks for the fix, but that seems to trigger another problem.

1. pip install shortcodes==5.2.0 (with Ivy 6.0.4).
2. Run ivy build in my theme repo's root directory: all good.
3. Modify line 72 of lib/mccole/extensions/figures.py to construct parser = shortcodes.Parser(inherit_globals=False).
4. Run ivy build: fail with a message about an unrecognized shortcode contents (which generates a table of contents). Stack trace below; looks like parser that's looking for figures isn't inheriting the contents shortcode (because I turned off inherit_globals), so it can't handle shortcodes that the full parsing cycle takes (?).

   Traceback (most recent call last):
   File "/miniforge3/envs/ivy/bin/ivy", line 8, in <module>
   sys.exit(main())
   File "/miniforge3/envs/ivy/lib/python3.9/site-packages/ivy/__init__.py", line 57, in main
   events.fire('init')
   File "/miniforge3/envs/ivy/lib/python3.9/site-packages/ivy/events.py", line 35, in fire
   func(*args)
   File "/theme/lib/mccole/extensions/figures.py", line 53, in collector
   ivy.nodes.root().walk(lambda node: _collect(node, collected))
   File "/miniforge3/envs/ivy/lib/python3.9/site-packages/ivy/nodes.py", line 123, in walk
   node.walk(callback)
   File "/miniforge3/envs/ivy/lib/python3.9/site-packages/ivy/nodes.py", line 124, in walk
   callback(self)
   File "/theme/lib/mccole/extensions/figures.py", line 53, in <lambda>
   ivy.nodes.root().walk(lambda node: _collect(node, collected))
   File "/theme/lib/mccole/extensions/figures.py", line 76, in _collect
   parser.parse(node.text, seen)
   File "/miniforge3/envs/ivy/lib/python3.9/site-packages/shortcodes.py", line 208, in parse
   raise ShortcodeSyntaxError(msg)
   shortcodes.ShortcodeSyntaxError: Unrecognised shortcode tag 'contents' in line 2.

[dmulholl]

That's actually expected behaviour -- the shortcode parser will throw an exception if it finds a shortcode with an unregistered tag.

If you want to parse two different sets of shortcodes in two different contexts in the same source text, you'll have to define two different syntaxes for them. The default syntax used by the shortcode library (and so by Ivy) is [% foobar ... %] but you can specify custom delimiters for your shortcodes in the Parser() constructor -- you could use something like (% foobar ... %) so the two different sets of shortcodes don't interfere with each other. (You can make the delimiters anything you like.)

[gvwilson]

Please see https://github.com/dmulholl/shortcodes/pull/8

[dmulholl]

Closing as I think we figured this one out with the shortcodes library updates. Feel free to re-open if any issues persist.