jgm / skylighting

A Haskell syntax highlighting library with tokenizers derived from KDE syntax highlighting descriptions
190 stars 61 forks source link

Wrap html source lines with <span>, not <a> #72

Closed dbaynard closed 5 years ago

dbaynard commented 5 years ago

Fixes jgm/pandoc#4386. Also fixes #33 (and therefore fixes jgm/pandoc#4278).

As it stands, it doesn't seem to work in epub. I suspect there are changes that need to go in data/epub.css (as per #32).

jgm commented 5 years ago

Excellent! Thanks for doing this. I'll see about updating epub.css.

jgm commented 5 years ago

Hm, epub.css doesn't contain anything relevant. I can confirm that the highlighting doesn't work well in iBooks, but it's not completely gone: I still get boldface, just no colors. Haven't been able to figure out why, but all the relevant styles are embedded in the HTML chapter itself...

jgm commented 5 years ago

OK, I may have been mistaken. When I change the appearance of the EPUB so that a lighter background is used (using iTune menu options), the colors show up.

Code doesn't wrap, though -- shouldn't it?

jgm commented 5 years ago

To answer my own question: no, it never wrapped before.

dbaynard commented 5 years ago

It didn't, but I think it should?

Also I couldn't get the line numbers to appear in the epub. I think I may need to reintroduce the data-line-number for epub. I don't quite have a suitable testing setup; I've been using Google play books which is a bit of a pain in this case.

jgm commented 5 years ago

The line numbers do appear in iBooks, so maybe this is a problem in Google play books.

As for wrapping: this gets requested from time to time. The main argument against it is that wrapping code can produce (what looks visually like) code that isn't semantically equivalent, or that is syntactically incorrect. So it's better for authors to ensure that code is hard-wrapped to a width that will be adequate when displayed. Counterargument: it's really hard to know what sort of a device will be displaying the code. I don't feel super-strongly about it. Maybe it would be good to make it an option, controllable via CSS that the user could add?

jgm commented 5 years ago

PS. There is code in the current CSS that seems intended to work with wrapping, so I wasn't sure what you intended.

dbaynard commented 5 years ago

By default, in print CSS, code wraps. With line numbers, it is clear which is a new line. Without line numbers it isn't. There may be a solution, with continuation characters. I'll fix the starting number issue first, then put together an example for you.

dbaynard commented 5 years ago

The main argument against it is that wrapping code can produce (what looks visually like) code that isn't semantically equivalent, or that is syntactically incorrect.

Incidentally, I agree on this. I suspect using continuation characters to indicate that a line wraps will clarify this. I'll check out the details, tomorrow, and implement something in the print css, if it isn't too complicated.

Maybe it would be good to make it an option, controllable via CSS that the user could add?

I suspect it would be helpful for me to write some guidelines to the default css, and how users might wish to amend it.