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

Epub documents chapters will be ordered according to the document's spine where applicable #246

Merged
merged 7 commits into from
Feb 6, 2024

Conversation

pauliyobo
Copy link
Collaborator

Link to issue number:

fixes #243

Summary of the issue:

Basically whenever an epub document would have chapters with, say, a different naming scheme, such as roman numbers, their order would be totally inconsistent with what the result should be.

Description of how this pull request fixes the issue:

Since epub chapters are files in a folder, their current order would follow the OS filesystem ordering scheme rather than the book.
This PR essentially compares the files to be included in the book to render and if necessary they are adjusted according to the spine structure, which we always assume to be correct.

Testing performed:

Unit tests

Known issues with pull request:

None

@ogomez92 would you mind trying this version as soon as it builds?
You will probably have to clear the document cache for the fix to take effect.
Also, in the testsuite I have added the roman.epub file you attached in the issue. If this is a problem, please let me know and I will find something else to use instead.

@ogomez92
Copy link

Hi.
It's no problem, you can use this, it's a book by Michael connelly. It's in spanish though.

I'll try as soon as it builds but your appveyor build failed. can you check?

Thanks.

@cary-rowen
Copy link
Collaborator

@pauliyobo
Can you fix the build error?

@ogomez92
Copy link

ogomez92 commented Jan 26, 2024 via email

pauliyobo and others added 4 commits January 28, 2024 22:59
* Added github actions

* Fixed typo

* Fixed typo

* Added python encoding

* Trying to optimize build

* Added nsis github action

* Removed makensis action

* Naming installer

* Testing release workflow

* fixed indentation

* Downloading all artifacts

* Added invoke to dependencies

* Caching dependencies

* Modified cache key

* Incorrectly set up virtual env

* Fixing sintax error

* Activating virtual environment in other steps

* Trying to fix caching

* Merging two steps into one because of venv

* Trying to fix deploy job

* Fixed cache key

* Moving instead of copying

* Updated zip steps

* Adding version-information and translation catalogs

* Fixed oversight

* Fixed typo

* added distinct globs for other extension types

* Mistaked directory

* Made code more readable

* feat: Added test job

* fix: Fixed test job

* Fix: Fixed indentation

* feat: Added pytest.ini configuration

* Using invoke in dependencies job

* Added invoke

* Updated github actions versions

* Updated cache

* Uploading release-info only when a release will be generated

* Handling error when zipping artifacts
@pauliyobo
Copy link
Collaborator Author

Hello
I apologize for the delay, it has been a busy week at work.
@ogomez92 now that #234 has been merged you can grab the artifacts from the actions page. Please let me know if it works on your end as well before I merge.

@ogomez92
Copy link

ogomez92 commented Jan 29, 2024 via email

@ogomez92
Copy link

06 El_vuelo_del_angel_Michael_Connelly.zip
Hi,
OK nthat was strange, it seemed to be an error when replacing the new over the old one, but it worked.
The problem is that the issue seems to still be there with some files.
I cleared the document cache and deleted the database folder in the user-config just to double checked.
With the file I'm sending you it still happens, can you check?

Thanks.

@pauliyobo
Copy link
Collaborator Author

Hello. Do you mean the first file? I happen to have lost it, would you mind providing another link?
Thanks.

@ogomez92
Copy link

ogomez92 commented Jan 29, 2024 via email

@pauliyobo
Copy link
Collaborator Author

Thank you. Can you try now? You'll have to clear the cache again.

@pauliyobo
Copy link
Collaborator Author

Hello,
@ogomez92 just checking to understand if the PR solves your issue or if there are still things that don't work.

@ogomez92
Copy link

ogomez92 commented Feb 6, 2024 via email

@pauliyobo pauliyobo merged commit 61e8180 into develop Feb 6, 2024
4 checks passed
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.

Wrong chapter order when reading epub file (chapter 29, chapter 3, chapter 30.
3 participants