-
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
👌 IMPROVE: Store rawtext in AST nodes (for gettext) #301
Conversation
Codecov Report
@@ Coverage Diff @@
## master #301 +/- ##
==========================================
+ Coverage 90.60% 90.65% +0.05%
==========================================
Files 14 14
Lines 1809 1809
==========================================
+ Hits 1639 1640 +1
+ Misses 170 169 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Actually, the problem is lower-level. I'll open an issue. |
Sorry, nevermind - this branch actually does address #304 (at least for the node types I tested). |
lol yeh I was just going to say it does capture the "bold" syntax etc. you can use https://markdown-it.github.io/ (and the |
Is there any other additions you want to make before merging? |
I see you removed all the changes for list items, etc. Was that intended? |
@chrisjsewell I added a bunch more test cases in https://github.com/jpmckinney/myst-parser-tests and they all extract correctly now with only the changes to these two lines. I can still add a table test case, and maybe some other block-level elements I missed. I don't really know how to test this without looking in Sphinx's code to see what it does when the gettext builder is run. |
I've now added a table test case, and it extracts incorrectly, so I'll add another commit. |
The new commit fixes terms ( |
I think we should add a test for this, to https://github.com/executablebooks/MyST-Parser/tree/master/tests/test_sphinx/sourcedirs:
|
thats great thanks |
If you have Markdown like:
Sphinx needs to be passed
My **Awesome** markdown
.The code does not do that.
This is the same bug as in recommonmark: readthedocs/recommonmark#187 which had originally been fixed by a Sphinx maintainer, and which I more recently fixed (though recommonmark is very behind on merging PRs).
As you can see in the code for paragraphs, sometimes the original Markdown content is not where you expect it. So far, I've only tested a few tokens.
cc @choldgraf