Conversation
ihalaij1
left a comment
ihalaij1
left a comment
There was a problem hiding this comment.
Great!
I tested this with the aplus-manual PR and did not notice any obvious issues on the A+ manual materials.
I added some comments.
| <ul class="dropdown-menu"> | ||
| <li role="presentation"> | ||
| <a href="{{ pathto(master_doc) }}"> | ||
| Index | ||
| <a href="{{ pathto(root_doc) }}"> | ||
| </a> |
There was a problem hiding this comment.
Is there a reason "Index" was removed here but not in some of the other places?
| <ul class="dropdown-menu"> | ||
| <li role="presentation"> | ||
| <a href="{{ pathto(master_doc) }}"> | ||
| Index | ||
| <a href="{{ pathto(root_doc) }}"> | ||
| </a> |
There was a problem hiding this comment.
Is there a reason "Index" was removed here but not in some of the other places? x2
There was a problem hiding this comment.
I don't think it makes sense to create empty links there, so I guess it's a copy-paste or search & replace error or something...
|
|
||
| * a-plus-rst-tools v1.4 support Sphinx v4.1. | ||
| * a-plus-rst-tools v1.5 and later support Sphinx v7.x (up to v7.4). | ||
| * a-plus-rst-tools v1.4 supports Sphinx v4.1. |
There was a problem hiding this comment.
We already have existing a-plus-rst-tools versions 1.5, 1.6, 1.7, and 1.8.
The latest version is 2.0.0 with BS5 support, so shouldn't the next version be 2.1.0?
There was a problem hiding this comment.
Seems I didn't check the actual versions from GitHub, but just blindly updated the README...
| If you are upgrading from RST tools v1.4 or earlier to v1.5, one change is | ||
| required in your course `conf.py`: the Sphinx setting `master_doc` was renamed | ||
| to `root_doc`. Open `conf.py` and replace the line | ||
|
|
||
| master_doc = 'index' | ||
|
|
||
| with | ||
|
|
||
| root_doc = 'index' | ||
|
|
||
| Note also that v1.5 (with Sphinx v7) changes how custom stylesheets are passed to the | ||
| theme. If your course uses a custom CSS file via `html_style` in `conf.py`, | ||
| you should verify that the stylesheet still loads correctly after the upgrade. | ||
| The a-plus theme templates have been updated accordingly. |
There was a problem hiding this comment.
Isn't there also another change required for the rst_prolog setting in conf.py (mentioned in the aplus-manual PR).
There was a problem hiding this comment.
Hmm, doesn't ring a bell. Which PR are we talking about?
There was a problem hiding this comment.
There was a problem hiding this comment.
I don't see rst_prolog mentioned in there. Or should I understand what the setting actually does...?
There was a problem hiding this comment.
It's the comment you added that is required so that the first content isn't considered as part of the last directive.
There was a problem hiding this comment.
If you expand the code in the diff view, you see it is part of rst_prolog.
There was a problem hiding this comment.
Ah, now I see what you mean. Indeed, if someone has copied that conf.py for the course, it needs to be pached there as well. Sorry, I'm really not very familiar with building courses in A+.
There was a problem hiding this comment.
At least Y1 is using the rst_prolog setting in conf.py, but with different directives than A+ manual.
|
I created a PR for compile-rst to update it to use Sphinx 7: |
Nice. Debian bullseye support ends this June, so those references could be upgraded (and tested) soon as well. |
3 participants
Fixes #177
Description
Add support for Sphinx version 7.4. The main changes to RST tools 1.4 from the user perspective are:
Also the Python 3.12+ warnings about regex patterns (SyntaxWarning: invalid escape sequence '\d', SyntaxWarning: invalid escape sequence '\w') are fixed in this PR.
This could become RST tools v1.5 and we could then later make a version 1.6 with support for Sphinx 9.x if deemed necessary.
Testing
This PR still needs more testing. I have tested that the rst compilation process for A+ manual (the version fixed for Sphinx 7's stricter rules) succeeds without warnings using Sphinx v7.4.7, but I haven't checked what the resulting HTML looks like in A+. This should be tested, and preferably compiling some actual course also.
What type of test did you run?
[ADD A DESCRIPTION ABOUT WHAT YOU TESTED MANUALLY]
Did you test the changes in
Think of what is affected by these changes and could become broken
Translation
Programming style
Have you updated the README or other relevant documentation?
Is it Done?
Clean up your git commit history before submitting the pull request!