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

Sort anchors across pages #1358

Merged
merged 2 commits into from
May 30, 2021
Merged

Sort anchors across pages #1358

merged 2 commits into from
May 30, 2021

Conversation

aschmitz
Copy link
Contributor

@aschmitz aschmitz commented May 19, 2021

Previously we had sorted anchors within one page, but if anchors happened to be out of order in subsequent pages, they wouldn't be written in order, which caused lingering issues in some PDF viewers.

Related to #1352 and the change in d4561b1.

I'm not a huge fan of using lists to store different types of data indexed by position, but this was a quick way to do it that gets handled well by sorted. I won't be offended if you want to rewrite it using dicts or something else that might be more Pythonic. 🙂

aschmitz added 2 commits May 19, 2021 00:37
Previously we had sorted anchors within one page, but if anchors happened to be
out of order *across* pages, they wouldn't be written in order, which caused
lingering issues in some viewers.

Related to Kozea#1352 and the change in d4561b1.
@aschmitz
Copy link
Contributor Author

aschmitz commented May 19, 2021

Apologies, I thought I had removed all of my debugging before committing. This should work better. (Feel free to squash / commit separately - I didn't want to amend the last commit in case you had pulled it.)

@liZe liZe merged commit ba41e76 into Kozea:master May 30, 2021
@liZe liZe added the bug Existing features not working as expected label May 30, 2021
@liZe liZe added this to the 53.0b2 milestone May 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Existing features not working as expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants