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

Sphinx - theming #1933

Merged
merged 87 commits into from
Jun 30, 2021
Merged

Sphinx - theming #1933

merged 87 commits into from
Jun 30, 2021

Conversation

AA-Turner
Copy link
Member

@AA-Turner AA-Turner commented Apr 20, 2021

See #2, #1385 for context. Superseeds #1568.

This is the Sphinx theming part, building on PR #1930.

Stylesheets:

  • style.css - styles
  • mq.css - media queries

Jinja2 Templates:

  • page.html - overarching template

Javascript:

  • doctools.js - fixes footnote brackets

Theme miscellany

  • theme.conf - sets pygments styles, theme internals

@AA-Turner AA-Turner requested a review from a team as a code owner April 20, 2021 22:41
@AA-Turner AA-Turner mentioned this pull request May 5, 2021
@AA-Turner AA-Turner force-pushed the sphinx-theming branch 3 times, most recently from 12a1a20 to 3b19ba8 Compare May 7, 2021 20:57
@AA-Turner AA-Turner mentioned this pull request Jun 9, 2021
@AA-Turner AA-Turner force-pushed the sphinx-theming branch 2 times, most recently from 58141f6 to d321101 Compare June 13, 2021 10:00
@AA-Turner AA-Turner requested a review from pfmoore as a code owner June 13, 2021 10:00
@AA-Turner AA-Turner force-pushed the sphinx-theming branch 2 times, most recently from dab95a4 to a1d49b7 Compare June 13, 2021 10:30
@AA-Turner
Copy link
Member Author

@pablogsal This one is now ready to review -- I do still have a way of producing visual parity styling with the current hosting, but it will take a couple hours to update if wanted.

Test: make sphinx-local and view locally. Should be styled, and should also build faster.

Note that in this PR (unlike all previous ones) I have edited 3 PEP files (310, 439, 3143). This was to correct styling errors I noticed that exist in current python.org.

A

@AA-Turner
Copy link
Member Author

AA-Turner commented Jun 16, 2021

Rendered site: https://aa-turner.github.io/peps/

Additional open questions:

Do we expand the list of explicitly whitelisted / unmasked emails? Last updated when it was added in 2002 with just peps@python, python-list@python, and python-dev@python. A fair few SIG type emails crop up in discussions-to etc, but for spam may want to keep masked?

Emails that came up more than once:

python-announce {at} py.org
distutils-sig {at} py.org
db-sig {at} py.org
doc-sig {at} py.org
web-sig {at} py.org
python-ideas {at} py.org
import-sig {at} py.org
datetime-sig {at} py.org
core-workflow {at} py.org
typing-sig {at} py.org
capi-sig {at} py.org

Dealing with titles with code (e.g. PEP 604) - render as code, or strip out and just render as normal text?

@AA-Turner
Copy link
Member Author

@pablogsal - rebased on master, I summarised changes above but can provide more detail if you'd like on anything :)

Do you know who best to contact to get GH pages enabled on this repo, incidentally?

A

@Mariatta
Copy link
Member

Just wanted to mention that I took a quick look at https://aa-turner.github.io/peps/ and it looks pretty cool.

I know that we can generate ePub using Sphinx. Is that something we can get if we're going with GitHub pages? I know that it is something we can get easily if hosting it in readthedocs.

cc @froi who was wondering about ePub version of the PEPs.

@AA-Turner
Copy link
Member Author

AA-Turner commented Jun 18, 2021

Hi Mariatta,

Thanks for the feedback :)

Will have a look at ePubs, although I don't have a kindle so I don't think I can preview? (after writing this I'll probably find an epub app for my phone within 30 seconds!)

Could I suggest though that it's tackled in a separate PR? Currently I'm focussed on trying to replicate what we currently have - but appreciate moving to Sphinx opens more avenues, which is partly the point I guess! I will open an issue to track ePub generation. (edit: see #1994)

A

@pablogsal
Copy link
Member

@AA-Turner is anything missing in this PR or can it be reviewed already?

@AA-Turner
Copy link
Member Author

There's only one thing - code in headings renders as grave accents not code (e.g. https://aa-turner.github.io/peps/pep-0604/)

But getting it working was proving a little challenging, so perhaps this PR can be reviewed as is, and I add the code headings in a followup? Technically I guess I should have solved it in #1931, but only noticed recently.

If you'd prefer otherwise let me know and I will prioritise getting the code-headings stuff working

A

@pablogsal
Copy link
Member

This looks good in general, but I am a bit confused why this is mixing changes to the sphinx files (the python code) with the theme files (css, html...etc). It will be easier to review if this were separated somehow. Maybe i am missing something, though :)

@AA-Turner
Copy link
Member Author

AA-Turner commented Jun 29, 2021

This looks good in general, but I am a bit confused why this is mixing changes to the sphinx files (the python code) with the theme files (css, html...etc). It will be easier to review if this were separated somehow. Maybe i am missing something, though :)

Blame poor planning, sorry - there were a few minor things that came up that I needed to fix as part of the theme work (e.g. the contents bit you reviewed just now, I realised I needed to change it to a proper header, the pep_url changes were needed to do the dynamic builder updates bit, I realised that the PEPTranslator needed to be added for each individual builder)

Co-authored-by: Pablo Galindo <Pablogsal@gmail.com>
@AA-Turner
Copy link
Member Author

@pablogsal fixed the literal markup issue so added that to this PR

Copy link
Member

@pablogsal pablogsal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Let's merge this and we can iterate in future PRs if we detect some problems.

Well done @AA-Turner !

@pablogsal pablogsal merged commit 0d93abf into python:master Jun 30, 2021
@AA-Turner
Copy link
Member Author

Thanks Pablo :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants