-
Notifications
You must be signed in to change notification settings - Fork 196
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
✨ NEW: Add Docutils MyST config and CLI #426
✨ NEW: Add Docutils MyST config and CLI #426
Conversation
This is part of executablebooksGH-420, an effort to make myst_parser integrate better with Docutils. * myst_parser/docutils_.py (Parser): Use new _myst_docutils_setting_tuples function to generate a settings_spec. (Parser.parse): Use new create_myst_config function to reconstruct a MdParserConfig object from a Docutils configuration object.
for more information, see https://pre-commit.ci
cheers, I’ll give a proper look soon
It would be very possible to self document the
is it ok if I push directly to your branch? |
Please do, thanks a lot! |
Yep, that would be ideal. Ideally that would also be used to generate the relevant section of the docs, to avoid having two sources of truth. I just didn't find the time to do it ^^ |
Codecov Report
@@ Coverage Diff @@
## master #426 +/- ##
==========================================
- Coverage 90.84% 90.32% -0.52%
==========================================
Files 16 16
Lines 1912 1985 +73
==========================================
+ Hits 1737 1793 +56
- Misses 175 192 +17
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Ok @cpitclaudel, how's this!? Now you have separate CLI commands for writers: and full doc-strings:
I removed the addition of the section level mathjax classes, when using docutils (since this is only useful with I also removed the still to add some extra testing and documentation... |
😍 |
I forget if the |
On the topic of script names, since Docutils uses
|
Yeh, I was hesitant to have this, since it does not make it clear that it will only parse using docutils and not sphinx, e.g. you can't properly parse any files that use sphinx/sphinx-extension roles/directives |
I very much agree with this issue though; it's a shame there is currently no easy "middle ground" between the single document docutils API/CLI, lacking any support for sphinx features, and the sphinx-build CLI, requiring use of the full, complex, project build system |
myst_parser/docutils_.py
Outdated
setting = f"myst_{attribute.name}" | ||
val = getattr(settings, setting, DOCUTILS_UNSET) | ||
with suppress(AttributeError): | ||
delattr(settings, setting) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cpitclaudel why do you need to delete the setting here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(note here I made the getting/deleting optional, since if you include a MyST file from an RST file, the MyST settings will not have been loaded)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I forget the details :/ I think it was to avoid namespace pollution? I'm guessing I was worried about having the settings both directly in the document under myst_*
and in a config object, so I was planning to remove all the myst_*
settings from document.settings
and instead have a single object document.settings.myst_config
.
Of course, this explanation would make more sense if I had then actually stored the config object as document.settings.myst_config
, but the code doesn't seem to do that, so either I forgot or my guess above is wrong ^^
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'll remove it for now then, as it doesn't appear to break anything
If you want to open this as a future improvement issue |
Documentation added 😄 https://myst-parser--426.org.readthedocs.build/en/426/docutils.html |
Ok I think this is good to go! @cpitclaudel do you want to review your own PR lol, and let me know if you are happy for it to be merged |
@cpitclaudel let me know if you happy for this to be merged cheers, eager to get out version 0.16.0 😄 (#426 -> #456 -> #457 -> 🚀) |
Ok times up 😛 but I'm sure its all fine |
Yep! Thanks a lot for moving ahead, I'm terrible at email and super busy these days. Very excited to see things moving in this direction 👍 |
This is a follow-up to my previous PR (GH-418), part of an effort (GH-420) to make myst_parser integrate better with Docutils.
This exposes (almost) all MyST configuration settings in docutils.conf, making it possible to configure myst_parser for use without Sphinx.
I've added a simple entry point to make it easier to experiment (the second commit on this branch). Here's a quick demo:
Remaining issues: