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 support for specifying metadata options in MDX comments #29

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

bunchesofdonald
Copy link

@bunchesofdonald bunchesofdonald commented Sep 13, 2024

Summary

This PR adds support for specifying test metadata in .mdx files using MDX-style comments ({/* pmd-metadata: */}) placed above code blocks. This should support all existing options and allow for easily adding more without having to alter the MDX parsing logic.

Example:

{/* pmd-metadata: notest, fixture:capsys */}
```python
print("hello")
captured = capsys.readouterr()
assert captured.out == "hello\n"
  • Updated documentation with MDX metadata usage.
  • Minor refactoring for readability and metadata extraction.

Issue: #28

@bunchesofdonald
Copy link
Author

@freider or @mwaskom could I get a review of this? I'd really love to use this module, but this is a big blocker for that.

Copy link
Collaborator

@freider freider left a comment

Choose a reason for hiding this comment

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

Hi! Thanks for contributing! Feels related to #26 (a bit annoying that markdown variants haven't settled on a single metadata/comment notation)

As long as it can't accidentally break regular .md specifications I'd be happy to add it. Might want to refactor it slightly to make the parser-specific metadata injection more configurable (or dependent on the .mdx context in this case)

src/pytest_markdown_docs/plugin.py Outdated Show resolved Hide resolved
src/pytest_markdown_docs/plugin.py Outdated Show resolved Hide resolved
tests/plugin_test.py Outdated Show resolved Hide resolved
@freider
Copy link
Collaborator

freider commented Oct 14, 2024

An alternative to having the mdx context propagate in would be to have another pytest marker/arg "--markdown-docs-syntax=mdx-comments" (I just suggested something similar for the superfences syntax that #26 is adding)

@bunchesofdonald
Copy link
Author

bunchesofdonald commented Oct 14, 2024

An alternative to having the mdx context propagate in would be to have another pytest marker/arg "--markdown-docs-syntax=mdx-comments" (I just suggested something similar for the superfences syntax that #26 is adding)

Do you have a preference? I think having it pick up this style from the fact it's parsing an .mdx file makes sense, but if you feel strongly that having an arg is the way to go that also works for me.

Since I think the context would be just a bit friendlier I'll start on that version, but should be easy-ish to change.

@freider
Copy link
Collaborator

freider commented Oct 15, 2024

An alternative to having the mdx context propagate in would be to have another pytest marker/arg "--markdown-docs-syntax=mdx-comments" (I just suggested something similar for the superfences syntax that #26 is adding)

Do you have a preference? I think having it pick up this style from the fact it's parsing an .mdx file makes sense, but if you feel strongly that having an arg is the way to go that also works for me.

Since I think the context would be just a bit friendlier I'll start on that version, but should be easy-ish to change.

The argument for an arg rather than context is that it's more Mintlify-specific than .mdx-specific, but I can see the point for having it as an always-on-thing for .mdx files anyways, just in case there are other renderers than Mintlify that use code fence info blocks for rendered content. Also could be quite useful to have file extension context available in the block extractor anyways, so go for it :)

@bunchesofdonald
Copy link
Author

@freider I've addressed your comments, please have a look at the updated code when you have a moment. Let me know if there are any further adjustments or improvements you'd like me to make.

Thanks again for your guidance!

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