Sitelet https://web.archive.org/web/20210718120618/https://github.com/python/peps/pull/1933
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
Contributor

@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 python/pep-editors as a code owner Apr 20, 2021
@AA-Turner AA-Turner force-pushed the AA-Turner:sphinx-theming branch 3 times, most recently from 12a1a20 to 3b19ba8 May 7, 2021
@AA-Turner AA-Turner force-pushed the AA-Turner:sphinx-theming branch from 3b19ba8 to 1ace935 Jun 8, 2021
@AA-Turner AA-Turner mentioned this pull request Jun 9, 2021
@AA-Turner AA-Turner force-pushed the AA-Turner:sphinx-theming branch 2 times, most recently from 58141f6 to d321101 Jun 9, 2021
@AA-Turner AA-Turner requested a review from pfmoore as a code owner Jun 13, 2021
@AA-Turner AA-Turner force-pushed the AA-Turner:sphinx-theming branch 2 times, most recently from dab95a4 to a1d49b7 Jun 13, 2021
@AA-Turner
Copy link
Contributor Author

@AA-Turner AA-Turner commented Jun 13, 2021

@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 AA-Turner force-pushed the AA-Turner:sphinx-theming branch from 58d3e03 to 2f5ac1e Jun 15, 2021
@AA-Turner
Copy link
Contributor Author

@AA-Turner 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 AA-Turner force-pushed the AA-Turner:sphinx-theming branch from ed5bb09 to 387b19b Jun 18, 2021
@AA-Turner
Copy link
Contributor Author

@AA-Turner AA-Turner commented Jun 18, 2021

@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

@Mariatta Mariatta commented Jun 18, 2021

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
Contributor Author

@AA-Turner 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

@AA-Turner AA-Turner force-pushed the AA-Turner:sphinx-theming branch from 6bc4dd1 to e52b0da Jun 29, 2021
@pablogsal
Copy link
Member

@pablogsal pablogsal commented Jun 29, 2021

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

@AA-Turner
Copy link
Contributor Author

@AA-Turner AA-Turner commented Jun 29, 2021

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

@pablogsal pablogsal 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 :)

@AA-Turner
Copy link
Contributor Author

@AA-Turner 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
Contributor Author

@AA-Turner AA-Turner commented Jun 30, 2021

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

Copy link
Member

@pablogsal pablogsal left a comment

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
2 checks passed
2 checks passed
@github-actions
build
Details
@github-actions
pre-commit
Details
@AA-Turner
Copy link
Contributor Author

@AA-Turner AA-Turner commented Jun 30, 2021

Thanks Pablo :)

@AA-Turner AA-Turner deleted the AA-Turner:sphinx-theming branch Jul 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants