executablebooks / sphinx-tabs

Tabbed views for Sphinx
https://sphinx-tabs.readthedocs.io
MIT License
263 stars 67 forks source link

Make the tests pass with Sphinx 7.1 #178

Closed mitya57 closed 10 months ago

mitya57 commented 11 months ago

The tests were failing with this diff:

-<document source="index.rst">
+<document source="index.rst" translation_progress="{'total': 0, 'translated': 0}">

See https://bugs.debian.org/1042589 for the full log.

This was caused by sphinx-doc/sphinx#11509, which is part of Sphinx ≥ 7.1.0.

welcome[bot] commented 11 months ago

Thanks for submitting your first pull request! You are awesome! :hugs:
If you haven't done so already, check out EBP's Code of Conduct and our Contributing Guide, as this will greatly help the review process.
Welcome to the EBP community! :tada:

foster999 commented 11 months ago

Thanks for raising the PR ☺️ we've had previous requests for supporting translation, so I wanted to check whether this change will prevent this in future? We would generally try to support new sphinx features rather than disable them

mitya57 commented 11 months ago

I disabled this feature only in tests, it won't be disabled for sphinx-tabs users.

When you decide to drop support for old Sphinx versions (< 7.1), you can revert this change and update the test expectations instead.

foster999 commented 11 months ago

I think it would be best for the tests to reflect the latest sphinx version. We should be able to fix this by regenerating the regression tests under a normal test run

mitya57 commented 11 months ago

But then the tests will fail with older Sphinx.

Although, I can put some .replace() in Python code to make it work with old Sphinx too. Will that work for you?

mitya57 commented 11 months ago

I have implemented the approach from my last comment now.

welcome[bot] commented 10 months ago

Congrats on your first merged pull request in this project! :tada: congrats
Thank you for contributing, we are very proud of you! :heart: