Syntax Highlighter

[robertpanzer]

These are still the very first baby steps, but still I'd like to open a PR to discuss some things.

This PR currently only supports Syntax Highlighters that add doc info to the header or footer.
"Converting Highlighters" are not supported yet.

There are a few questions though:

• Should there be an own SyntaxHighlighterRegistry to register highlighters? Or would it make sense to simply add this to the JavaExtensionRegistry?
• What is the correct term to expose? Syntax Highlighter or Source Highlighter? The Ruby API uses Syntax Highlighter, but the corresponding attribute is called source-highlighter.

Currently the plan is to have a base interface SyntaxHighlighter that can be implemented to add any DocInfo.
Then a user should be able to implement a sub interface ConvertingSyntaxHighlighter which adds the methods for transforming the source blocks directly.

[mojavelinux]

What is the correct term to expose? Syntax Highlighter or Source Highlighter? The Ruby API uses Syntax Highlighter, but the corresponding attribute is called source-highlighter.

Syntax highlighter is the correct term. I address this question in the docs. https://asciidoctor.org/docs/user-manual/#enabling-source-highlighting

[mojavelinux]

Converting Highlighters

The correct term here is "server-side syntax highlighter" (whereas the converse is "client-side syntax highlighter"). I'd also consider "build-time" instead of "server-side" as technically the document could be converted on the client. If you have other suggestions, I'm open to it.

[mojavelinux]

Should there be an own SyntaxHighlighterRegistry to register highlighters?

Yes, I think it should be separate as it's really a different concern. This is more about augmenting the capabilities of the processor than it is about the document itself.

I also don't think you need to add the "Java" prefix to the registry. It's well understood given this is a Java API that what you are registering is a Java-based syntax highlighter. I don't really see a need for integrating Ruby-based syntax highlighters this way as it can already be handled using the require option.

[robertpanzer]

I also don't think you need to add the "Java" prefix to the registry. It's well understood given this is a Java API that what you are registering is a Java-based syntax highlighter. I don't really see a need for integrating Ruby-based syntax highlighters this way as it can already be handled using the require option.

Oh yes, I forgot about Asciidoctor.requireLibrary().
I was caught in the impression that a user would need to register a Ruby Syntax Highlighter via RubyExtensionRegistry.requireLibrary() or we would need to add a method allowing to register a Syntax Highlighter with a specific name, comparable to RubyExtensionRegistry.block(name, processor).
If we don't need that we can indeed remove the Java-prefix.
My feeling tells me though that Asciidoctor.requireLibrary() is a candidate for deprecation, as it assumes a certain implementation, it's the same situation that we currently have with the ExtensionRegistries.

[robertpanzer]

Added support for writing stylesheet to disk.
A SyntaxHighlighter has to implement the interface WriteStylesheet.
If it implements this, the 2 methods isWriteStylesheet(Document) and writeStylesheet(Document, File) will be called, if the other preconditions are fulfilled.

[robertpanzer]

Added support for server side highlighters.
The API now looks like this:

• Every highlighter has to implement org.asciidoctor.syntaxhighlighter.SyntaxHighlighter. This interface contains the methods for adding doc info.
• If a highlighter is able to write an external stylesheet, it can do so by also implementing org.asciidoctor.syntaxhighlighter.WriteStylesheet.
• A "server-side" syntax highlighter also has to implement the interface org.asciidoctor.syntaxhighlighter.ServerSide.

These 3 interfaces have no formal relation in terms of inheritance, because that would require to create all combinations, or have unused, useless default implementations.

The naming is bad, I know, and I suck at it. I am happy to use any other name than ServerSide and WriteStylesheet.

The last missing functionality imo is to allow implementing format in Java.
In my current understanding this creates the "frame" for the source block, ie it could for example add custom classes or use sth else than a <pre/> tag to render the code.
Server side rendering itself – in my understanding – is done via a substitute that calls the highlight method.

[mojavelinux]

You're off to a great start!

org.asciidoctor.syntaxhighlighter.WriteStylesheet

I would name this StylesheetWriter to conform with the naming for converter APIs.

org.asciidoctor.syntaxhighlighter.ServerSide

Perhaps name this Highlighter. I know it seems redundant to say a "highlighting highlighter", but oddly enough, that's actually what's happening. The integration may just prepare the assets for performing highlighting on the client, or it might actually do the highlighting itself.

Another idea would be SyntaxHighlighterCaller (or Invoker).

SyntaxHighlighter

I would then rename this to SyntaxHighlighterAdapter since the main API is really about adapting to a library.

Hopefully those suggestions help with the naming.

The last missing functionality imo is to allow implementing format in Java.
In my current understanding this creates the "frame" for the source block, ie it could for example add custom classes or use sth else than a

 tag to render the code.

Correct. Another term for it would be "layout".

[robertpanzer]

Thank you, @mojavelinux! That's great input!

You mentioned Layout as a possible interface name for the format method.
In german there exists the noun Layouter which is somebody who does layouts, unfortunately this doesn't seem to be an english word :-D I'll try to go with Layout for now.

[robertpanzer]

I was thinking about making this experimental at first until we get some user feedback.
I would like to avoid going from AsciidoctorJ 2.x to 3.x, to 4.x etc just because we found out that we need to change the API after the first users tried it out.

That said I think the test suite should also try to reimplement rouge, coderay, etc adapters to prove that the API works.

[robertpanzer]

Added an interface Layout that includes the format method.
I guess this PR is feature complete now.
What is missing is more tests that reimplement existing highlighters in Java, plus the documentation in the integrator guide. (And more Javadoc).

[mojavelinux]

Added an interface Layout that includes the format method.

You could also consider Formatter, which is a word.

[robertpanzer]

I removed the WIP from the PR title.
This means that imo this PR is now ready for review.
I also added a few words to the integrator guide, only conversion time highlighting is missing from that.
I also added tests that simulate the existing highlighters for highlight.js and coderay.

For now I'd like to keep this API as experimental.
I don't know if this is covered by semver.org, but I'd like to be able to make changes to this API without having to bump the first digit in the version, i.e. for bug fixes that result in incompatible API changes.
I am unsure if I get agreement for this, but I jump started this and I am not sure if it stands the tests of actual users.
(Even though I am not even sure if anybody will really implement something else than the broad range of syntax highlighters that Asciidoctor already brings)

Wrt OSS this was probably my most productive PTO ever 😂

[mojavelinux]

I don't know if this is covered by semver.org, but I'd like to be able to make changes to this API without having to bump the first digit in the version, i.e. for bug fixes that result in incompatible API changes.

My understanding is that as long as you clearly mark an API as experimental (or prerelease), and it's a new API, you can continue to make changes to it without impacting the semver rules of the whole. You have essentially sectioned off a portion of the API as "alpha" and can then continue to change that API without warning.

[robertpanzer]

One thing about the API that I might want to change is that I currently use the org.asciidoctor.extension.LocationType as the parameter to hasDocInfo().
I probably better duplicate this enumeration in the package org.asciidoctor.syntaxhighlighter to avoid any coupling between these 2 packages.

[mojavelinux]

I don't think it's a bad thing to couple them (unless you are worried about modularity). Technically, the refer to the same concept. It's a property that belongs to docinfo. So maybe it should be promoted to the core package (since it's not limited to extensions per say).

[robertpanzer]

So maybe it should be promoted to the core package (since it's not limited to extensions per say).

My initial thinking went into the other direction: Move syntax highlighting to extensions, therefore my initial question about whether to create an own syntax highlighter registry or add it as a new extension type.

In hindsight moving it to core sounds like the right way.
But I'd like to avoid a breaking change again, also if it's just about updating imports.

[robertpanzer]

Updated the PR with addition to the integrator guide: Added an example for how to do static, or server-side, syntax highlighting.
In the example I used prism.js, it uses the MIT license.
This should be compatible to use, right?
Before merging this PR I'd like to update the integrator guide with a generated documentation for the calling sequence of a syntax highlighter (because you know, living documentation...)

[mojavelinux]

Move syntax highlighting to extensions, therefore my initial question about whether to create an own syntax highlighter registry or add it as a new extension type.

That works too.

[mojavelinux]

In the example I used prism.js, it uses the MIT license.
This should be compatible to use, right?

Correct.

[robertpanzer]

Updated the integrator guide to generate the documentation about the invocation order of the methods of a syntax highlighter.

For me the PR could be merged now unless there is more feedback.

[robertpanzer]

That means, we still need a services interface to allow auto registration of syntax highlighters.
(Actually this is just another name for the ExtensionRegistry that we already have)

[robertpanzer]

I'll try to release AsciidoctorJ 2.1.0 in the next days.