chore: Automatically generate example format tabs#4959
chore: Automatically generate example format tabs#4959bradenhilton merged 1 commit intotwpayne:masterfrom
Conversation
|
Just to clarify, I'm waiting for a bit of feedback on this (whenever anyone has time) before I proceed any further. As always, there's no rush. |
twpayne
left a comment
There was a problem hiding this comment.
This looks excellent to me. I particularly like that this is done inside the material-mkdocs framework.
Another nice feature is that when you choose the format, all examples in the same page change to use that format.
Great work @bradenhilton!
|
I've decided to not convert any blocks that contain template syntax, since It's non-trivial to manipulate the TOML text to work around this if the TOML is comprised of sections with template syntax between them, or if it contains sequences like At a minimum, you would probably need to do something like:
(there's probably a smarter/more elegant way to do this (perhaps a way to do it in a single pass or something), but the idea is essentially the same) It probably can be solved, but in these cases it's probably far easier to just manually write the examples, especially now that the basic examples are done automatically. Also, parsing most TOML blocks acts like a linter of sorts. I actually found a subtle quoting error on one of the blocks because it wouldn't parse correctly. |
|
It can enforce manual conversions if it can't be done automatically, so we can make sure all examples are available in all versions. |
|
I've made a first pass at changing examples. I searched the site directory for I also added some temporary code that outputs all modified markdown files to an |
| # We allow capturing *.toml.tmpl filename titles but do not support | ||
| # converting examples containing any template syntax. |
There was a problem hiding this comment.
I should probably update this to clarify that things like template syntax within values is fine, but at the moment we don't support things like template syntax around or in-between blocks of TOML
| return textwrap.indent(examples, indent) | ||
|
|
||
| new_md = example_pattern.sub(replace, markdown) | ||
| if new_md != markdown: |
There was a problem hiding this comment.
This can be useful for debugging, hide behind env var?
There was a problem hiding this comment.
I don't disagree, but it's outside the scope of this PR, and temporary environment variables don't have great ergonomics on Windows.
Perhaps we can integrate it into taskipy (or some other task runner), but I won't add it to this PR.
cc @halostatue, who is more familiar with taskipy
There was a problem hiding this comment.
I'd have to investigate.
There was a problem hiding this comment.
Not sure I get your "outside the scope" argument. This is debug feature which needs to be disabled or removed before merging. I think it makes sense to keep it and activate by CLI flag (e.g. --debug) or env var.
There was a problem hiding this comment.
Adding this feature will likely require using an env var directly inside a task runner, as I alluded to in #4959 (comment). The feature is also potentially useful for other kinds of transformations, not just the one specific to this PR.
If it can't be done with taskipy, that means we need to potentially switch to a different task runner (if we can find one that works). I don't want to do either that or manual handling of an env var in this PR because I am currently using Windows, and temporary env vars for a single command are beyond tedious unless you use a sane operating system.
The debug output code is temporary and will be removed before this PR is merged.
| return textwrap.indent(examples, indent) | ||
|
|
||
| new_md = example_pattern.sub(replace, markdown) | ||
| if new_md != markdown: |
There was a problem hiding this comment.
I'd have to investigate.
| yaml_obj.dump(data, yaml_stream) | ||
| yaml_text = yaml_stream.getvalue() | ||
|
|
||
| json_text = json.dumps(data, indent=4) |
There was a problem hiding this comment.
tomllib.loads may return non-serializable type:
text = tomllib.loads("updated = 2026-03-27")
json.dumps(text)
# TypeError: Object of type datetime is not JSON serializableIt'd be nice to handle such error.
There was a problem hiding this comment.
The TypeError is sufficient, no?
The primary point of this change is to make it easier and less error-prone to provide equivalent examples in multiple formats. If any example contains any types that are not serializable in a different format, it isn't suitable for this transformation.
Tom also said it's fine for errors to blow up in #4959 (comment).
| elif FENCE_CLOSE_RX.match(line): | ||
| state = MarkdownTransformState.IN_BLOCK | ||
| else: | ||
| if line.lstrip().startswith('{{'): |
There was a problem hiding this comment.
Can template be in the middle of the line?
There was a problem hiding this comment.
I believe only template syntax within strings will parse correctly (and also work in the other formats). I am aware that the current approach is naive.
Perhaps it would be better to pull the TOML parsing/validation out into a separate function that is called in the loop, rather than doing it inside the function that also creates the tabs.
| return textwrap.indent(examples, indent) | ||
|
|
||
| new_md = example_pattern.sub(replace, markdown) | ||
| if new_md != markdown: |
There was a problem hiding this comment.
Not sure I get your "outside the scope" argument. This is debug feature which needs to be disabled or removed before merging. I think it makes sense to keep it and activate by CLI flag (e.g. --debug) or env var.
| def on_page_markdown(markdown: str, page: Page, **kwargs) -> str: | ||
| lines = markdown.splitlines() | ||
|
|
||
| new_md = '\n'.join(transform_markdown(lines)) |
There was a problem hiding this comment.
It'd be good to add a training newline here.
| ] | ||
|
|
||
|
|
||
| class MarkdownTransformState(Enum): |
There was a problem hiding this comment.
This is much cleaner than old regexp!
twpayne
left a comment
There was a problem hiding this comment.
This is excellent, thank you very much @bradenhilton for the work and @KapJI and @halostatue for the reviews.
One minor thing I noticed is that example configs don't get highlighted correctly when they're in !!! info boxes, but this is almost certainly an issue with mkdocs-material and doesn't need to be fixed here. The easy fix is probably to simply avoid using so many !!! info boxes.
Please tidy up the commit history and merge it when you feel that it's ready.
Fixes #4954