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

➕ Add myst:template directive for documentation #1209

Merged
merged 9 commits into from May 15, 2024
Merged

➕ Add myst:template directive for documentation #1209

merged 9 commits into from May 15, 2024

Conversation

choldgraf
Copy link
Member

@choldgraf choldgraf commented May 11, 2024

There's a hand-coded table of theme options, but the options have changed since it was written. This adds a little code-cell that pulls the latest YAML for each theme's configuration, and inserts it as a pandas table. This PR adds a new myst:template directive that pulls in the template information from https://api.mystmd.org.

It is current pretty simplistic, in a future iteration we could parse the output as markdown but that's blocked on:

This also documents the site.actions and site.nav options, since those exist but aren't documented anywhere.

@choldgraf choldgraf changed the title Add theme options to docs Parse theme options from github and add to docs via a code cell May 11, 2024
@rowanc1
Copy link
Member

rowanc1 commented May 11, 2024

Super cool!

@choldgraf
Copy link
Member Author

Can you think of a way to attach a label to each? Then we could embed each in their own table w a header

@choldgraf
Copy link
Member Author

Oh maybe the easiest would be to split that into a function and call it twice. I can't find a way to attach multiple labels to outputs of a cell.

Makes me wonder if it could be useful to support something like multiple labels comments in a cell, and if found we attach them to cell outputs sequentially

@choldgraf
Copy link
Member Author

choldgraf commented May 11, 2024

Now reusing a function with 3 cells in total, so that we can separate out the themes into sections. I think this is good to go as an incremental improvement...

@rowanc1
Copy link
Member

rowanc1 commented May 11, 2024

If we do it through a plugin, then this could be full, parsed mdast as well. With labels etc.

@choldgraf
Copy link
Member Author

Could you explain more what you mean by a plugin? Parsing these as mdast is exactly what I want. I am just a lot more comfortable with python than JavaScript

@choldgraf
Copy link
Member Author

One more update: I've documented site.actions and site.options.

I had a non-blocking question come up as I was doing this:

What is the difference between site.options.foo, and site.nav + site.actions? Are nav and actions special keys that are treated differently from site.options? Why aren't they nested under site.options?

@agoose77
Copy link
Collaborator

agoose77 commented May 13, 2024

Could you explain more what you mean by a plugin? Parsing these as mdast is exactly what I want. I am just a lot more comfortable with python than JavaScript

An example of a custom plugin used on https://mystmd.org is the myst:directive directive, which is used to provide documentation for directives.

We can use the same approach to implement a theme information role. Using @choldgraf's idea, I'll implement it as a plugin :)

@agoose77 agoose77 changed the title Parse theme options from github and add to docs via a code cell Add myst:theme directive for documentation May 14, 2024
@agoose77 agoose77 changed the title Add myst:theme directive for documentation ➕ Add myst:theme directive for documentation May 14, 2024
@agoose77 agoose77 requested a review from rowanc1 May 14, 2024 06:15
@agoose77 agoose77 changed the title ➕ Add myst:theme directive for documentation ➕ Add myst:template directive for documentation May 14, 2024
Comment on lines +114 to +119
let templates;
try {
templates = await _promise;
} catch (err) {
throw new Error('Error loading template information from https://api.mystmd.org');
}
Copy link
Collaborator

@agoose77 agoose77 May 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure that all concurrent builds have access to this at the same time by setting the global promise before resolving it.

@agoose77 agoose77 requested review from fwkoch and removed request for rowanc1 May 14, 2024 09:05
@choldgraf
Copy link
Member Author

choldgraf commented May 14, 2024

I think that the rendered table looks nice @agoose77 ! What are the benefits of doing this via a mystmd plugin vs. doing this via Python? Is the main improvement that we get a proper structured table? Or is this something we can re-use this plugin for in general? And would this be any different than if we had markdown parsing via #1026 ?

I added a quick commit to document to each of these plugins as examples of plugin syntax, btw. Maybe they'll be a helpful path to follow for people that want to learn how the plugin infrastructure works.

@agoose77 any chance you could add some comments to the plugin so that others could use it as a learning guide?

Either way, I'd suggest that we just merge this in and iterate from there. @agoose77 or @fwkoch want to approve and then we can move forward?

docs/website-templates.md Outdated Show resolved Hide resolved
Copy link
Member

@fwkoch fwkoch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works great, and it's exciting to have another plugin example in the docs (as well as more explicit documentation about them)!

Regarding nav/actions vs options - I don't think there is anything too deep there, it was just a decision about what was common to all sites (top-level keys) vs. theme-specific (nested under options). Other top level keys I think are pretty universal to sites (title, subtitle, thumbnail, favicon, domains...), but I do see it's easy to argue that you could build a theme without nav or actions... I think that's ok, though - it's similar to, like, a tex template - you could make a tex template that ignores frontmatter.authors, but that doesn't mean authors should be an "option" rather than "frontmatter"

We are probably missing more documentation around what can be specified directly on the site... in addition to nav/actions there's quite a bit of other "site frontmatter" https://github.com/executablebooks/mystmd/blob/main/packages/myst-config/src/site/types.ts#L26-L33

@choldgraf
Copy link
Member Author

choldgraf commented May 15, 2024

Thanks for that explanation @fwkoch ! I've added a short explanation to the PR saying that other options might not be documented, and linked to two files that I think are relevant. We can flesh that out over time, but at least now the information is linked somewhere.

I think your reasoning about site.foo vs. site.options.foo makes sense, though I did find it a bit confusing that several top-level keys don't seem to have an impact on the book theme. For example site.banner didn't seem to do anything either.

It would be helpful if we raised a warning any time somebody specified a site.key that was not supported by the current theme.

@agoose77
Copy link
Collaborator

I think that the rendered table looks nice @agoose77 ! What are the benefits of doing this via a mystmd plugin vs. doing this via Python? Is the main improvement that we get a proper structured table? Or is this something we can re-use this plugin for in general? And would this be any different than if we had markdown parsing via #1026 ?

The main benefit is that we can use this plugin across multiple files using a directive -- no need to require an execution context and non-JS dependencies. It also feels more readable because it's a declarative inclusion rather than an imperative "use Pandas to parse options". As we get an AST, that is also nice (even though we can build ASTs from Jupyter outputs)!

I added a quick commit to document to each of these plugins as examples of plugin syntax, btw. Maybe they'll be a helpful path to follow for people that want to learn how the plugin infrastructure works.

Fantastic, what a great idea @choldgraf!

@agoose77 any chance you could add some comments to the plugin so that others could use it as a learning guide?

👍 I'll do that.

@choldgraf
Copy link
Member Author

Sounds good! My feeling is that we can merge this one and iterate from there. @agoose77 do you wanna merge? Otherwise I'll do so later today since @fwkoch already approved

@agoose77
Copy link
Collaborator

Yep, merging now!

@agoose77 agoose77 merged commit 68b9973 into main May 15, 2024
5 checks passed
@agoose77 agoose77 deleted the theme-opts branch May 15, 2024 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants