s-leroux / asciidoctor.js-pug

Allow to override the html5 output of asciidoctor.js with pug templates
3 stars 2 forks source link

Use the `template_engine` option to customize the template engine #5

Closed s-leroux closed 6 years ago

s-leroux commented 6 years ago

See https://github.com/s-leroux/asciidoctor.js-pug/issues/3#issuecomment-376004800

Currently, the template engine is identified using the file name extension while loading a template directory. The user may wish to override that by explicitly specifying a template engine as described in the Asciidoctor manual

According to the doc, template_engine is the "Template engine name (e.g., slim, haml, erb, etc.)

Does that mean, when specified, all templates should be processed by the engine named template_engine regardless of their file name? @Mogztter is that you had in mind?

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

I see two different things here:

  1. registering a new template engine and indeed https://github.com/s-leroux/asciidoctor.js-pug/commit/2c385d46c17a137949cdc2bfb0f2cfddab01ed1c is supposed to be the first step toward that,
  2. overriding the "auto selection" of the template engine. Currently, it is based on the extension of the template to be loaded.
danShumway commented 6 years ago

When you talk about overriding auto selection, do you mean turning off auto selection entirely or just overriding what the auto selection is for a specific extension?

s-leroux commented 6 years ago

When you talk about overriding auto selection, do you mean turning off auto selection entirely or just overriding what the auto selection is for a specific extension?

I'm absolutely not sure of the role of the template_engine option. As far as I understood the doc it turns off auto selection entirely:

image

Certainly, overriding auto-selection for a given extension seems more useful to me at first sight. Unless there are some use cases I missed?

@Mogztter do you know how this option is used in practice?

ggrossetie commented 6 years ago

In my opinion, the current template_engine is not really useful.

What I had in my mind was an easy way to override the template engine. In other words, if the template engine is defined we should use it to compile the templates.

overriding the "auto selection" of the template engine. Currently, it is based on the extension of the template to be loaded.

I don't know if it's really useful if you can register/override template engine.

My idea is that the template converter should not be bound to a template engine. But at the same time it should be really straight forward to register a template engine:

{
  template_engine: require ('asciidoctor-template-engine-pug')
}
{
  template_engines: [{
    patterns: ['*.jade', '*.pug'],
    engine: require('asciidoctor-template-engine-pug')
  },
  {
    pattern: '*.mustache',
    engine: require('asciidoctor-template-engine-mustache')
    },
    {
      pattern: '*.bar',
      engine: {compile: function(file) {}}
    }]
}
danShumway commented 6 years ago

I agree with registering per-pattern - if you can override a specific pattern, you can also override the entire engine.

{
  template_engines: [{
     pattern: '*',
     engine: require('assciidoctor-template-engine-full')
   }]
}

So it doesn't look like anything is lost by being pattern specific.

s-leroux commented 6 years ago

Thank you for your comments. I will probably look at that this WE.

@Mogztter A question about pattern vs patternS. It leads to some corner cases where both are defined, and it is debatable how this should be handled.

I would suggest instead to use only patternS, taking an array of patterns as argument. And, as this is already done in several places, we could silently convert a single string to an array of length 1 internally.

ggrossetie commented 6 years ago

A question about pattern vs patternS. It leads to some corner cases where both are defined, and it is debatable how this should be handled. I would suggest instead to use only patternS, taking an array of patterns as argument. And, as this is already done in several places, we could silently convert a single string to an array of length 1 internally.

Excellent idea! 👍

s-leroux commented 6 years ago

I progressed on that idea: https://github.com/s-leroux/asciidoctor.js-pug/tree/issue/5/template_engine

I didn't have the time to update the README file but here is an example straight out of the test file:

      const doc = asciidoctor.loadFile('./test/data/005-img-uri.adoc', {
        template_dirs: [
          './test/templates-alt'
        ],
        template_engines: [{
          patterns: "*.xyz",
          engine: require('../lib/template-engines/pug.js'),
        }],
      });
      const html = doc.convert();

As an alternate API, it also supports that syntax:

      const composite = require("../lib/template-engines/composite.js");
      const engine = composite.create()
                      .register("*.xyz", require('../lib/template-engines/pug.js'));

      const doc = asciidoctor.loadFile('./test/data/005-img-uri.adoc', {
        template_dirs: [
          './test/templates-alt'
        ],
        template_engines: engine,
      });
      const html = doc.convert();
s-leroux commented 6 years ago

... or that BTW if you want to process every file with the same template engine regardless of the filename:

      const doc = asciidoctor.loadFile('./test/data/005-img-uri.adoc', {
        template_dirs: [
          './test/templates-alt'
        ],
        template_engines: require('../lib/template-engines/pug.js'),
      });
      const html = doc.convert();
s-leroux commented 6 years ago

Closed by https://github.com/s-leroux/asciidoctor.js-pug/commit/c25276307471592e075816639c5c7d921f695a65