Join forces with Asciidoctor template.js

[ggrossetie]

Hello Sylvain,

I think it could be great to join our forces!

You may know that I'm working on a similar project: https://github.com/asciidoctor/asciidoctor-template.js This project extends/overrides the default templates converter (provided by Asciidoctor core) to use Pug/Jade templates (instead of Slim/Haml templates): https://asciidoctor.org/docs/user-manual/#provide-custom-templates

I believe that both projects have the same goal and it would be great if we could work together. I'm sure we can provide a simple solution to use custom templates in Asciidoctor.js :smiley:

Let me know what you think!

Guillaume

[s-leroux]

Hi Guillaume, Thank you for having contacted me!

I was aware of https://github.com/asciidoctor/asciidoctor-template.js --and it was my principal source of inspiration. You may even consider asciidoctor.js-pug as a rewrite of asciidoctor-template.js in pure JavaScript.

Now, to give you the reason I started that project: asciidoctor-template.js is great. But I found two obstacles in using it:

1. It "felt" too coupled with the Reveal.js backend. After having studied the source, I'm not sure it is really "that" tightly coupled--but at this point, I already have worked a couple of hours on asciidoctor.js-pug
2. The second reason, and somewhat the most important to me, is I'm not proficient with Ruby. So I was looking for a pure JS solution I could understand and hack--and that didn't require transpilling code through Opal

This is just my opinion, but even if there are some Opal specific-code, I still expect a JavaScript developer to understand the code of the converter(https://github.com/s-leroux/asciidoctor.js-pug/blob/master/lib/converter.js) --which is basically:

• Create a new Opal class (1 line)
• Add methods whose body is written in pure-JavaScript.

Not only a pure JS implementation would reduce the barrier of adoption, but we may also expect this will increase the likeliness that the library users (which are by definition JavaScript developers) will submit patches.

For something completely different, worth mentioning since I was working with node.js, I didn't take into account at all other JS platforms. So there is clearly some work to do in that area.

I believe that both projects have the same goal and it would be great if we could work together. I'm sure we can provide a simple solution to use custom templates in Asciidoctor.js

Indeed, that would be great. Now you know my motivations, what would you suggest to start converging the two projects?

[ggrossetie]

I was aware of https://github.com/asciidoctor/asciidoctor-template.js --and it was my principal source of inspiration. You may even consider asciidoctor.js-pug as a rewrite of asciidoctor-template.js in pure JavaScript.

That's exactly the reason why we created the wrapper API in Asciidoctor.js. I strongly believe that we need to provide a simple and consistent API so that users can write extensions and custom converters in pure JavaScript! :smiley:

Now, to give you the reason I started that project: asciidoctor-template.js is great. But I found two obstacles in using it: It "felt" too coupled with the Reveal.js backend. After having studied the source, I'm not sure it is really "that" tightly coupled--but at this point, I already have worked a couple of hours on asciidoctor.js-pug

I don't think that Asciidoctor template.js is tightly coupled with the Reveal.js backend. We do look for templates in an extra directory to support the Reveal.js backend... but apart from that the project should work with any Pug/Jade templates.

Having said that, I agree that we are lacking documentation and the README only mentions the Reveal.js backend and does not provide any example on how to create a custom backend... :disappointed:

The second reason, and somewhat the most important to me, is I'm not proficient with Ruby. So I was looking for a pure JS solution I could understand and hack--and that didn't require transpilling code through Opal

:+1:

This is just my opinion, but even if there are some Opal specific-code, I still expect a JavaScript developer to understand the code of the converter(https://github.com/s-leroux/asciidoctor.js-pug/blob/master/lib/converter.js) --which is basically: Create a new Opal class (1 line) Add methods whose body is written in pure-JavaScript.

We could also create a wrapper API to hide Opal. We are currently working with Dan on a friendly API to create extension classes:

let includeProcessor = asciidoctor.Extensions.createIncludeProcessor('StaticIncludeProcessor', {
  process: (doc, reader, target, attrs) => {
    reader.pushInclude(['foo'], target, target, 1, attrs);
  }
});

https://github.com/asciidoctor/asciidoctor.js/pull/412

Not only a pure JS implementation would reduce the barrier of adoption, but we may also expect this will increase the likeliness that the library users (which are by definition JavaScript developers) will submit patches.

I totally agree :+1:

For something completely different, worth mentioning since I was working with node.js, I didn't take into account at all other JS platforms. So there is clearly some work to do in that area.

Indeed, Asciidoctor.js targets the following environments:

• Browsers
• Node
• Nashorn

It would be great if we could use custom templates in all of these environments.

Indeed, that would be great. Now you know my motivations, what would you suggest to start converging the two projects?

I think we should take your approach and write the TemplateConverter in pure JavaScript. But I strongly believe that we need to use the same API as Asciidoctor core: https://asciidoctor.org/docs/user-manual/#provide-custom-templates

So basically, the following code:

before.js

const asciidoctor = require('asciidoctor.js')();
require('asciidoctor.js-pug')('./path/to/template/directory'); // template directory should be defined in the options

console.log(asciidoctor.convert('Hello world'));

will become:

after.js

const asciidoctor = require('asciidoctor.js')();
require('asciidoctor.js-pug');

console.log(asciidoctor.convert('Hello world', {template_dirs: './path/to/template/directory'}));

I think the first step is to update this project to be able to configure the template converter using the template_dirs option. Then we can merge the two projects and work together to make it work on a browser environment.

What do you think ?

[s-leroux]

I don't think that Asciidoctor template.js is tightly coupled with the Reveal.js backend. [...] Having said that, I agree that we are lacking documentation and the README only mentions the Reveal.js I think that is was triggered my initial feeling.

Speaking of that, one thing that confused me when reading the docs, was the exact relationship between a "converter" and a "backend". And that is still not clear to me.

I think we should take your approach and write the TemplateConverter in pure JavaScript. But I strongly believe that we need to use the same API as Asciidoctor core: https://asciidoctor.org/docs/user-manual/#provide-custom-templates

No doubt about that. We should absolutely have a similar API between the different languages so the user can easily adapt examples and tutorials from one implementation to the other one.

As it is now, the API in asciidoctor.js-pug is mostly a hack corresponding to my immediate needs. So there is plenty of room for improvement.

I think the first step is to update this project to be able to configure the template converter using the template_dirs option.

Sounds good to me. I will take a look at that this WE.

[ggrossetie]

Speaking of that, one thing that confused me when reading the docs, was the exact relationship between a "converter" and a "backend". And that is still not clear to me.

Me too... https://github.com/asciidoctor/asciidoctor.org/issues/644 🤣

No doubt about that. We should absolutely have a similar API between the different languages so the user can easily adapt examples and tutorials from one implementation to the other one. As it is now, the API in asciidoctor.js-pug is mostly a hack corresponding to my immediate needs. So there is plenty of room for improvement. Sounds good to me. I will take a look at that this WE.

Excellent news! thanks 👍 Let me know if I can help or if you have questions.

[s-leroux]

Apparently, I have to register the template converter as the ::Asciidoctor::Converter::TemplateConverter otherwise when I pass the template_dirs option, asciidcotor.js breaks when trying to load the "asciidoctor/converter/template" module (self.$require("asciidoctor/converter/template".$to_s()) around line 26446):

          if ($truthy((($o = Opal.const_get_qualified('::', 'Asciidoctor', 'skip_raise')) && ($n = Opal.const_get_qualified($o, 'Converter', 'skip_raise')) && ($m = Opal.const_get_qualified($n, 'TemplateConverter', 'skip_raise')) ? 'constant' : nil))) {
            } else {
            self.$require("asciidoctor/converter/template".$to_s())
          };

Or did I missed something?

[ggrossetie]

Apparently, I have to register the template converter as the ::Asciidoctor::Converter::TemplateConverter otherwise when I pass the template_dirs option, asciidcotor.js breaks when trying to load the "asciidoctor/converter/template" module (self.$require("asciidoctor/converter/template".$to_s()) around line 26446):

You're right, we have to "override" or "patch" the default TemplateConverter defined in Asciidoctor core: https://github.com/asciidoctor/asciidoctor/blob/master/lib/asciidoctor/converter/template.rb

You can have a look at the generated code here: https://github.com/asciidoctor/asciidoctor-template.js/blob/68626125a426ef0e04e10bb48d3c60728c186567/dist/main.js#L104 (where the TemplateConverter class is defined)

[s-leroux]

Ok, it shouldn't be an issue. I assume that JS code was generated from that: https://github.com/asciidoctor/asciidoctor-template.js/blob/68626125a426ef0e04e10bb48d3c60728c186567/lib/asciidoctor/core_ext/template.rb#L3

[s-leroux]

@Mogztter Take a look at the pull request https://github.com/s-leroux/asciidoctor.js-pug/pull/2.

I think it implements the API as you suggested above.

[mojavelinux]

one thing that confused me when reading the docs, was the exact relationship between a "converter" and a "backend".

A "converter" is the component that performs conversion from AsciiDoc to another format. A "backend" is a binding word (think of it like a keyword) that activates a converter. The backend expresses an intent to transform to a certain format. Multiple converters can bind to the same backend to provide differ solutions for performing that conversion (last one wins, of course). (We might have used the word "format", but "backend" was the already established term in the AsciiDoc ecosystem).

[mojavelinux]

I 100% agree that a pure JS solution is the way to go. I believe that can be achieved now that the Asciidoctor.js API has matured.

we have to "override" or "patch" the default TemplateConverter defined in Asciidoctor core: asciidoctor/asciidoctor:lib/asciidoctor/converter/template.rb@master

It's fine to bend the TemplateConverter to your needs. I even do that myself in the Bespoke converter (though that's in Ruby). See https://github.com/asciidoctor/asciidoctor-bespoke/blob/master/lib/asciidoctor-bespoke/converter.rb. In fact, there I just started with the composite converter and implemented the same logic. This is perfectly acceptable. As long as it works the same from the perspective of the user (meaning it supports the template_dirs option).

[s-leroux]

(emphasis mine)

This is perfectly acceptable. As long as it works the same from the perspective of the user (meaning it supports the template_dirs option).

I noticed that in the doc: the template_dir option is deprecated and replaced by template_dirs--which is said to be an array. Few questions about that:

• Does that mean we should read all templates from all path provided in the array?
• Is it expected for the API to silently convert a string to an array of only one item when the caller pass a string in template_dirs instead of an array?
• What is there are several templates for the same element in the different paths provided in template_dirs?

[mojavelinux]

Does that mean we should read all templates from all path provided in the array?

Correct.

Is it expected for the API to silently convert a string to an array of only one item when the caller pass a string in template_dirs instead of an array?

Core takes care of this. See https://github.com/asciidoctor/asciidoctor/blob/b6c8ecde8b0efd740f3ce356658be9a851e911f7/lib/asciidoctor/document.rb#L1075-L1099

What is there are several templates for the same element in the different paths provided in template_dirs?

Last wins.

[s-leroux]

Thank you for the clarifications Dan.

One thing though:

Is it expected for the API to silently convert a string to an array of only one item when the caller pass a string in template_dirs instead of an array?

Core takes care of this. See https://github.com/asciidoctor/asciidoctor/blob/b6c8ecde8b0efd740f3ce356658be9a851e911f7/lib/asciidoctor/document.rb#L1075-L1099

I can see how the string template_dir (no s) is converted to an array template_dirs (with s): https://github.com/asciidoctor/asciidoctor/blob/b6c8ecde8b0efd740f3ce356658be9a851e911f7/lib/asciidoctor/document.rb#L1079

 converter_opts[:template_dirs] = [template_dir]

But I can't see how/if template_dirs (s) is mapped to an array if the user provided a string in that options. In other words, is my current implementation consistent with other implementations:

    const doc = asciidoctor.load("Hello world",
                     { template_dirs: './path/to/template/directory' });

Or should template_dirs to be mandatory provided as an array instead:

    const doc = asciidoctor.load("Hello world",
                     { template_dirs: [ './path/to/template/directory' ] });

Or don't bother since it will be passed "like that" to the converter?

[mojavelinux]

My mistake. I misspoke. It does not convert template_dirs to an Array, as you have pointed out. However, I think it should. For now, it's best to handle it if you want to support it.

[mojavelinux]

Or should template_dirs to be mandatory provided as an array instead:

To be consistent with core, it should be mandatory atm.