Refactor slim/html5 templates using Helpers

[jirutka]

I’ve refactored Slim templates for HTML5 backend in order to separate most of logic into helpers, make templates more clear, simpler and “DRY”.

This PR includes seven previous (not yet merged) PRs and also some future changes/fixes. I’ll rebase it later to make it more clear, so consider this PR just as a preview for review. The most important commit is this.

I depends on changes in the upcoming Asciidoctor 1.5.2.

[mojavelinux]

Nice work! Though, we'll need to hold on this pull request until I get Asciidoctor 1.5.2 (and probably AsciidoctorJ 1.5.2) out the door.

[jirutka]

Note: Please don’t merge it yet, I’m gonna update it.

[mojavelinux]

Excellent.

[jirutka]

@mojavelinux Ready for merge! :smile_cat:

[mojavelinux]

Excellent! I'll review and merge asap!

[jirutka]

@mojavelinux gentle reminder :cat:

[mojavelinux]

Thanks! I'm finally getting around to working through the outstanding pull requests and this one is certainly on my list.

[mojavelinux]

Review complete!

In general, I'm excited by your changes as they drastically reduce the complexity of the templates. I do have to admit that it requires some warming up on my part since I had been intentionally maintaining the long-hand form of all the templates. I guess it really depends on the focus. If the focus is for demonstration purposes, the long-hand is easy to understand. However, if the focus is on keeping the authoring simple, which it should be, then we should be consolidating the logic as much as possible.

There is a risk of overusing the helpers such that you end up creating another Ruby-based template. I think we should keep that in mind. The helpers should keep logic DRY without muddling the meaning of the template. If we feel like the template is not doing enough, we need to swing the pendulum in the other direction. It's clearly a judgment call. Let's just keep your ears and mind open.

[mojavelinux]

Once you address the inline comments, I'll review again and merge.

[mojavelinux]

Give me the thumbs up once you're done with the updates.

[jirutka]

If the focus is for demonstration purposes, the long-hand is easy to understand.

I don’t think so, it was quite hard to understand for me due to a complexity of logic inside templates (that really should not be here).

if the focus is on keeping the authoring simple, which it should be, then we should be consolidating the logic as much as possible.

:+1: Not only that, it’s also important for people who wants to modify the converter for their purpose.

There is a risk of overusing the helpers such that you end up creating another Ruby-based template. … The helpers should keep logic DRY without muddling the meaning of the template.

I agree, there’s a thin line between. I always try hard to do a good trade-off.

[mojavelinux]

I always try hard to do a good trade-off.

Nice. It's important that we've established that strategy because it helps decide which changes to make.

[mojavelinux]

What do you think about block_with_caption instead of block_with_captioned_title?

Another option is to maintain an internal list of the blocks which support a caption in addition to a title and simply handle this internally. Down the road, all blocks that support titles probably need to support a caption anyway, with the caption controlled by whether the block type is configured to support a caption. In other words, it probably doesn't even belong in the template itself (better to hide it away).

[mojavelinux]

Let's add the following Rubies to Travis:

rvm:
  - 2.2.0
  - 1.9.3
  - jruby-19mode

That should cover the major bases...unless you want to add more.

[jirutka]

Let's add the following Rubies to Travis:

rvm:
  - 2.2.0
  - 1.9.3
  - jruby-19mode

DocTest currently needs Ruby ≥ 2.1.0. I can support 2.0.0 (there’s just one place in the code that isn’t compatible with < 2.1), but I really don’t want to support anything older.

Ruby 1.9.3 is obsolete and at the end of life, the same probably applies for jruby-19mode.

btw Since 1.9.3 is at the EOL, it’s IMHO a good time to discuss dropping support of Ruby < 2.0.0 in Asciidoctor core… what do you think? :smiley_cat:

[mojavelinux]

DocTest currently needs Ruby ≥ 2.1.0. I can support 2.0.0 (there’s just one place in the code that isn’t compatible with < 2.1), but I really don’t want to support anything older.

It would be nice to support Ruby 2.0.0 at least until 2.1 is available on Windows. But I'm not going to stress about it.

Ruby 1.9.3 is obsolete and at the end of life. Since 1.9.3 is at the EOL, it’s IMHO a good time to discuss dropping support of Ruby < 2.0.0 in Asciidoctor core

I plan to drop older Rubies in 1.6.0, but not in the 1.5.x series (just to avoid distractions). We've already said we'll be dropping 1.8.7. I haven't made a decision yet about 1.9.3. I will strongly consider it, but I do want to make sure that we don't leave the mainstream users high and dry (trust me, it becomes a major issue for me when this stuff doesn't just work for them).

For what it's worth, Asciidoctor PDF & EPUB3 require at least 1.9.3 and soon to be 2.0.0 at a minimum because of minimum dependencies of requirements of dependencies.

the same probably applies for jruby-19mode.

That's a totally separate thing. JRuby 1.7 doesn't have a complete 2.0 mode, so technically jruby-19mode is the most current...and in some ways has partial 2.0 support. Things will clean up once JRuby 9000 is released...but running jruby-head on Travis CI takes forever.

Having said that, I do think we should add at least jruby-19mode or jruby-head to Travis CI so we know this stuff is tested for AsciidoctorJ users.

[jirutka]

DocTest now supports MRI 2.0.0 and JRuby 9000-dev. Rubinius doesn’t support keyword arguments yet, so it’s out of the game for now.

However, there’s some problem with installing Slim on JRuby (see build log).

btw JRuby is really VERY slooow — both on my machine and Travis — I wonder how anyone can use it… :scream_cat:

[jirutka]

The problem on JRuby causes posix-spawn, dependency of pygments.rb.

Is it really necessary to support pygments.rb when we have CodeRay?

[jirutka]

I’ve just found that you already know a lot about issues with pygments.rb. Well, I’ll disable pygments.rb when running on JRuby.

[jirutka]

Okay, ready for merge! :+1:

Tests on jruby-head passes except block_listing:source_highlighter_pygments (obviously). I’ll add option for selective exclusion of examples in the next version of DocTest.

[mojavelinux]

we don’t need extra helper for captioned titles at all.

\o/

[mojavelinux]

JRuby is really VERY slooow — both on my machine and Travis — I wonder how anyone can use it…

It's definitely not meant for running single executions (like a cli or test suite). The benefit of JRuby is when it runs operations at scale. In that domain, it breezes past anything that doesn't have a hotspot compiler like MRI. It also essential for integration into other JVM languages, hence the value of AsciidoctorJ.

[mojavelinux]

Is it really necessary to support pygments.rb when we have CodeRay?

No, just cut it. In fact, we are strongly considering cutting Pygments from Asciidoctor in favor of Rouge, which is a Pygments port to Ruby. The situation with Pygments.rb is just messy.

[mojavelinux]

Okay, ready for merge!

Super, I'll do a final review asap, but I'm optimistic we've got it the way we want it!

[mojavelinux]

I should be able to get to this in the next day or 2. Finally managed to fend off all the New Year's resolution-fueled e-mails hitting my inbox :)

[mojavelinux]

Looks good! It's time to take the plunge! Thanks for your awesome work on this refactoring and test suite.

:coffee: + :beers: + :chocolate_bar: