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

html output: Line numbers misalign with code lines (since sphinx 3.1.0, theme independent) #8254

Closed
jonascj opened this issue Sep 29, 2020 · 15 comments

Comments

@jonascj
Copy link

jonascj commented Sep 29, 2020

Describe the bug
In Sphinx 3.1.0 and later versions the line numbers of code blocks misalign with the code lines they number, as shown in the screenshots below. Version 3.0.4 aligns them properly like older versions 2.x and 1.x. Tested for both the alabaster and classic theme.

To Reproduce
Steps to reproduce the behavior:

pip install sphinx==3.1.0

sphinx-quickstart

cat >> index.rst << EOL

.. code-block:: python
    :linenos:

    import math
    for x in range(0,10):
        print(math.sqrt(x))
EOL

make html

firefox _build/html/index.html

Expected behavior
The line numbers and code lines should align as in the second screenshot (from sphinx v. 3.0.4).

Your project
A MWE is attached (make html with sphinx v. 3.1.0 to see the problem)

lineno-misalign-bug.zip

Screenshots
scrot_2020-09-29_133401_746x561

scrot_2020-09-29_132448_753x597

Environment info

  • OS: Linux
  • Python version: 3.7.4
  • Sphinx version: 3.0.4, 3.1.0, 3.2.1
  • Extra tools: a browser to view html output
@jonascj
Copy link
Author

jonascj commented Sep 29, 2020

It basically appears to be 7px of top and bottom padding which is missing from the <pre> tag which holds the line numbers, or another vertical centering mechanism (present in 3.0.4 but not in 3.1.0).

The below screenshots hint that the <pre>-element containing the line numbers is not equal in height to the <pre> containing the code. The height of the two <td>-elements (containing line numbers and code respectively) have equal heights, so the line number's <pre>-element is somehow vertical centered within <td class="linenos">-element. That vertical centering mechanism/rule is present in 3.0.4 but missing in 3.1.0 as far as I can see...

scrot_2020-09-29_225630_1598x878

However it would be prettier to have the two <pre> elements (code and line numbers) have the same height by giving them equal top and bottom padding (which also solves the alignment issue).

Something like this fixes it, but of course it should be fixed elsewhere

/* custom.css */
td.linenos pre, td.code pre {
    padding-top: 7px;
    padding-bottom: 7px;
}
 

_build.zip

@jonascj jonascj changed the title Line numbers misalign with code lines (since sphinx 3.1.0, theme independent) html output: Line numbers misalign with code lines (since sphinx 3.1.0, theme independent) Sep 29, 2020
@mgeier
Copy link
Contributor

mgeier commented Oct 18, 2020

This is probably somewhat related to my PR #7482, but I think it is a bit more complicated.

There is a rule from pygments.css:

td.linenos pre {
    padding: 0 5px 0 5px;
}

If you disable this, it looks fine in the classic theme, and the vertical alignment is fine in the alabaster theme, but the horizontal spacing becomes odd ...

I don't know yet how to fix this, I'll try to look into it. If you find anything new, let me know!

@mgeier
Copy link
Contributor

mgeier commented Oct 18, 2020

FYI, starting with Sphinx 4, html_codeblock_linenos_style will be by default switched to 'inline' and in Sphinx 6 the current style will not be available anymore, see #7942.

So we should probably concentrate on fixing the 'inline' style ...

@mgeier
Copy link
Contributor

mgeier commented Oct 18, 2020

OK, it looks like on top of my changes #7942 [correction: #7482] to Sphinx there have also been changes to Pygments (which happened after my changes, that's why I didn't notice any problems at the time!):

pygments/pygments#1477

I think this is a bug in Pygments (@jonascj would you like to create an issue there?), but we might also need to change a few things on the Sphinx side, I don't know.

As a work-around, you can use Pygments 2.6.1, which is the last working version:

python3 -m pip install "pygments<2.7"

Again, this leads to some strange horizontal spacing in the alabaster theme, but the vertical alignment should be good.

Since alabaster doesn't seem to be maintained anymore, you might want to consider switching theme ... might I suggest my own one: https://insipid-sphinx-theme.readthedocs.io/?

@jonascj
Copy link
Author

jonascj commented Oct 18, 2020

@mgeier Good to know about the table vs. inline style in Sphinx 4 and Sphinx 6. A table indeed seems like an outdated way of doing the line numbers. I'll look into the inline style/method. Is Sphinx using the raw pygments html output (i.e. pygments is generating the table with line numbers)?

I might well change theme on my project, but as you also mentioned yourself the issue isn't isolated to alabaster.

@mgeier
Copy link
Contributor

mgeier commented Oct 19, 2020

Is Sphinx using the raw pygments html output (i.e. pygments is generating the table with line numbers)?

I don't know, but let's look at the source code, shall we?

It looks like the HTML is generated here:

highlighted = self.highlighter.highlight_block(
node.rawsource, lang, opts=opts, linenos=linenos,
location=node, **highlight_args
)
starttag = self.starttag(node, 'div', suffix='',
CLASS='highlight-%s notranslate' % lang)
self.body.append(starttag + highlighted + '</div>\n')

And here for the old HTML writer:

highlighted = self.highlighter.highlight_block(
node.rawsource, lang, opts=opts, linenos=linenos,
location=node, **highlight_args
)
starttag = self.starttag(node, 'div', suffix='',
CLASS='highlight-%s notranslate' % lang)
self.body.append(starttag + highlighted + '</div>\n')

The highlighter is assigned here:

self.highlighter = PygmentsBridge('html', style)

Which leads us to the call to Pygments:

hlsource = highlight(source, lexer, formatter)

This is using pygments.highlight and the pygments.formatters.HtmlFormatter, which seems to take the linenos argument (which seems to include the html_codeblock_linenos_style setting).

So I guess the answer is "yes", the HTML is generated by Pygments and Sphinx just includes it as it is.

But there's also CSS, which I think is actually the source of the problem.

The CSS comes from pygments.css, which seems to be written there:

with open(path.join(self.outdir, '_static', 'pygments.css'), 'w') as f:
f.write(self.highlighter.get_stylesheet())

This uses the same highlighter and formatter from before to get the CSS definitions from Pygments:

return formatter.get_style_defs('.highlight')

I hope this helps!

[...] but as you also mentioned yourself the issue isn't isolated to alabaster.

That's true, but I was talking about the work-around of using pygments<2.7. This seems to (at least temporarily) fix the problem for most themes, but the alabaster theme ends up having quite awkward horizontal spacing.

@mgeier
Copy link
Contributor

mgeier commented Oct 21, 2020

I've created a Pygments issue: pygments/pygments#1579.

@jonascj
Copy link
Author

jonascj commented Oct 21, 2020

@mgeier Cheers, it was enlightening, I didn't mean to force you to look at the source (I could have done that myself), I meant to ask you if you knew the answer off the top of your head.

Also, fine work on the pygments issue.

My personal interest in this is temporarily quite low, since I might change theme from alabaster and the HTML5 formatter (with the inline option/solution) might not suffer from the issue. So I might keep searching for more than a work-around, I might not, for now.....

@Anteru
Copy link

Anteru commented Oct 21, 2020

Just for my understanding: It works with Pygments 2.6 and 2.7 in Sphinx 3.0, but it's broken with Pygments 2.7 in 3.1 -- however Pygments 2.6 works in 3.1?

@mgeier
Copy link
Contributor

mgeier commented Oct 22, 2020

That's a good question, but the answer isn't quite as straightforward.

Most likely all combinations are broken.

It works with Pygments 2.6 and 2.7 in Sphinx 3.0

Well, not quite, that would be too easy.

In Sphinx 3.0 (with Pygments 2.6), the main code block doesn't extend to the right margin. It's just as wide as the contained code. That's one of the problems I wanted to fix with #7482.
Other than that, it looks right.

Switching to Pygments 2.7 (but staying with Sphinx 3.0) changes two things (AFAICT), one might say they are "broken":

  • The vertical margin of the <pre> element vanishes, leading to insufficient distance from the surrounding elements.
  • The colors of the line numbers are suddenly not defined by the Sphinx theme anymore, but Pygments takes over. That may be intentional, but I would consider it a breakage, because it goes against the intention of the theme authors.

The vertical alignment of line numbers seems to be fine with both Pygments 2.6 and Pygments 2.7.
I guess this is because the table cells are centered vertically?

[...] but it's broken with Pygments 2.7 in 3.1

Yes. To be a bit more concrete, the vertical alignment of the line number is now off (that's what this issue is all about).
But also the colors of the line numbers are changed against the intention of the theme author, same as mentioned above.
The width of the main code block is now correct, though!
And the margin of the code block (the whole thing including line numbers) is now also correct.

however Pygments 2.6 works in 3.1?

Pretty much.
There is just a little problem with the alabaster theme (but not with others I've tried): While the vertical spacing is good, the horizontal spacing is now larger than intended by the theme author.
This is a bit ugly, but IMHO it's the least problematic error of those mentioned in this issue.
And it affects only one specific theme.


I tried to see what's going on ...

The good news: the HTML is unchanged between versions:

<div class="highlight-python notranslate"><table class="highlighttable"><tbody><tr><td class="linenos"><div class="linenodiv"><pre>1
2
3</pre></div></td><td class="code"><div class="highlight"><pre><span></span><span class="kn">import</span> <span class="nn">math</span>
<span class="k">for</span> <span class="n">x</span> <span class="ow">in</span> <span class="nb">range</span><span class="p">(</span><span class="mi">0</span><span class="p">,</span><span class="mi">10</span><span class="p">):</span>
    <span class="nb">print</span><span class="p">(</span><span class="n">math</span><span class="o">.</span><span class="n">sqrt</span><span class="p">(</span><span class="n">x</span><span class="p">))</span>
</pre></div>
</td></tr></tbody></table></div>

The CSS does change indeed:

In Sphinx 3.0, basic.css looks like this:

table.highlighttable td {
    padding: 0 0.5em 0 0.5em;
}
td.linenos pre {
    padding: 5px 0px;
    border: 0;
    background-color: transparent;
    color: #aaa;
}

The vertical padding doesn't really have an effect, since the table cells seem to be vertically centered. And lo and behold, it has been removed in Sphinx 3.1.

In Sphinx 3.1, basic.css changes quite a bit:

table.highlighttable tr {
    display: flex;
}
table.highlighttable td.linenos {
    padding-right: 0.5em;
}
table.highlighttable td {
    margin: 0;
    padding: 0;
}
td > :last-child {
    margin-bottom: 0px;
}
td > :first-child {
    margin-top: 0px;
}
div.highlight pre, table.highlighttable pre {
    margin: 0;
}
td.linenos pre {
    border: 0;
    background-color: transparent;
    color: #aaa;
}

I hope I didn't miss some relevant parts.

Most notably, flex has been introduced to be able to fix the width of the main code block.
I don't know if there is another way to achieve that, but if you know one, please let me know!

The change from Sphinx 3.0 to 3.1 doesn't break the vertical alignment because there is no padding at all in td.linenos pre. The main pre padding also affects the line numbers, therefore vertical-aligning them correctly.

Pygments 2.6 doesn't affect the padding either. In fact, it doesn't have any CSS for the line numbers!
Which kinda makes sense.
Why should Pygments control the line numbers?
The line numbers should have a background controlled by the Sphinx theme, so Pygments shouldn't interfere.

But that's exactly what Pygments 2.7 does by introducing this CSS:

td.linenos pre {
    color: #000000;
    background-color: #f0f0f0;
    padding: 0 5px 0 5px;
}

This overrides most settings from basic.css, taking over the colors.
But it also sets the vertical padding to 0 (for no reason?).

In Sphinx 3.0, where the table cells are still vertically centered, this isn't really noticed (but that doesn't make it correct).
In Sphinx 3.1, the table cells (now flex contents) are aligned on top, making the wrong padding visible.

I guess we could align the flex contents with align-items: center, but I don't think that's the correct fix.

So, in summary, all combinations are broken, where Pygments 2.6 and Sphinx 3.1 is least broken, only affecting the alabaster theme (which probably could be fixed).

@Anteru
Copy link

Anteru commented Oct 22, 2020

There is a speculative fix in Pygments (see the corresponding bug there for details), if that does work for you, I'll get a 2.7.2 out quickly. Please let me know.

Pygments does control the line numbers though, as they're part of the Pygments output. If I read your comment correctly, Sphinx could put the original

td.linenos pre {
    padding: 5px 0px;
    border: 0;
    background-color: transparent;
    color: #aaa;
}

rule back? That should fix it. In any case, I don't see Pygments stopping emitting CSS rules for linenos, as it's required for Pygments itself, but clearly we should strive to make this work for both sides. I hope the patch is sufficient.

@mgeier
Copy link
Contributor

mgeier commented Oct 23, 2020

If I read your comment correctly, Sphinx could put the original [...] rule back?

It should definitely not put padding: 5px 0px; back, because 5px is generally not the right padding for all themes. It should simply not specify any vertical padding.

For the other stuff, this is already there:

td.linenos pre {
border: 0;
background-color: transparent;
color: #aaa;
}

Sadly, this gets overruled by the Pygments settings.

I guess this is because pygments.css is loaded after the theme CSS and the last CSS rule wins?

I'm not sure what's the correct solution here ...

Probably Sphinx should load pygments.css before the theme CSS?

@Anteru
Copy link

Anteru commented Oct 23, 2020

Yep. The Sphinx rules should come last, or alternatively, use !important to override Pygments. Note that you can use unset to reset settings.

@mgeier
Copy link
Contributor

mgeier commented Oct 24, 2020

The Sphinx rules should come last

I've created a PR for this: #8333.

@Anteru
Copy link

Anteru commented Oct 24, 2020

Pygments 2.7.2 has been released.

@tk0miya tk0miya added this to the 3.3.0 milestone Oct 27, 2020
@tk0miya tk0miya closed this as completed Oct 27, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants