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

Allow integer md_header_level #137

Conversation

makukha
Copy link
Contributor

@makukha makukha commented Feb 13, 2025

A simple change fixing #90.

I tried to be as conservative as possible, letting md_header_level remain str internally and be converted from int on input. This change is backwards compatible and allows string values as before.

@nedbat
Copy link
Owner

nedbat commented Feb 17, 2025

Thanks! Can you take a look at the linter failures?

@nedbat
Copy link
Owner

nedbat commented Feb 17, 2025

Also, do you want to make a scriv fragment for the changelog? :)

@nedbat
Copy link
Owner

nedbat commented Feb 17, 2025

BTW, I just fixed the test suite issues, if you rebase on top of master.

@makukha
Copy link
Contributor Author

makukha commented Feb 18, 2025

@nedbat, thank you for the advice! I fixed the lint errors and managed to add some test, as well as the changelog fragment. Please feel free to change this PR, if needed, to match the the project standards. And thank you for the scriv :)

@makukha
Copy link
Contributor Author

makukha commented Feb 18, 2025

It seems that tests failures are not related to my changes

@nedbat
Copy link
Owner

nedbat commented Feb 18, 2025

They are related: you can't test TOML support on a version without toml lib. You need this line from the tests a few above yours:

@pytest.mark.skipif(tomllib is None, reason="No TOML support installed")

@makukha
Copy link
Contributor Author

makukha commented Feb 18, 2025

Yes, my bad, I misinterpreted the tox log. Fixed.

@nedbat nedbat merged commit 111fe3f into nedbat:main Feb 18, 2025
7 checks passed
@nedbat
Copy link
Owner

nedbat commented Feb 18, 2025

Thanks!

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