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.

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

Fix start-at line numbers #663

Merged
merged 4 commits into from May 4, 2018
Merged

Conversation

akrabat
Copy link
Member

@akrabat akrabat commented May 3, 2018

In test_issue_310, the line numbers for the :start-at: test when using :linenos_offset: are off by one.

Addresses one of the problems with #662.

Add explanation of how each code block is set up so that it's easier to
determine if the test is correct.
As we have walked back to the start of the line, the calculation of
line_offset is one too high.
The :start-at: line numbers are now correct.
@akrabat
Copy link
Member Author

akrabat commented May 3, 2018

Note, that as the test_issue_310.ignore file is still in place, the Travis tests don't actually check that this half of #662 fixed…

Run the test locally with:

cd rst2pdf/tests
./autotest.py input/test_issue_310.txt --ignore-ignore

And check that output/test_issue_310.pdf matches reference/test_issue_310.pdf - specifically the second test should now have line numbers that start at 3, not 2.

@akrabat akrabat added this to the 0.94 milestone May 3, 2018
@lornajane
Copy link
Contributor

This looks good to me, the line numbers are fixed and my output matches the reference doc. I get a new md5 though, that you might like to add? It's 939b8f9fa1a592df9221c15657088bff.

akrabat added a commit to akrabat/rst2pdf that referenced this pull request May 4, 2018
@akrabat akrabat merged commit bd769d5 into rst2pdf:master May 4, 2018
@akrabat
Copy link
Member Author

akrabat commented May 4, 2018

Thanks @lornajane!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants