Compiling slim templates into pure ruby (simplifies Javascript maintenance) #131

obilodeau · 2017-08-24T03:37:02Z

I'm leveraging asciidoctor-templates-compiler by @jirutka. The goal here is to get rid of the jade templates and only have to maintain one set of templates. See #63.

This is not ready, I want doctest (#116) merged in first to see for potential regressions using doctest.
Will rebase/fix once it is in.

Waiting on upstream

Remove replaceUnsupportedFeatures function from npm/builder.js once [Bugfix] Use the specified capture_generator even for nested captures judofyr/temple#112 is merged and released
Remove capture_generator from lib/asciidoctor-templates-compiler.rb once Change default :capture_generator to self judofyr/temple#113 is merged and released (or at least fix the typo capture_generate -> capture_generator)

Thanks to @jirutka for the asciidoctor-templates-compiler! It took me only an hour maybe to get the PoC running.

ggrossetie · 2017-08-24T12:27:16Z

adapt to make it work from javascript

@obilodeau You can remove the following lines:

asciidoctor-reveal.js/lib/asciidoctor-revealjs.rb

Line 3 in a1fddf4

`require('asciidoctor-template.js')`

asciidoctor-reveal.js/package.json

Line 43 in a1fddf4

"asciidoctor-template.js": "1.5.5-3",

Since asciidoctor-template.js is not required anymore.
I'm trying right now to use the generated lib/asciidoctor-revealjs/converter.rb in Asciidoctor.js.

@jirutka @obilodeau Is the json module required ? By default the opal-compiler does not include this module but I can it if necessary.

ggrossetie · 2017-08-24T12:37:48Z

templates/slim/helpers.rb

@@ -3,14 +3,13 @@
 require 'asciidoctor'


This does not work in Asciidoctor.js because Opal will try to load this module from the OPAL_PATH.

It’s probably used only for the version check in this helpers.rb.

Is that version check even necessary given that the way to use this module is through bundler?

It’s not necessary. I suggest to remove it.

ggrossetie · 2017-08-24T12:40:14Z

templates/slim/helpers.rb

@@ -3,14 +3,13 @@
 require 'asciidoctor'
 require 'json'

+# Needed only in compile-time.
+require 'slim-htag' if defined? Slim
+
 if Gem::Version.new(Asciidoctor::VERSION) <= Gem::Version.new('1.5.3')


Opal does not implement Gem. You can still check the version in JavaScript with:

asciidoctor.getCoreVersion(); // available in the next release of Asciidoctor.js

Though you will need to use something like that to compare the version:

https://github.com/asciidoctor/asciidoctor.js/blob/34e64c3e95293a03d459421a55fa34978b971066/spec/share/asciidoctor.spec.js#L3-L7

This check is not necessary, IMHO it should not be included in JS variant.

Repeating my above comment: Is that version check even necessary given that the way to use this module is through bundler for ruby (which would take care of making sure proper version of asciidoctor is there)?

No, it’s not necessary. I suggest to remove it.

ggrossetie · 2017-08-24T13:00:58Z

I'm making progress on my branch https://github.com/mogztter/asciidoctor-reveal.js/tree/compile-slim-templates

NOTE: You should use asciidoctor.js master branch.

Mutable String methods are not supported in Opal:

NotImplementedError: String#<< not supported. Mutable String methods are not supported in Opal.

In the generated converter, we are using String#<<. For instance:

_buf = ''
if (has_role? 'aside') or (has_role? 'speaker')
  _buf << ("<aside class=\"notes\">".freeze)
  _buf << ((content).to_s)
  _buf << ("</aside>".freeze)
else
  _buf << ("<div".freeze)
  # ...
end

Could we use the same strategy as Asciidoctor core and use an Array instead ?

_buf = []
if (has_role? 'aside') or (has_role? 'speaker')
  _buf << ("<aside class=\"notes\">".freeze)
  _buf << ((content).to_s)
  _buf << ("</aside>".freeze)
else
  _buf << ("<div".freeze)
  # ...
end
_buf * LF

I'm not sure about the performance impact but if @mojavelinux is doing it in Asciidoctor core I think this should be fine ?
As an alternative we could replace it in the JavaScript build in order to keep mutable String in Ruby.

@jirutka What do you think ?

ggrossetie · 2017-08-24T14:00:45Z

I've added a hack to replace mutable operations on String: ggrossetie@ef168e0#diff-32d7599354463e657c27d56ccc4b16bfR61

// Replace variable = '' by variable = []
data = data.replace(/([^ ]*) = ''/g, '$1 = \[\]');

// Replace _buf by _buf * ''
data = data.replace(/_buf$/g, '_buf * \'\'');

// Replace variable (alone in a single line) by variable * ''
data = data.replace(/^(\s*)(_[a-z1-9_\[\]]*)$/gm, '$1$2 = $2 * \'\'');

For now I replace in-place the converter.rb file but we could generate a new file (dedicated to Asciidoctor.js)

Here's the result:

= Title

== Slide 1
Content 1

== Slide 2
Content 2

<section id="_slide_1"><h2>Slide 1</h2><div class="paragraph"><p>Content 1</p></div></section>
<section id="_slide_2"><h2>Slide 2</h2><div class="paragraph"><p>Content 2</p></div></section>

jirutka · 2017-08-24T20:02:48Z

Is the json module required ? By default the opal-compiler does not include this module but I can it if necessary.

I don’t know why it’s here, it doesn’t seem to be used in reveal.js templates.

Could we use the same strategy as Asciidoctor core and use an Array instead ?

Yes, just use different Temple generator – Temple::Generators::ArrayBuffer instead of default Temple::Generators::StringBuffer.

There’s currently no configuration option for this in asciidoctor-templates-compiler (I’ll add it), but you can subclass or patch Asciidoctor::TemplatesCompiler::Slim and modify #engine_options.

I've added a hack to replace mutable operations on String…

Hacks are not needed here (see above), Slim and especially Temple (backend for code generation) is nicely configurable.

jirutka · 2017-08-24T20:08:49Z

templates/slim/helpers.rb

@@ -3,14 +3,13 @@
 require 'asciidoctor'
 require 'json'

+# Needed only in compile-time.
+require 'slim-htag' if defined? Slim


This is not required, I’ve added it to my helpers.rb just to allow using Slim templates even directly without ahead of time compilation.

For development it might be useful to avoid the build step. Worth keeping as currently scoped in the unless opal block. I'm very open to remove it if there's a good argument against though.

As you said, it might be useful in development, for someone.

jirutka · 2017-08-24T20:23:33Z

templates/slim/block_table.html.slim

@@ -24,9 +25,10 @@ table(id=@id class=['tableblock',"frame-#{attr :frame, 'all'}","grid-#{attr :gri
                  - cell_content = cell.text
                - else
                  - cell_content = cell.content
-              *{:tag=>(tblsec == :head ? 'th' : 'td'), :class=>['tableblock',"halign-#{cell.attr :halign}","valign-#{cell.attr :valign}"],
+              = html_tag(tblsec == :head || cell.style == :header ? 'th' : 'td',


html_tag is not provided by Asciidoctor (or Slim), it’s defined in my helpers.rb:

https://github.com/jirutka/asciidoctor-html5s/blob/a71db48a1dd5196b668b3a3d93693c5d877c5bf3/data/templates/helpers.rb#L84-L108

I was thinking about creating Slim/Temple extension like slim-htag, but for an arbitrary tag/element. Actually I’ve implemented it and it works, but I don’t know how to name it (and how to name the special attribute) and if it’s really a good thing to do: it’s not idiomatic and it does break The Principle of Least Surprise.

It may look like this:

??? [ ??=(tblsec == :head || cell.style == :header ? 'th' : 'td') class=['tableblock', "halign-#{cell.attr :halign}", "valign-#{cell.attr :valign}"] colspan=cell.colspan rowspan=cell.rowspan style=((@document.attr? :cellbgcolor) ? %(background-color:#{@document.attr :cellbgcolor};) : nil) ]

??? should be name of the special “tag” (e.g. element, dyntag, …?) and ?? name of the special attribute defining the actual tag (e.g. tag, _, …?).

After quickly looking at HTML documentation I would say ??? be html_element and ?? = tag_name but that's far from compact. dyntag and tag would work for me as shorter alternatives.

In the meantime, I've added your html_tag function to our helpers.rb

ggrossetie · 2017-08-24T20:34:03Z

Yes, just use different Temple generator – Temple::Generators::ArrayBuffer instead of default Temple::Generators::StringBuffer.

Super nice 🤓

Hacks are not needed here (see above), Slim and especially Temple (backend for code generation) is nicely configurable.

I didn't know about Temple::Generators::ArrayBuffer, thanks for sharing 👍

There’s currently no configuration option for this in asciidoctor-templates-compiler (I’ll add it), but you can subclass or patch Asciidoctor::TemplatesCompiler::Slim and modify #engine_options.

Ok I will remove my hack and give it a try.

ggrossetie · 2017-08-24T20:58:56Z

Yes, just use different Temple generator – Temple::Generators::ArrayBuffer instead of default Temple::Generators::StringBuffer.

@jirutka It's almost working, the buffer is now an array but there's still some String variables. Do you think we can configure how these variables are generated ?

_buf = []
_buf << ("<div".freeze) # OK

_temple_html_attributeremover1 = ''
_temple_html_attributeremover1 << ((_temple_html_attributemerger1.reject(&:empty?).join(" ")).to_s) # KO

ggrossetie · 2017-08-24T21:16:48Z

verify javascript-side packaging scripts/docs

@obilodeau As mentioned by @jirutka, we should remove the generated file dist/main.js from the repository. This is a common practice in JavaScript but a bad one. I think we should use the same approach as Asciidoctor.js. The dist files are generated by Travis when we need to publish a new release.

jirutka · 2017-08-24T21:29:58Z

It's almost working, the buffer is now an array but there's still some String variables.

It seems that you have to set generator: Temple::Generators::ArrayBuffer and also capture_generator: 'ArrayBuffer', but I’m not sure, it’s quite strange.

obilodeau · 2017-08-25T03:24:47Z

@Mogztter I pulled in your changes and added some of mine on top. Ideally to avoid rebasing it would be nice if you would pull my tree (or start a branch from scratch) so that we keep the history clean.

ggrossetie · 2017-08-25T16:13:05Z

It seems that you have to set generator: Temple::Generators::ArrayBuffer and also capture_generator: 'ArrayBuffer', but I’m not sure, it’s quite strange.

Indeed, we can configure capture_generator on the Temple:Generators:
https://github.com/judofyr/temple/blob/2dafd522d60acc7b912ec0da4756ed2bb0160d15/lib/temple/generator.rb#L12-L15

It's one step closer but the generated code still use String#<<:

before.rb

_buf = []
_buf << ("<div".freeze)
_temple_html_attributeremover1 = ''
_temple_html_attributemerger1 = []
_temple_html_attributemerger1[0] = "paragraph"
_temple_html_attributemerger1[1] = ''

# ... KO!
_temple_html_attributeremover1 << ((_temple_html_attributemerger1.reject(&:empty?).join(" ")).to_s)
_temple_html_attributeremover1

# ... KO!
_temple_html_attributemerger1[1] << ((_slim_codeattributes1).to_s)

after.rb

_buf = []
_buf << ("<div".freeze)
_temple_html_attributeremover1 = []
_temple_html_attributemerger1 = []
_temple_html_attributemerger1[0] = "paragraph"
_temple_html_attributemerger1[1] = ''

# ... OK
temple_html_attributeremover1 << (_temple_html_attributemerger1.reject(&:empty?).join(" "))
_temple_html_attributeremover1 = _temple_html_attributeremover1.join("".freeze)

# ... KO!
_temple_html_attributemerger1[1] << ((_slim_codeattributes1).to_s)

ggrossetie · 2017-08-25T16:25:43Z

@obilodeau I've updated my branch: https://github.com/Mogztter/asciidoctor-reveal.js/commits/compile-slim-templates

EDIT: I've added a .travis.yml file to build both the converter.rb and the converter.js. Though, I'm not sure about the syntax to build a multi-language project 😁
Could you please integrate this commit into your pull request so we can see if the build is working on Travis ? Here's the file: ggrossetie@a03aa0e

obilodeau · 2017-08-25T18:42:20Z

@Mogztter: your changes have been pushed.

ggrossetie · 2017-08-25T19:31:59Z

Thanks!
I made a typo in the file lib/asciidoctor-templates-compiler.rb, it's capture_generator not capture_generate.

Also the build does not work on Ruby 2.1 and Ruby 2.2 since the library asciidoctor-templates-compile requires Ruby >= 2.3.
Should we remove Ruby 2.1 and Ruby 2.2 ?

ggrossetie · 2017-08-25T19:34:09Z

Also the build with jruby-9k does not work:

Bad Syntax
/home/travis/.rvm/gems/jruby-9.1.7.0/gems/ruby-beautify-0.97.4/lib/ruby-beautify.rb:20:in `pretty_string'
...

jirutka · 2017-08-25T22:19:20Z

It's one step closer but the generated code still use String#<<:

Hm, this looks like a bug in Slim or Temple. I’ll take a look at it later.

…since the library asciidoctor-templates-compiler requires Ruby >= 2.3.

Yeah, because I use <<~ (squiggly heredoc) operator introduced in 2.3. asciidoctor-templates-compiler is needed only in build time, so it should not be a problem. You can use different Ruby version to compile the templates. Travis uses rvm, so it should be easy.

~~Or maybe I can replace <<~ with <<- and #unindent from Corefines.~~ (EDIT: I’d be more complicated)

Also the build with jruby-9k does not work:

That’s bad, ruby-beautify executes ruby as external process: https://github.com/erniebrodeur/ruby-beautify/blob/v0.97.4/lib/ruby-beautify.rb#L54-L58. It should be at least parametrized…

You can avoid this error by running only fast compilation (pretty: false).

EDIT: I’ve patched it in asciidoctor-templates-compiler 0.1.3.

jirutka · 2017-08-25T22:30:34Z

templates/slim/helpers.rb

+  ##
+  # This function is from the asciidictor-html5s project
+  # Copyright 2014-2017 Jakub Jirutka <jakub@jirutka.cz> and the Asciidoctor Project.
+  # Licensed under the MIT License.


It’s not needed to copy copyright here; both projects are MIT licensed and contain Asciidoctor project as one of the copyright holders.

jirutka · 2017-08-25T22:31:24Z

templates/slim/helpers.rb

+    attrs_str = attrs.empty? ? '' : attrs.join(' ').prepend(' ')
+
+
+    if VOID_ELEMENTS.include? name.to_s


Where’s VOID_ELEMENTS constant…?

Left behind. I added it.

jirutka · 2017-08-27T23:34:17Z

It's one step closer but the generated code still use String#<<:

Hm, this looks like a bug in Slim or Temple. I’ll take a look at it later.

It’s a bug in Temple, I’ve fixed it and opened PR judofyr/temple#112.

Also the build with jruby-9k does not work:

That’s bad, ruby-beautify executes ruby as external process…

I’ve fixed it, opened PR erniebrodeur/ruby-beautify#43 and patched it in asciidoctor-templates-compiler 0.1.3. It’s now tested against JRuby on CI.

It has also nice side effect – significantly faster code beautification. Thus we don’t need build:converter:fast anymore.

It seems that you have to set generator: Temple::Generators::ArrayBuffer and also capture_generator: 'ArrayBuffer', but I’m not sure, it’s quite strange.

I’ve opened PR judofyr/temple#113 to change default capture_generator to more reasonable value.

ggrossetie · 2017-08-28T08:33:22Z

Awesome! Thanks @jirutka 💯
Hopefully your pull requests will be merged quickly.

ggrossetie · 2017-08-28T08:56:50Z

TODO

Remove Ruby 2.1 and 2.2 from .travis.yml
Remove replaceUnsupportedFeatures function from npm/builder.js once [Bugfix] Use the specified capture_generator even for nested captures judofyr/temple#112 is merged and released
Remove capture_generator from lib/asciidoctor-templates-compiler.rb once Change default :capture_generator to self judofyr/temple#113 is merged and released (or at least fix the typo capture_generate -> capture_generator)

obilodeau · 2017-08-29T04:02:37Z

Merged with master (which now has doctest testing #116). Addressed several comments.

Doctest are failing, I think it is all related to the handling of <h level=X> instead of <hX> (which I've never heard of before). Note: this is still using asciidoctor 1.5.4 (see #132). I'll port to 1.5.6.1 in a different PR.

I'll try to fix doctest failures. It's just too late now to continue working on this.

You can use different Ruby version to compile the templates. Travis uses rvm, so it should be easy.

I could use a hand on this.

jirutka · 2017-08-29T14:21:48Z

Doctest are failing, I think it is all related to the handling of instead of (which I've never heard of before).

This means that slim-htag was not loaded. <h level=X> is not a real element, it should be converted to <hX>.
It is loaded in https://github.com/asciidoctor/asciidoctor-reveal.js/pull/131/files#diff-52c976fc38ed2b4e3b1192f8a8e24cffR18 and even https://github.com/asciidoctor/asciidoctor-reveal.js/pull/131/files#diff-a327a0ed9729c22f5d32cb5593a372aeR7, so I don’t understand why it doesn’t work for you.

obilodeau · 2017-09-02T02:02:49Z

This means that slim-htag was not loaded. is not a real element, it should be converted to .

doctests were not using the ruby back-end but the slim templates directly. Fixed it.

jirutka · 2017-09-02T10:34:33Z

doctests were not using the ruby back-end but the slim templates directly.

Hm, still it should work; templates/slim/helpers.rb:7 does not do the job for some reason. :/

EDIT: It really does not work, because Asciidoctor loads helpers after initializing Slim. So one must take care of loading slim-htag when using templates directly (w/o precompilation). :/

jirutka · 2017-09-04T09:56:12Z

FYI, if you want to fallback to built-in converter for nodes you don’t have template for, set delegate_backend: 'html5' in asciidoctor-templates-compiler.

jirutka · 2017-09-04T16:14:53Z

I’ve released asciidoctor-templates-compiler 0.2.0 with these important additions:

new parameter engine_opts to pass options into Temple Engine,
patch Temple to pass options to capture_generator,
patch Temple to use the same type of generator for capture_generator by default.

So you can remove all the workarounds.

I found out that Opal does not handle String::freeze correctly, so you must disable it when generating converter for Opal: Temple::Generators::ArrayBuffer.new(freeze_static: false). See this revision of Rakefile in my repo.

jirutka · 2017-09-05T00:06:23Z

Rakefile

@@ -28,6 +33,10 @@ namespace :build do
            outfilesuffix: '.html',
            filetype: 'html',
          },
+	  delegate_backend: 'html5',


Indentation… 😠

Fixed. My vim isn't properly setup for ruby still 😞

jirutka · 2017-09-05T00:07:12Z

templates/slim/helpers.rb

@@ -5,10 +5,6 @@

  # Needed only in compile-time.
  require 'slim-htag' if defined? Slim


You can remove this as well, it doesn’t work as intended (for direct use of templates) anyway.

obilodeau · 2017-09-05T00:16:05Z

Current status:

from ruby: everything seems fine
from javascript: I can't even run it, I followed HACKING.adoc doc to set it up (unpublished node module) but I think it needs adjustments now maybe?

$ node build.js 
/home/olivier/src/asciidoc/presentation-asciidoctorjs-test/build.js:3
var Asciidoctor = asciidoctor.Asciidoctor();
                              ^

TypeError: asciidoctor.Asciidoctor is not a function
    at Object.<anonymous> (/home/olivier/src/asciidoc/presentation-asciidoctorjs-test/build.js:3:31)
    at Module._compile (module.js:573:30)
    at Object.Module._extensions..js (module.js:584:10)
    at Module.load (module.js:507:32)
    at tryModuleLoad (module.js:470:12)
    at Function.Module._load (module.js:462:3)
    at Function.Module.runMain (module.js:609:10)
    at startup (bootstrap_node.js:158:16)
    at bootstrap_node.js:598:3

build.js:

// Load asciidoctor.js + asciidoctor-reveal.js
var asciidoctor = require('asciidoctor.js')();
var Asciidoctor = asciidoctor.Asciidoctor();

require('asciidoctor-reveal.js');

// Convert the document 'presentation.adoc' using Reveal.js backend
var attributes = 'revealjsdir=node_modules/reveal.js@';
var options = {safe: 'safe', backend: 'revealjs', attributes: attributes};
Asciidoctor.convertFile('presentation.adoc', options);

package.json:

{
  "name": "presentation-asciidoctorjs-test",
  "version": "1.0.0",
  "description": "",
  "main": "build.js",
  "dependencies": {
    "asciidoctor-reveal.js": "file:../asciidoctor-reveal.js",
    "asciidoctor.js": "^1.5.5-5"
  },
  "devDependencies": {},
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "keywords": [],
  "author": "",
  "license": "ISC"
}

package-lock.json: https://gist.github.com/obilodeau/bfba282a43e5e9572ce1d4f7cdf06b69

@Mogztter any hints?

ggrossetie · 2017-09-05T06:07:23Z

You need to remove this line :

var Asciidoctor = asciidoctor.Asciidoctor();

And use asciidoctor (with lowercase) here :

asciidoctor.convertFile('presentation.adoc', options);

Also attributes can now be defined as JSON object:

var attributes = { 'revealjsdir': 'node_modules/reveal.js@' };

EDIT: ggrossetie@3e9ede2

ggrossetie · 2017-09-05T12:17:55Z

@obilodeau We also need to update bestikk-opal-compiler to 0.3.0-integration7 (based on Opal 0.11), if we want to be compatible with Asciidoctor.js 1.5.6. Though the latest version of Asciidoctor.js 1.5.6-preview.3 does not work because Opal cannot resolve ThreadSafe.Cache. This bug has been fixed on master asciidoctor/asciidoctor.js#370 but I need to publish a new preview version.

obilodeau · 2017-09-05T13:47:16Z

@Mogztter I pulled in your change and updated package.json according to your last comment.

As expected, I'm stuck at the ThreadSafe.Cache issue:

$ node asciidoctor-revealjs.js 

/home/olivier/src/asciidoc/presentation-asciidoctorjs-test/node_modules/asciidoctor.js/dist/asciidoctor.js:26264
              } else { throw $err; }
                       ^
Cache: uninitialized constant ThreadSafe::Cache

I'm going to add a task to wait until the fix is released before rebase/review time.

obilodeau · 2017-09-05T20:06:29Z

You can use different Ruby version to compile the templates. Travis uses rvm, so it should be easy.

@jirutka I tried this. Using two different approaches. I'm close but it still doesn't work...

Can you look at it and give me a pointer?

Attempt 1

Build matrix with shell environment variable and I alter my script's behavior.
obilodeau/asciidoctor-reveal.js@compile-slim-templates...obilodeau:travis-build-attempts-shell

Remaining problems:

on 2.2

it fails because it says it requires temple (althought it shouldn't) but the bundle install --without development acts kind of weird (failed on first attempt then retry works)
https://travis-ci.org/obilodeau/asciidoctor-reveal.js/jobs/272199895

on 2.1

bundle install --without development fails, looks like the --without simply doesn't work
https://travis-ci.org/obilodeau/asciidoctor-reveal.js/jobs/272199903

Attempt 2

Forcing ruby 2.4 on all rake build then back to original ruby.
obilodeau/asciidoctor-reveal.js@compile-slim-templates...obilodeau:travis-build-attempts

Problems:

2.4, 2.3, jruby

rake missing: https://travis-ci.org/obilodeau/asciidoctor-reveal.js/jobs/272211812, jruby: https://travis-ci.org/obilodeau/asciidoctor-reveal.js/jobs/272211820

2.2

LoadError: cannot load such file -- asciidoctor/doctest

https://travis-ci.org/obilodeau/asciidoctor-reveal.js/jobs/272211816

2.1

cannot load such file -- asciidoctor/doctest

https://travis-ci.org/obilodeau/asciidoctor-reveal.js/jobs/272211818

I've spent way to much time on this than I'm willing to admit. Guidance would be appreciated.

jirutka · 2017-09-07T23:24:54Z

There are more traps than I’ve expected, but it works:

I’ll reconsider adding support for older rubies to asciidoctor-templates-compiler.

jirutka · 2017-09-08T21:27:16Z

Okay, I’ve just released asciidoctor-templates-compiler 0.3.0 that is compatible with Ruby 2.1+ (and also adds CLI).

obilodeau · 2017-09-20T03:37:32Z

Tonight:

Will file these issues:

v1.1.0 blocker: wait on the next asciidoctor.js release that integrate this fix: Opal.const_get_qualified does not resolve ThreadSafe.Cache asciidoctor.js#370
integrate javascript-side doctest once Have the ability to use asciidoctor-doctest from the JavaScript Asciidoctor ecosystem asciidoctor-doctest#13 is implemented

Ready for review. I think this work can go into master and get more exposure now.

obilodeau · 2017-09-22T02:26:03Z

I filed #143 and #144. Last chance for reviews before this goes in.

Took the opportunity to drop the `block_` prefix in template names to align with other asciidoctor plugins

ggrossetie · 2017-09-22T06:02:47Z

npm/builder.js

-}
+};
+
+Builder.prototype.replaceUnsupportedFeatures = function (callback) {


Maybe add a comment to state that this is hack that need to be removed once issue XX is resolved ?

This is already not needed, see my comments.

I just removed it and confirmed things reach the same ThreadSafe::Cache error without it. It was reintroduced in the rebase process. The rebase was quite complicated to merge since that branch started before doctest was in and master was pulled in at some point.

obilodeau mentioned this pull request Aug 24, 2017

Jade templates are behind slim templates #63

Closed

12 tasks

ggrossetie reviewed Aug 24, 2017

View reviewed changes

jirutka reviewed Aug 24, 2017

View reviewed changes

jirutka reviewed Aug 25, 2017

View reviewed changes

obilodeau mentioned this pull request Aug 28, 2017

Asciidoctor 1.5.6 introduced a rendering regression for data-background new-style images #132

Closed

obilodeau mentioned this pull request Sep 4, 2017

resolves #132 fix templates to work with 1.5.6 #135

Merged

jirutka reviewed Sep 5, 2017

View reviewed changes

obilodeau and others added 4 commits September 19, 2017 23:17

Migration to asciidoctor-templates-compiler (ruby side)

7c106b7

Make it compatible with Asciidoctor.js

0a2bcdc

Enable Travis build

661c0a7

Documentation updates regarding using the precompiled Ruby converter

ea3e127

obilodeau force-pushed the compile-slim-templates branch from 4ee53dd to ea3e127 Compare September 20, 2017 03:20

obilodeau changed the title ~~[WIP] Compiling slim templates into pure ruby (simplifies Javascript maintenance)~~ Compiling slim templates into pure ruby (simplifies Javascript maintenance) Sep 20, 2017

obilodeau mentioned this pull request Sep 22, 2017

Fix adminition that had '|| @caption' appended #142

Merged

Dropped Jade templates, re-org a bit to be cleaner

b7f1c25

Took the opportunity to drop the `block_` prefix in template names to align with other asciidoctor plugins

obilodeau force-pushed the compile-slim-templates branch from 57157c9 to b7f1c25 Compare September 22, 2017 02:34

obilodeau mentioned this pull request Sep 22, 2017

asciidoctor.js support is currently broken #143

Closed

ggrossetie approved these changes Sep 22, 2017

View reviewed changes

Removed dead code

36afe33

obilodeau merged commit b18f917 into asciidoctor:master Sep 26, 2017

obilodeau deleted the compile-slim-templates branch September 26, 2017 02:52

obilodeau mentioned this pull request Oct 10, 2017

Vertical slides not working #155

Closed

danielhodson mentioned this pull request Jan 19, 2018

--template-dir argument doesn't seem to override the built-in slim templates #177

Closed

		attrs_str = attrs.empty? ? '' : attrs.join(' ').prepend(' ')


		if VOID_ELEMENTS.include? name.to_s

		@@ -5,10 +5,6 @@

		# Needed only in compile-time.
		require 'slim-htag' if defined? Slim

Compiling slim templates into pure ruby (simplifies Javascript maintenance) #131

Compiling slim templates into pure ruby (simplifies Javascript maintenance) #131

Conversation

obilodeau commented Aug 24, 2017 • edited

ggrossetie commented Aug 24, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jirutka Aug 25, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ggrossetie commented Aug 24, 2017

ggrossetie commented Aug 24, 2017

jirutka commented Aug 24, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jirutka Aug 24, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ggrossetie commented Aug 24, 2017

ggrossetie commented Aug 24, 2017

ggrossetie commented Aug 24, 2017

jirutka commented Aug 24, 2017

obilodeau commented Aug 25, 2017

ggrossetie commented Aug 25, 2017

ggrossetie commented Aug 25, 2017 • edited

obilodeau commented Aug 25, 2017

ggrossetie commented Aug 25, 2017

ggrossetie commented Aug 25, 2017

jirutka commented Aug 25, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jirutka commented Aug 27, 2017 • edited

ggrossetie commented Aug 28, 2017

ggrossetie commented Aug 28, 2017 • edited by obilodeau

obilodeau commented Aug 29, 2017

jirutka commented Aug 29, 2017

obilodeau commented Sep 2, 2017

jirutka commented Sep 2, 2017 • edited

jirutka commented Sep 4, 2017

jirutka commented Sep 4, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jirutka Sep 5, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

obilodeau commented Sep 5, 2017

ggrossetie commented Sep 5, 2017 • edited

ggrossetie commented Sep 5, 2017

obilodeau commented Sep 5, 2017

obilodeau commented Sep 5, 2017

Attempt 1

on 2.2

on 2.1

Attempt 2

2.4, 2.3, jruby

2.2

2.1

jirutka commented Sep 7, 2017 • edited

jirutka commented Sep 8, 2017

obilodeau commented Sep 20, 2017 • edited

obilodeau commented Sep 22, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

obilodeau commented Aug 24, 2017 •

edited

ggrossetie commented Aug 24, 2017 •

edited

jirutka Aug 25, 2017 •

edited

jirutka Aug 24, 2017 •

edited

ggrossetie commented Aug 25, 2017 •

edited

jirutka commented Aug 25, 2017 •

edited

jirutka commented Aug 27, 2017 •

edited

ggrossetie commented Aug 28, 2017 •

edited by obilodeau

jirutka commented Sep 2, 2017 •

edited

jirutka Sep 5, 2017 •

edited

ggrossetie commented Sep 5, 2017 •

edited

jirutka commented Sep 7, 2017 •

edited

obilodeau commented Sep 20, 2017 •

edited