Add support for syntax highlighting in PEPs#1638
Closed
hugovk wants to merge 6 commits intopython:mainfrom
Closed
Conversation
This was referenced Sep 21, 2020
di
reviewed
Sep 21, 2020
peps/converters.py
Outdated
Comment on lines
+22
to
+27
| # 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] |
Member
There was a problem hiding this comment.
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.
Member
Author
There was a problem hiding this comment.
I agree, I'll comment below what's happening.
hugovk
commented
Sep 22, 2020
peps/converters.py
Outdated
Comment on lines
+22
to
+27
| # 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] |
Member
Author
There was a problem hiding this comment.
I agree, I'll comment below what's happening.
| # Fix PEP links | ||
| pep_content = BeautifulSoup(data['content'], 'lxml') | ||
| body_links = pep_content.find_all("a") | ||
| # Fix highlighting code |
Member
Author
There was a problem hiding this comment.
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 |
Member
Author
There was a problem hiding this comment.
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.
Member
Author
|
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? |
Member
|
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'.
hugovk
force-pushed
the
ilevkivskyi-colour
branch
from
b18a853 to
ee186f6
Compare
Member
Author
|
Thanks for the bump! I'd forgotten I had something still do to. Now updated to use Python highlighting for the default |
di
reviewed
Dec 18, 2020
| 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. |
Member
There was a problem hiding this comment.
Not sure I understand this comment. Is there a plan to host the PEPs elsewhere?
Member
Author
There was a problem hiding this comment.
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
Comment on lines
+22
to
+27
| # 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] |
di
approved these changes
Dec 18, 2020
Member
di
left a comment
di
left a comment
There was a problem hiding this comment.
(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? |
Member
|
I don't have commit access here. |
|
@ewdurbin Hello again! Would you be able to merge this? |
Member
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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: