aeosynth / bk

Terminal Epub reader
MIT License
262 stars 17 forks source link

Add handling for pre tags #18

Closed jonseitz closed 2 years ago

jonseitz commented 2 years ago

Previously, <pre> tags in an epub were falling through to the default case and just being rendered as text, which was stringing all the individual lines together into a mess.

As an example, this is a code snippet from "Rust in Action":

2021-10-20_224714

This PR adds another arm to handle <pre> by rendered each line of text individually, adding a single line of whitespace above and below the whole block, along a tab character at the start of each line for indentation.

Here's the same snippet from above, with the new arm:

2021-10-20_224602

I have not tested this extensively with other sources, but I'm happy to do additional QA work if necessary. I did see that it can lead to some odd text wrapping in cases where a <pre> block contains longer lines, as is this other snippet:

2021-10-20_225942

This may actually be the expected behavior for the tag, but if that's not appropriate here we can look at some other options.

aeosynth commented 2 years ago

Could you find examples in a free book? I think the official rust book has an epub. Long lines wrapping seems fine.

jonseitz commented 2 years ago

@aeosynth Ah, good call. I didn't see an "official" epub for the Rust Book, but Eloquent Javascript has a free epub version (available under creative commons), so I did some additional testing with that.

Unfortunately, it was breaking because of syntax highlighting where each operator is wrapped in a <span>, so iterating through n.children() was printing each text node on a separate line. The new version pushed in ac5d3ce uses n.descendants() to recurse through all the nodes and append each character from each text node instead, which preserves all the whitespace and handles the potential edge case of multiply nested nodes.

Here's a javascript block (from the "Asynchronous Bugs" section under the "Asynchronous Programming" chapter) as rendered in the current release of bk:

2021-10-21_212019

And here it is in the PR version:

2021-10-21_211955

aeosynth commented 2 years ago

i think i did co-authoring correctly: https://github.com/aeosynth/bk/commit/61beded518e759884d72732a068744935c11ea6f