Codecov Report

Merging #110 (3bf8111) into master (506e69f) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #110   +/-   ##
=======================================
  Coverage   97.16%   97.16%           
=======================================
  Files           2        2           
  Lines         212      212           
=======================================
  Hits          206      206           
  Misses          6        6           

Flags with carried forward coverage won't be shown. Click here to find out more.

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 506e69f...3bf8111. Read the comment docs.

@foster999 could you take a look at the incompatibility with Sphinx 4? Feel free to push to my fork

Thanks for this @Daltz333

I've compared outputs between sphinx 3.5.4 and 4 (pre-release). Looks like the formatting of code-block's with line numbers has changed from:

to:

This seems to be an intentional change to improve accessibility (sphinx-doc/sphinx#7849), so I'll push a new sphinx-version-dependent test for this.

We may want to default html_codeblock_linenos_style to the old style for the user or expose with code-tab

Good point!

I've just tried setting the default to the old style in sphinx-tabs, but it seems like users are then unable to override this in their sphinx config.

Looking at the sphinx changelog, they're depreciating the html_codeblock_linenos_style option too. Given this, and that the new format improves accessibility, I'd be more inclined to follow their lead. We could document that users should use the sphinx option to revert the style (or stick with sphinx 3) and/or bump to a new major version to reflect that sphinx 4 causes a non-backwards compatible change?

Yeah, didn't realize they are deprecating the "table" option. I would do a major version bump for it.

I've pushed my changes, but I'm not sure how to conditionally skip the regression test

Okay, got things figured out.

Duplicated the other_assets test to include the Sphinx 4 generated output. They are conditionally skipped depending on sphinx version.

We probably shouldn't merge this until Sphinx 4 gets a stable release, that way we don't need to pin 4.0.0b1

Release is only 5 days away. I'm going to go ahead and merge it, and we can test Sphinx 4 major/minor versions in a later PR.