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

Pandoc Markdown Writers #302

Merged
merged 23 commits into from
Dec 7, 2023
Merged

Pandoc Markdown Writers #302

merged 23 commits into from
Dec 7, 2023

Conversation

has2k1
Copy link
Contributor

@has2k1 has2k1 commented Oct 23, 2023

Ref: #247

In trying to get this nice and well done it has become bigger than I envisioned but I think it will be worth it.

The main source of the "scope creep" has been need to come up with a systematic way of generating markdown for pandoc elements. The benefits are Renderers do less funky string manipulations and that uniform markup is generated. This is the reason for the pandoc sub-package modelled after module-pandoc that underpins the lua-filters and the pandoc types.

Relate to the above, I have noticed some kind of duplication/conflict in the interlinks module. I have not looked into it, but in the end this should be harmonised.

In trying to ensure that we create valid markdown markup, we should rethink the use of backticks for intermediate links created for the interlink filter. i.e. [title](`target`). As backticks are markup for code snippets, using them for a second purpose means that we create markup that may be confusing. An alternative is [title](:target:).

This gist has code that you may use to test out this PR.

@machow
Copy link
Owner

machow commented Oct 23, 2023

Thanks for this heroic PR! The new pandoc submodule seems like a useful layer (I can see how it cleans up renderer code!). Let me know if / when I can be helpful!

Note that quartodoc uses syrupy for snapshot tests, which should allow you to check bigger renderer outputs. All these tests with the snapshot fixture use syrupy to test rendering different things!

@has2k1 has2k1 force-pushed the numpydoc-renderer branch from e179fb9 to ac2a1bb Compare November 2, 2023 06:56
@machow
Copy link
Owner

machow commented Nov 8, 2023

Thanks for working on this--let me know if there's anything I can do to help with the plotnine docs (linking below so I can find easily). Happy to help!

has2k1/plotnine#706

Once the plotnine docs are deployed, I'm happy to work on refactoring the renderer back into quartodoc. I think it might be a bit tricky because it inherits the base renderer (vs this commit, which used MdRenderer), but I don't mind spending time refactoring the common parts back into the base Renderer. This numpydoc renderer is a super great addition!

@machow
Copy link
Owner

machow commented Dec 1, 2023

@has2k1 I'm on board with merging in the pandoc work, and then a separate PR for the renderer, if that's useful. (We can also modify the pandoc work in the later PR as needed).

WDYT? If you're able to add tests for the pandoc module (say in tests/test_pandoc.py), or something similar, then seems useful to get merged!

@has2k1
Copy link
Contributor Author

has2k1 commented Dec 1, 2023

@has2k1 I'm on board with merging in the pandoc work, and then a separate PR for the renderer, if that's useful.

I will do that.

@has2k1 has2k1 force-pushed the numpydoc-renderer branch from 17f811b to 2d02ced Compare December 4, 2023 12:51
@has2k1 has2k1 force-pushed the numpydoc-renderer branch from 2d02ced to fc30dfc Compare December 4, 2023 12:53
@has2k1
Copy link
Contributor Author

has2k1 commented Dec 4, 2023

Now, this PR only adds the pandoc sub-package plus tests.

@has2k1 has2k1 changed the title WIP: Numpydoc renderer Pandoc Markdown Writers Dec 4, 2023
@has2k1 has2k1 mentioned this pull request Dec 4, 2023
This reduces the cases were Inlines is needed.
 This reduces the cases were Blocks is needed.
@machow machow self-requested a review December 7, 2023 20:40
@machow
Copy link
Owner

machow commented Dec 7, 2023

This looks great--thanks for taking the time to add this, and for all the tests!

@machow machow merged commit e7eb12d into machow:main Dec 7, 2023
5 of 6 checks passed
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.

2 participants