Opened 4 years ago
Closed 4 years ago
#27528 closed enhancement (fixed)
Remove the highlighting patch for Sphinx
Reported by: | jhpalmieri | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.8 |
Component: | packages: standard | Keywords: | |
Cc: | gh-timokau | Merged in: | |
Authors: | John Palmieri, Jeroen Demeyer | Reviewers: | Dima Pasechnik |
Report Upstream: | N/A | Work issues: | |
Branch: | 83436ea (Commits, GitHub, GitLab) | Commit: | 83436ea82a17406bbc3fdd9e8c386208abc93727 |
Dependencies: | Stopgaps: |
Description (last modified by )
The goal of this ticket is to clean up highlighting in the documentation. There are several issues. Right now, we are patching pygments and Sphinx so that they know about the "sage:" prompt. Also, we are telling Sphinx to highlight all code blocks as if they were Python console blocks. There are various other types of code blocks in the Sage docs: Python, Cython, reST, LaTeX, etc., and right now many are not highlighted at all: Sphinx/pygments tries to parse them as the Python console and fails, so does nothing. Even some of the Sage console blocks aren't highlighted correctly, because pygments refuses to parse input like sage: a?
: valid IPython but not valid Python.
To improve the situation: Sage's console is based more on IPython than on Python, and IPython comes with a lexer to use when building its own documentation. Furthermore, that lexer explicitly allows for customization of the input prompt. So we can get rid of the prompt patches and use IPython's lexer, and it succeeds at highlighting blocks containing sage: a?
and other IPython-type text. The only disadvantage to this is that IPython's lexer doesn't convert <BLANKLINE>
in doctest output to a blank line, so we add a Sphinx transform to handle this.
We also mark some non-Python blocks as what they are, to enable their highlighting.
Change History (39)
comment:1 Changed 4 years ago by
comment:2 Changed 4 years ago by
A few blocks in Sage look like
Traceback: .. <-- should be exactly three dots more stuff
Having 2 or 4 dots means that Sphinx can't handle the highlighting: you can see this here, where there are 4 dots instead of 3.
Along the same lines, having extra spaces at the end of NotImplementedError
causes highlighting to fail here.
comment:3 Changed 4 years ago by
Branch: | → u/jhpalmieri/sphinx-highlight |
---|
comment:4 Changed 4 years ago by
Commit: | → 1debde6cf246bd2ed7a5f90aa58c877c6c61586f |
---|---|
Status: | new → needs_review |
New commits:
1debde6 | trac 27528: remove highlighting.patch for Sphinx
|
comment:5 Changed 4 years ago by
comment:6 Changed 4 years ago by
Cc: | gh-timokau added |
---|
comment:7 Changed 4 years ago by
Status: | needs_review → needs_work |
---|
comment:8 Changed 4 years ago by
Commit: | 1debde6cf246bd2ed7a5f90aa58c877c6c61586f → 687b3385e80af4efd6ee8db79200c518a731af1d |
---|
comment:9 Changed 4 years ago by
Description: | modified (diff) |
---|---|
Status: | needs_work → needs_review |
comment:10 Changed 4 years ago by
Here is a new approach: I just discovered the IPython lexer, and we should definitely use it rather than Python lexer.
comment:11 Changed 4 years ago by
A few things to check:
- See http://doc.sagemath.org/html/en/constructions/interface_issues.html for some code blocks for other languages (LaTeX, BibTeX, shell session). These are not currently highlighted, but they will be with this branch. Same goes for various shell session blocks scattered through the developer's guide.
- The first block in the tour-help section of the tutorial is not currently highlighted, but it will be with this branch.
- double check that
<BLANKLINE>
does not appear in the reference manual. One spot to check: from the main reference manual page, click "Symbolic Calculus" and then "Riemann Mapping". The method "complex_to_spiderweb" (toward the end) has some<BLANKLINE>
s in the doctest output, and they should be rendered as blank lines in the reference manual.
comment:12 Changed 4 years ago by
To do on another ticket: go through more of the reST docs and perhaps the reference manual, marking the non-sage-doctest blocks using .. CODE-BLOCK:: language
. I'll open up a followup ticket eventually.
comment:13 Changed 4 years ago by
Reviewers: | → Dima Pasechnik |
---|---|
Status: | needs_review → positive_review |
works for me nicely. I looked at few random places in various docs, all good.
comment:14 Changed 4 years ago by
Thank you for the review.
See #27549 for a followup: more code-block cleanup.
comment:15 Changed 4 years ago by
Status: | positive_review → needs_work |
---|
Remember to change the package version if you change patches.
comment:16 follow-up: 18 Changed 4 years ago by
Status: | needs_work → positive_review |
---|
How about we just mark this ticket to be merged together with #26451 - which does a version bump?
comment:17 Changed 4 years ago by
Status: | positive_review → needs_work |
---|
comment:18 Changed 4 years ago by
comment:19 Changed 4 years ago by
Status: | needs_work → positive_review |
---|
What's the point of doing this? To do more work than needed? Let's just merge them together.
comment:20 Changed 4 years ago by
Status: | positive_review → needs_work |
---|
comment:21 Changed 4 years ago by
Branch: | u/jhpalmieri/sphinx-highlight → u/jdemeyer/sphinx-highlight |
---|
comment:22 Changed 4 years ago by
Commit: | 687b3385e80af4efd6ee8db79200c518a731af1d → 278fa3b3fca010621d41df9d63e0b11801875adc |
---|---|
Status: | needs_work → needs_review |
New commits:
278fa3b | Bump package versions
|
comment:23 Changed 4 years ago by
Sometimes it takes less effort to do the right thing rather than arguing about it...
comment:25 Changed 4 years ago by
Status: | positive_review → needs_work |
---|
Looking at this diff, it seems that indentation is not correct in a few places, for example
+ that the user should be aware of.
+
+ .. CODE-BLOCK:: rest
comment:26 Changed 4 years ago by
I also have doubts about
There are 2 tests in doc/en/developer/coding_in_python.rst that are not being run
comment:27 Changed 4 years ago by
And shouldn't the in2_regex
for IPythonConsoleLexer
be customized too to match ....:
?
comment:28 follow-up: 34 Changed 4 years ago by
Commit: | 278fa3b3fca010621d41df9d63e0b11801875adc → a323c0965f3020045900cdb0fcb7702d4ec56f88 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
a323c09 | Fix a few CODE-BLOCK indentations
|
comment:29 Changed 4 years ago by
Commit: | a323c0965f3020045900cdb0fcb7702d4ec56f88 → 4cabfa73badacb19731e95d959a1cf4bedcf531c |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
4cabfa7 | Fix _test_enough_doctests()
|
comment:30 Changed 4 years ago by
Commit: | 4cabfa73badacb19731e95d959a1cf4bedcf531c → 83436ea82a17406bbc3fdd9e8c386208abc93727 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
83436ea | Fix usage of IPythonConsoleLexer
|
comment:31 Changed 4 years ago by
Authors: | John Palmieri → John Palmieri, Jeroen Demeyer |
---|---|
Description: | modified (diff) |
Status: | needs_work → needs_review |
comment:32 Changed 4 years ago by
I didn't understand the rationale for making a difference depending on the Python version running the docbuilder: the sources are the same on Python 2 and Python 3, so the documentation should also be the same. So I'm using the default version (which is currently Python 2) always.
comment:34 follow-up: 36 Changed 4 years ago by
comment:35 Changed 4 years ago by
My one objection is that the version for the pygments
package should not say .p0
since there are no patches any more. Is there anything to be done about this? (Aside from waiting for the next release, that is.)
comment:36 follow-up: 37 Changed 4 years ago by
comment:37 Changed 4 years ago by
Replying to jhpalmieri:
Replying to dimpase:
Replying to git:
Branch pushed to git repo; I updated commit sha1. New commits:
a323c09 Fix a few CODE-BLOCK indentations
I thought wrong indentations result in sphinx errors...
Sometimes it just results in Sphinx silently ignoring the directive.
Actually this case looked like
text .. CODE-BLOCK:: rest more text
and Sphinx interpreted the extra space in front of .. CODE-BLOCK...
just as indentation, so the code block was highlighted correctly, just indented more than it should have been.
comment:38 Changed 4 years ago by
Status: | needs_review → positive_review |
---|
Jeroen, thanks for the fixes. There is probably nothing to be done about the pygments version number.
comment:39 Changed 4 years ago by
Branch: | u/jdemeyer/sphinx-highlight → 83436ea82a17406bbc3fdd9e8c386208abc93727 |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
We want Sage's doctest blocks (and most other literal blocks) to be typeset as Python console code. Setting the default highlighting language to
pycon
does this, and it means that we can then not use the highlighting patch for Sphinx.Currently in Sage, there are some blocks which are not highlighted correctly. See this page: in the first block (containing
CombinatorialFreeModule?
), the prompt "sage:" is not highlighted. With the proposed change, that block will still not be highlighted the way we want, and in fact it will produce a warning:We have set up the docbuilding process so that warnings stop it, so we also need to silence this warning.
There are no doubt some blocks in the Sage code which should not be highlighted using
pycon
, but as things stand in current Sage, Sphinx already tries to highlight them using thedefault
lexer, which basically meanspython3
with a graceful failure if it can't parse it as Python 3: see the Sphinx docs. Once we set the highlighting language to something other thandefault
, we lose the graceful failure, and so we have to silence the warnings.In my spot-checking, the current non-Python console blocks fail their highlighting and so have no highlighting at all, and that behavior continues with this branch. At some point we could try to modify those blocks: something like
But that can be on another ticket.