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.

Sign up for GitHub

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

Jump to bottom

Fix highlight's linenothreshold for Sphinx >=1.8+ & <2.0 #783

Merged
merged 1 commit into from Sep 5, 2019
Merged

Fix highlight's linenothreshold for Sphinx >=1.8+ & <2.0 #783

merged 1 commit into from Sep 5, 2019

Conversation

akrabat
Copy link
Member

@akrabat akrabat commented Jun 22, 2019 •
edited

Replace Sphinx's HighlightLanguageTransform with our own just for Sphinx versions between 1.8.0 & less than 2.0.0 as these versions have broken the :linenothreshold: setting in the highlight directive.

This code can be removed when we drop support for Python 2

Fixes #721

@akrabat
Fix highlight's linenothreshold for Sphinx >=1.8+ & <2.0
0c35198 
Replace Sphinx's HighlightLanguageTransform with our own just for Sphinx
versions between 1.8.0 & less than 2.0.0 as these versions have broken
the linenothreshold setting in the highlight directive.

This code can be removed when we drop support for Python 2

Fixes rst2pdf#721
@akrabat akrabat added this to the 0.95 milestone Jun 22, 2019
@akrabat
Copy link
Member Author

akrabat commented Sep 5, 2019

@oz123, @ralsina or @lornajane Any chance of a review on this one?

@ralsina
Copy link
Contributor

ralsina commented Sep 5, 2019 via email

@oz123 oz123 self-assigned this Sep 5, 2019
oz123
oz123 reviewed Sep 5, 2019
View reviewed changes
Copy link
Collaborator

@oz123 oz123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am OK with this change, I would be happy if there was a test for it ...

@akrabat
Copy link
Member Author

akrabat commented Sep 5, 2019

I'm not sure how to write one?

@oz123
Copy link
Collaborator

oz123 commented Sep 5, 2019

I'm not sure how to write one?

I would probably check the contect of a PDF which has:

.. highlight:: python
   :linenothreshold: 5

.. code-block:: python
   
   "This has more than 5 lines, so it has linenos"
   "This has more than 5 lines, so it has linenos"
   "This has more than 5 lines, so it has linenos"
   "This has more than 5 lines, so it has linenos"
   "This has more than 5 lines, so it has linenos"
   "This has more than 5 lines, so it has linenos"
   "This has more than 5 lines, so it has linenos"

But since this code is supposed to be removed in the next version, I don't mind having this merged as is.

oz123
oz123 approved these changes Sep 5, 2019
View reviewed changes
Copy link
Collaborator

@oz123 oz123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@oz123 oz123 merged commit 4d0ddf7 into rst2pdf:master Sep 5, 2019
@akrabat
Copy link
Member Author

akrabat commented Sep 6, 2019

Thanks!

@akrabat akrabat deleted the 721-fix-linenothreshold-in-sphinx branch September 6, 2019 06:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oz123 oz123 oz123 approved these changes

@ralsina ralsina Awaiting requested review from ralsina

@lornajane lornajane Awaiting requested review from lornajane

Assignees

@oz123 oz123

Labels
None yet
Projects
None yet
Milestone
0.95
Development

Successfully merging this pull request may close these issues.

Sphinx 1.8.0+ changes sphinx-issue168
3 participants
@akrabat @ralsina @oz123