Content iterator #82
Content iterator #82
Conversation
| return "" | ||
| } | ||
|
|
||
| // TODO partialCfi and domRange getters |
There was a problem hiding this comment.
AFAIK they are only used in Thorium, so we might not even need them.
There was a problem hiding this comment.
Thorium doesn't really use / need EPUB CFI. We generate them in our "locators" next to the data structure we actually rely on to make things work internally (DOM Range for precise locations, percentage progression otherwise ... oh, and by the way we still don't support positions list yet). We emit CFI "just because we can", and it's only the path to the element anyway, no character-level reference. To achieve feature-parity with our internal DOM Range (de)serialisation mechanism, we would need a properly-tested and conformant CFI implementation, which apparently doesn't exist :)
- https://github.com/fread-ink/epub-cfi-resolver
- https://github.com/vivliostyle/vivliostyle.js/blob/master/packages/core/src/vivliostyle/cfi.ts
- https://github.com/johnfactotum/foliate-js/blob/main/epubcfi.js
- https://github.com/futurepress/epub.js/blob/master/src/epubcfi.js -- https://github.com/futurepress/EpubCFI/blob/master/src/epubcfi.js
- https://github.com/readium/readium-cfi-js
- https://github.com/satorumurmur/bibi/blob/master/__src/bibi/extensions/epubcfi.js
- Colibrio? (https://api-docs.colibrio.com/web/3.9.0/modules/colibrio_core_selector_epubcfi.html)
Also see futurepress/epub.js#1358
There was a problem hiding this comment.
This work is based on the kotlin-toolkit, which is presumably based on the documentation here: https://github.com/readium/architecture/blob/master/models/locators/extensions/html.md . partialCfi is just a string in kotlin, so it could theoretically omitted since you can access it if it exists via OtherLocations, but since DomRange is implemented in the kotlin-toolkit I would want to implement it in the go-toolkit as well
There was a problem hiding this comment.
The extension helpers are implemented but never used. It might not be worth implementing it in the Go toolkit, unless you do want to generate partial CFI or DOM ranges at some point.
| Text(separator *string) (string, error) // Extracts the full raw text, or returns null if no text content can be found. | ||
| Iterator() iterator.Iterator // Creates a new iterator for this content. | ||
| Elements() ([]element.Element, error) // Returns all the elements as a list. |
There was a problem hiding this comment.
In the Kotlin toolkit, text() and elements() are in the interface with a default implementation, but they should probably be extensions instead. The only thing that should be required to implement is Iterator.
Maybe we can remove them from the interface and keep only ContentText() and ContentElements() on the side?
Alternatively if we want to keep the API easy to use, Content could be a struct and have a IteratorFactory interface instead.
Co-authored-by: Mickaël Menu <[email protected]>
Need to port over the tests too before this is ready
Notable changes:
manifest.Propertiesto avoid mutations, also adding aProperties()getter to theResourceinterface. See (Im)mutability in the toolkit APIs #88 for more infoFuture improvements: