Preloaded templates - Githubissues

danShumway commented 6 years ago

Thanks for all the effort you put in on this! I've been adapting the code from this extension to set up templates on my personal site. It's largely the same implementation, with a couple notable API differences:

The user passes in template instead of template_dirs.
The user compiles their templates before passing them in.

var content = [
            '= Title',
            '',
            '== Content'
        ].join('\n');

        var output = asciidoctor.convert(content, {
            header_footer : true,
            templates : {
                document : function (ctx) {
                    var content = ctx.node.content();
                    return `<div>${ content }</div>`;
                },
                section : function () {
                    return 'test';
                }
            }
        });

A couple of reasons for these changes:

I want to be able to pick my own templating language without worrying about whether or not AsciiDoctor.js supports it.
If the templating language supports it, I want to be able to use helpers and extensions, which requires me to do the template loading myself.
I want to be able to add custom wrappers or patches around templates without going through an AsciiDoctor specific API.
I want to be able to use templates in the browser where I won't have file access. Even if I did have file access I'd probably still want to pre-compile my templates and bundle them in my source.

I have a working implementation that I've been using while I put together my own site.

I think template_dirs is important, both for keeping parity with AsciiDoctor's Ruby API and just because people shouldn't need to compile their own templates if they don't want to. But for people who do want to, would it be worthwhile to try and add support for a second option alongside template_dirs?

danShumway commented 6 years ago

Related to #1.

One niggle with my approach is that I don't think AsciiDoctor's base Ruby API has a way to pass in precompiled templates, and it would be good to keep the same API.

But maybe it should? I'm guessing there would be times even in Ruby where you might want to do something weird with templates.

s-leroux commented 6 years ago

Hi Dan,

I think template_dirs is important, both for keeping parity with AsciiDoctor's Ruby API and just because people shouldn't need to compile their own templates if they don't want to. But for people who do want to, would it be worthwhile to try and add support for a second option alongside template_dirs?

I planned to work on that this WE :) Indeed, I modified my initial implementation to comply with the Ruby API for templates. But I'm not entirely satisfied with that.

For example, I mentioned here an use case I cannot solve elegantly with the current template API. This is something I need to change.

Your solution is interesting as it passes the template as an object. I had something in the same spirit in my mind. As of myself, I would have opted for an array or even a Map--but your solution is probably better. I hope I will have the time to work on that while it is still fresh in my mind!

danShumway commented 6 years ago

As of myself, I would have opted for an array or even a Map

Object literals feel cleaner to me, but that's probably just personal preference. :)

I most likely won't be doing much development on this over the weekend, but I'll be starting development up again on Monday. I'll try to keep you in the loop about anything I do, and I'll keep an eye on both asciidoctor.js-pub and asciidoctor-template.js as well.

ggrossetie commented 6 years ago

I have a working implementation that I've been using while I put together my own site.

The code looks good!

The user compiles their templates before passing them in.

This is a very interesting idea 👍 I've been thinking a lot about how we could support multiple template engines... and this could be the solution! Also the API is clean, I like it 😄

cc: @mojavelinux

s-leroux commented 6 years ago

I spend quite some time working on that today:

https://github.com/s-leroux/asciidoctor.js-pug/tree/issue/3

Take a look at the README file if you have some time, it gives a couple of examples.

s-leroux commented 6 years ago

Changes

The version in the issue/3 branch is a major rewrite. I'm now much less dependent on Opal. And while maintaining the consistency with the Ruby API, I improved it in several ways I think:

first I "borrowed" Dan's idea of using JS objects as templates; In my implementation, the templates parameter is an array similarly to template_dirs.
for efficiency, you can explicitly load templates before passing them in the templates option. This avoids reloading the content of the various template directories for each document we process
I've implemented a next() call allowing to pass control to the next template in the chain. This is useful to conditionally implement a template based on role or attribute. As well as to implement templates using decorator-like pattern.

For few code examples:

Load the templates from a directory but override some production rules

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

Process differently blocks based on a role

const doc = asciidoctor.load(someDoc, {
  templates: [{
    paragraph: (ctx) => {
      if (ctx.node.roles.has("SECRET")) {
        return '<div>CENSORED</div>';
      }

      // else
      return ctx.next();
    },
  }],
});

Decorators

const doc = asciidoctor.load(someDoc, {
  templates: [{
      image: (ctx) => {
        return `<div class="image">${ctx.next()}</div>`;
      },
    }],
});

danShumway commented 6 years ago

In my implementation, the templates parameter is an array similarly to template_dirs.

I like this, but I'd advocate that the API should just implicitly wrap an object literal in an array if one isn't supplied. If it's possible to detect that the user forgot an array, it's probably better to just fix the problem for them.

It looks like you're already doing that with template_dirs:

if (typeof template_dirs == "string") {
    template_dirs = [ template_dirs ];
}

Othewise, I've only just skimmed through the code/readme, but at first glance I like this API a lot. It solves a bunch of problems.

You can do decorators without writing a single line of Javascript, you can really quickly combine custom templates in code with a template directory you've downloaded off the Internet, you can build pseudo-extensions without making your docs impossible to build anywhere else, you can can take the output of a template and pass it into a completely different templating language...

Heck, since raw Javascript is supported, you can even do stupidly weird stuff like rewrite the output of other templates :)

asciidoctor.load(someDoc, {
   templates: [{ image: (ctx) => {

       //Ideally here you'd do something at least semi-useful instead,
       //like... I'm not sure, minification or something?
       return ctx.next().replace('e', 'i'); 
   }}] 
});

It's exactly what I want in an API - a very small number of straightforward rules that can be combined in a bunch of different ways.

s-leroux commented 6 years ago

Thanks for your reply @danShumway

I like this, but I'd advocate that the API should just implicitly wrap an object literal in an array if one isn't supplied.

I don't know why I didn't code defensive here. That will be done.

Heck, since raw Javascript is supported, you can even do stupidly weird stuff like rewrite the output of other templates :)

I didn't think about that, but indeed we can minimize or do some other kind of post-processing on the output of the other templates. But for that purpose maybe we can consider some form of catch-all template. Or would that go against the goal of having a clean and simple API?

danShumway commented 6 years ago

But for that purpose maybe we can consider some form of catch-all template.

I dunno, I'm not sure it would be necessary. document and embedded are already kind of catch all templates, since content() will also render any child blocks.

asciidoctor.load(someDoc, {
    header_footer: true,
    templates : [{ document : (ctx) => minify(ctx.next()) }]
});

I could very well be missing a use case though.

s-leroux commented 6 years ago

I dunno, I'm not sure it would be necessary. document and embedded are already kind of catch all templates, since content() will also render any child blocks.

Yes, that certainly does make sense--at least according to my own experience with Asciidoctor.

ggrossetie commented 6 years ago

That's awesome! Well done @s-leroux and @danShumway 👍 Do you think it could be possible to abstract the Pug converter ? Something like:

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

With a well defined API, a user could use its favorite template engine... but I think you are already working on it ? I just saw this commit: https://github.com/s-leroux/asciidoctor.js-pug/commit/2c385d46c17a137949cdc2bfb0f2cfddab01ed1c 😉

s-leroux commented 6 years ago

With a well defined API, a user could use its favorite template engine... but I think you are already working on it ? I just saw this commit: 2c385d4 wink

@Mogztter Let's continue the discussion about that particular topic here if we may: https://github.com/s-leroux/asciidoctor.js-pug/issues/5

mojavelinux commented 6 years ago

I'll just jump in to say that a templates option providing JavaScript objects that are used as templates is right in line with the vision I had from the start with templates. It's possible to do this in the Ruby version using Tilt, but what you've got going here is much cleaner and expressive. And I love the delegation chain idea. That is something I had not though of doing. And it's choice!

s-leroux / asciidoctor.js-pug

Preloaded templates #3

Changes

Load the templates from a directory but override some production rules

Process differently blocks based on a role

Decorators