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

fix pygments style in docs #535

Merged
merged 1 commit into from
May 19, 2018
Merged

Conversation

sorcio
Copy link
Contributor

@sorcio sorcio commented May 16, 2018

Fixing pygments_style according to readthedocs/sphinx_rtd_theme#633 (comment)

@sorcio
Copy link
Contributor Author

sorcio commented May 16, 2018

AppVeyor Python35-x64 Windows build failed (see logs):

pip install codecov
Collecting codecov
  Downloading https://files.pythonhosted.org/packages/8b/28/4c1950a61c3c5786f0f34d643d0d28ec832433c9a7c0bd157690d4eb1d5f/codecov-2.0.15-py2.py3-none-any.whl
Requirement already satisfied: coverage in c:\python35-x64\lib\site-packages (from codecov)
Collecting requests>=2.7.9 (from codecov)
  Could not find a version that satisfies the requirement requests>=2.7.9 (from codecov) (from versions: )
No matching distribution found for requests>=2.7.9 (from codecov)
You are using pip version 9.0.1, however version 10.0.1 is available.
You should consider upgrading via the 'python -m pip install --upgrade pip' command.

Closing/reopening to see if the problem magically goes away.

@sorcio sorcio closed this May 16, 2018
@sorcio sorcio reopened this May 16, 2018
@codecov
Copy link

codecov bot commented May 16, 2018

Codecov Report

Merging #535 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #535   +/-   ##
=======================================
  Coverage   99.28%   99.28%           
=======================================
  Files          89       89           
  Lines       10483    10483           
  Branches      726      726           
=======================================
  Hits        10408    10408           
  Misses         58       58           
  Partials       17       17

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 82ec7c7...7c0f924. Read the comment docs.

@smurfix
Copy link
Contributor

smurfix commented May 16, 2018

You could just go to appveyor and click the Rebuild button …

@sorcio
Copy link
Contributor Author

sorcio commented May 16, 2018

@smurfix right. If I remember correctly though @njsmith suggested to close/reopen in order to trigger a new build, so the previous build logs are not lost and the issue is documented somewhere. See the link in my comment above, which is still working after the rebuild.

@smurfix
Copy link
Contributor

smurfix commented May 16, 2018

@sorcio Actually that turns out not to be an issue with Appveyor – they keep the logs of superseded attempts around and assign new build IDs.

I'll ask the Travis people to please do the same thing.

@njsmith
Copy link
Member

njsmith commented May 19, 2018

That appveyor failure makes no sense :-(. If it was on pip 10, and on MacOS, it'd be #508, but in this case I have no idea :-(. Well, I'll add it to #200 anyway...

I also don't understand at all what's going on with RTD and sphinx and pygments and why this patch would fix anything, but I agree that people are claiming that it helped them and it seems harmless otherwise, so I guess let's merge it and see what RTD does.

@njsmith njsmith merged commit 6d434dc into python-trio:master May 19, 2018
@njsmith
Copy link
Member

njsmith commented May 19, 2018

I'm looking at trio.readthedocs.io now, and it doesn't look like highlighting is fixed :-(

@sorcio sorcio deleted the fix-pygments-style branch May 20, 2018 14:30
@sorcio
Copy link
Contributor Author

sorcio commented May 21, 2018

Nope :( Maybe we should revert this but I have no idea what I'm doing.

I think the issue is still the missing CSS. _static/pygments.css maybe? It's there but it's not included in the generated HTML document. But it's included unconditionally in the theme (https://github.com/rtfd/sphinx_rtd_theme/blame/master/sphinx_rtd_theme/layout.html#L47) and it's been since 0.3.0. Does that mean RTD is building docs with a very old version of the theme? But Trio explicitly includes the latest theme version (https://github.com/python-trio/trio/blob/master/ci/rtd-requirements.txt) which as of now is 0.3.1 on PyPI.

Other updates to the theme are missing, like alt text on the logo (readthedocs/sphinx_rtd_theme@ed48532#diff-1336f80431f476a232e3c48abb277c51) so I'm pretty sure that we're still using an older theme version. I don't know why yet. I don't have a clear memory of this: did syntax highlighting ever work? I will investigate further later.

@sorcio
Copy link
Contributor Author

sorcio commented May 21, 2018

Ah, now I get it. rtd-requirements.txt is installed after RTD installs its requirements. At that stage sphinx-rtd-theme<0.3 is installed, so when it comes to our local requirements an unspecified sphinx-rtd-theme is already satisfied and therefore nothing is installed. We can override this by pinning a more specific version (e.g. >=0.3,<0.4 or ==0.3.1). Or, even better, just ignore it altogether because RTD has already merged the fix that will change the default theme version to <0.4 and it's a matter of days before it gets deployed and our theme will automatically be updated next time we build documentation.

@sorcio
Copy link
Contributor Author

sorcio commented May 22, 2018

RTD deployed with the updated theme and now we have syntax highlighting! 🎉

BTW this possibly means that we can do away with some hacks and fixes (but I'm not going to look into that right now)

@njsmith
Copy link
Member

njsmith commented May 23, 2018

Well, I guess it's nice when ignoring a problem makes it go away :-) Thanks for following up!

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.

3 participants