Using syntect as a higlighting backend rather than highlight.js

[notriddle]

This version of the syntect highlighting backend fixes two of the complains coming from version 2:

• It fixes the bugs. This version takes advantage of syntect's Scope Stack API to insert the "boring" span in the perfect spot regardless of how the scopes are nested, with very little groveling around the generated HTML using regexes (it does run one simple post-processing step to replace <span class=""> with <span class="boring">, because we would have to actually patch the syntax definition to enable a span that's actually named "boring").
• It moves the playground handling step into the pulldown-cmark converter, instead of trying to do it by groveling around the generated HTML using regexes, which was one of the TODOs on the iteration 2 PR.

Here's the result of running all of these versions using hyperfine. These aren't directly comparable to v2's, since I'm using a different setup than @ThePuzzlemaker

Command	Mean [s]	Min [s]	Max [s]
Syntect V3	1.727 s ± 0.053 s	1.673 s	1.833 s
Syntect V2	2.934 s ± 0.085 s	2.805 s	3.133 s
HLJS	1.214 s ± 0.022 s	1.190 s	1.262 s

To-dos

• Consider detecting "runs" of boring lines, and wrapping these whole chunks in a boring span instead of wrapping each line in its own. This thing is already extremely complex and subtle, and adding that would make it even harder to understand, but the generated HTML gets pretty bloated under the current approach.
• Make sure CSS files are free of any possible bugs from syntect's sublime theme to CSS generator (carried over from v2).
• Update documentation accordingly (carried over from v2).
• Consider supporting both highlight.js and syntect, with config options to choose between them. (not gonna do it unless someone figures out how to make boring lines work with a current version of hljs, which it seems like they haven't)

[notriddle]

The color schemes aren't actually the same. Is that a bug?

Old version:

New version:

[ThePuzzlemaker]

That's a remnant of the differences between the original (hljs) color schemes and the new (.tmTheme) color schemes converted using syntect::html::css_for_theme_with_class_style. I tried to find ones that were the closest to the original, but clearly there were some differences. If we want them to be the exact same, we'll need to make our own. It'd probably be easiest to tweak the generated CSS (which I already did with some simple fixes) to be more like the originals, rather than to tweak the .tmThemes.

Otherwise, this looks good and thanks for figuring this out! :P

[notriddle]

@ThePuzzlemaker I've tweaked the themes to make them closer to before, and also wrote the documentation for generating these themes from their tmTheme source files.

[notriddle]

@ThePuzzlemaker where did you get the Handlebars syntax from? Whatever source it came from should really be documented.

[ThePuzzlemaker]

It was from https://github.com/daaain/Handlebars/blob/master/grammars/Handlebars.tmLanguage, converted using Sublime Text 3 to a .sublime-syntax file. Must've forgotten to write that down 😅

[notriddle]

Okay, @ThePuzzlemaker. It's added.

[ISSOtm]

Nice! 👍

This is very interesting for us, since we have Pan Docs, which contains some RGBASM assembly snippets; so we could test this with the existing syntax definition. That'd be some testing with a 3rd-party grammar, seeing if it beings up any bugs.

[ISSOtm]

What needs to be done for this to be merged?

[jonaprieto]

What is the status of this? are there any plans to merge this soon?

[KFearsoff]

Let's get this to be merged!

I've successfully built this PR locally after merging master changes. I'll submit it shortly. It passes all tests, but I haven't verified by hand that it looks correct.

[notriddle]

If you'd like any help, please @ me and I'll look through your revised PR.

[KFearsoff]

If you'd like any help, please @ me and I'll look through your revised PR.

Thank you! I definitely wouldn't mind any help as long as it helps this PR get through! @notriddle

Here's the current diff I have: KFearsoff/mdBook@notriddle/use-syntect-v3~2...KFearsoff:mdBook:notriddle/use-syntect-v3

One issue I noticed is that a lot of grammars in the test book are not formatted, so we should maybe change the docs (or the book, or the default grammars we build). The colorscheme would also be nice to fix I guess, though our options are quite limited (perhaps there's a better default colorscheme that can function as a default?)

Right now, I'm working on one more patch that would fix the merge conflicts with the master.

[KFearsoff]

Okay, this is more like it: master...KFearsoff:mdBook:temp

I have to say, I dislike adding so many variables to render_markdown. It propagates diffs like crazy. I'll try to just pass around the mdbook Config, since we already should have access to it in parent scopes.

I might have also messed up tests/searchindex_fixture.json; I'm really not sure what this file does, so I just randomly resolved existing conflicts.

[notriddle]

Okay, first of all, testindex_fixture.json is generated automatically. If there's a conflict, you're expected to re-generated it with this (somewhat hacky) tool:

mdBook/tests/rendered_output.rs

Lines 797 to 800 in 94b922d

	// Setting this to `true` may cause issues with `cargo watch`,
	// since it may not finish writing the fixture before the tests
	// are run again.
	const GENERATE_FIXTURE: bool = false;

Beyond that, the way you've rebased it seems fine. I'd like to know if there's anything blocking this feature (I've considered supporting both this version and highlight.js — and the answer's probably "no", unless somebody figures out a way to handle boring lines).

[SichangHe]

@KFearsoff, ping.

[KFearsoff]

So what are the action points now?