-
Notifications
You must be signed in to change notification settings - Fork 615
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
Add support for syntax highlighting in PEPs #1638
Conversation
peps/converters.py
Outdated
# To simplify syntax highlighting, all literal blocks (those produced by ::) | ||
# in the following PEPs will be automatically highlighted using Python lexer. | ||
# PEP editors/authors could make simple PRs extending this list. | ||
# This will be not needed when PEPs are moved to RtD and all code blocks are | ||
# formatted using .. code:: language. | ||
PURE_PYTHON_PEPS = [483, 484, 526] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like it would make more sense to update the PEPs with appropriate code blocks rather than do this. I think this would also remove the need for the following lines (although I'm not sure what "Fix highlighting code" is currently doing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, I'll comment below what's happening.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is no longer used.
peps/converters.py
Outdated
# To simplify syntax highlighting, all literal blocks (those produced by ::) | ||
# in the following PEPs will be automatically highlighted using Python lexer. | ||
# PEP editors/authors could make simple PRs extending this list. | ||
# This will be not needed when PEPs are moved to RtD and all code blocks are | ||
# formatted using .. code:: language. | ||
PURE_PYTHON_PEPS = [483, 484, 526] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, I'll comment below what's happening.
@@ -140,6 +150,21 @@ def convert_pep_page(pep_number, content): | |||
# Fix PEP links | |||
pep_content = BeautifulSoup(data['content'], 'lxml') | |||
body_links = pep_content.find_all("a") | |||
# Fix highlighting code |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This chunk (from here to the next comment) is dealing with blocks with an explicit language, such as .. code-block:: python
or .. code-block:: c
.
More detail: the generated PEPs from https://github.com/python/peps has HTML like for these:
<pre class="code python literal-block">
...
<pre class="code c literal-block">
And with no language specified (::
) have:
<pre class="literal-block">
This chunk finds all <pre class="code ...">
, deletes those classes from the pre
, and wraps them in a <div class="highlight">
for later processing.
peps/converters.py
Outdated
div = pep_content.new_tag('div') | ||
div['class'] = ['highlight'] | ||
cb.wrap(div) | ||
# Highlight existing pure-Python PEPs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This chunk is dealing with blocks that don't have an explicit language, those marked with ::
, and defaulting to Python.
This PR is being cautious and only defaulting to Python for PURE_PYTHON_PEPS
.
More detail:
It finds all <pre class="literal-block">
(the language-specific ones have just had their literal-block
removed), then puts it through the Python highlighter.
I think it'd be good to default to Python for all ::
blocks, and we can specify the language in the actual PEPs for non-Python ::
blocks.
Sphinx also defaults to Python*, so this will also help for when Sphinx/RtD stuff is ready, and avoid us needing to replace ::
with .. code-block:: python
.
* Sphinx defaults to Python unless a directive like highlight:: c
is used. The PEPs don't use highlight::
so we're good.
Some example code. pep-0526.txtPEP: 526
Title: Syntax for Variable Annotations
Version: $Revision$
Last-Modified: $Date$
Author: <snip>
Status: Final
Type: Standards Track
Content-Type: text/x-rst
Created: 09-Aug-2016
Python-Version: 3.6
Post-History: 30-Aug-2016, 02-Sep-2016
Resolution: https://mail.python.org/pipermail/python-dev/2016-September/146282.html
TODO DEFAULT::
# Should be a comment in Python
#include <stdlib.h>
# 'primes' is a list of integers
primes = [] # type: List[int]
# 'captain' is a string (Note: initial value is a problem)
captain = ... # type: str
class Starship:
# 'stats' is a class variable
stats = {} # type: Dict[str, int]
TODO PYTHON:
.. code-block:: python
# Should be a comment in Python
#include <stdlib.h>
# 'primes' is a list of integers
primes = [] # type: List[int]
# 'captain' is a string (Note: initial value is a problem)
captain = ... # type: str
class Starship:
# 'stats' is a class variable
stats = {} # type: Dict[str, int]
TODO C:
.. code-block:: c
#include <stdlib.h>
size_t alloc_padding = 2;
size_t arena_padding = 10;
void* my_malloc(void *ctx, size_t size)
{
int padding = *(int *)ctx;
return malloc(size + padding);
} Output of
|
Can this be merged? |
Needs a rebase |
Are there any remaining concerns or can we merge? |
I think this still needs work, baking in
And then do the work of updating PEPs that are mis-highlighted. |
Other languages can be set like '.. code-block:: c'.
b18a853
to
ee186f6
Compare
Thanks for the bump! I'd forgotten I had something still do to. Now updated to use Python highlighting for the default |
@@ -39,3 +39,5 @@ django-filter==1.1.0 | |||
django-ordered-model==3.4.1 | |||
django-widget-tweaks==1.4.8 | |||
django-countries==6.1.3 | |||
|
|||
pygments==2.7.3 # This will be not needed when PEPs are moved to RtD. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure I understand this comment. Is there a plan to host the PEPs elsewhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the plan is to build PEPs using Sphinx and host them on Read the Docs at peps.python.org: python/peps#2 (June 2016).
There's a PR to do add Sphinx support, but it's a big one and hasn't been reviewed yet: python/peps#1385 (April 2020).
Hence reviving this PR.
peps/converters.py
Outdated
# To simplify syntax highlighting, all literal blocks (those produced by ::) | ||
# in the following PEPs will be automatically highlighted using Python lexer. | ||
# PEP editors/authors could make simple PRs extending this list. | ||
# This will be not needed when PEPs are moved to RtD and all code blocks are | ||
# formatted using .. code:: language. | ||
PURE_PYTHON_PEPS = [483, 484, 526] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is no longer used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I don't have the ability to merge PRs here)
Who can merge? |
@JelleZijlstra since python/peps#1577 was just merged, can this now be too? |
I don't have commit access here. |
@ewdurbin Hello again! Would you be able to merge this? |
This is the same as @ilevkivskyi's #1063: rebased with conflicts resolved.
It was closed in June 2017 because the plan was to get the PEPs onto Read the Docs (#1063 (comment)).
In December 2017 @brettcannon suggested (#1063 (comment)):
Three years on: I agree, it'd be great get colouring in whilst waiting on RtD (this big recent effort has stalled: python/peps#1385). I find highlighted code much easier to read.
Refs: