Register this converter for "reveal.js"

[ggrossetie]

Since the project name is "reveal.js" I think this converter should be registered for "revealjs" (for backward compatibility) and "reveal.js".

[mojavelinux]

To provide context, is the current backend name for the converter "revealjs"?

[ggrossetie]

To provide context, is the current backend name for the converter "revealjs"?

Yes 😉

[mojavelinux]

Then I agree.

[ggrossetie]

@mojavelinux What about the attribute backend-revealjs? Should we replace invalid characters when the processor adds this attribute? backend-reveal.js is invalid right?

https://github.com/asciidoctor/asciidoctor/blob/2acdcece395a8327373b5f7ebf01c71dcd80ec24/lib/asciidoctor/document.rb#L1321

So it should produce backend-reveal-js? Another idea is to use ifeval::["{backend}" == "reveal.js"] but it's verbose.

With the benefit of hindsight, I don't think it's worth it. It will most likely add confusion and it has no real benefit for the end-user.

[obilodeau]

It's unclear to me what should happen here. Is the 3.0.0 release an opportunity to improve things or not?

[mojavelinux]

What about the attribute backend-revealjs?

That's a really good point. That's a strong case for leaving "revealjs" as the official backend name.

Though the following would still work:

ifdef::backend-reveal.js[]

In the end, this comes down to a usability issue. Do you think people will try to use -b reveal.js and be confused when it doesn't work?

You can register under multiple names:

register_for 'revealjs', 'reveal.js'

As of now, though, the converter can't normalize the backend so that you get consistent value out of it. It might be nice if you could do:

def initialize backend, opts
  backend = 'revealjs' if backend == 'reveal.js'
  super
end

But that doesn't work.

So maybe it's best to just stick with a consistent name. If revealjs is fine for users, then I suppose it's not worth the cost to change it.

[mojavelinux]

So the real question is, should the converter work with a parallel backend alias named reveal.js? The user would have to know to consistently use this name when checking the value of the backend.

[obilodeau]

Doing the register_for trick is fairly easy and, to me, seems without foreseeable adverse effects. What I'm worried about here is breaking backward compatibility.

The backend test is clearly documented in the README as being revealjs. As long as this one work still works I'm not that worried.

I'm going to do some tests. Expect a PR.

[obilodeau]

So, with this patch, tests fails:

diff --git a/Rakefile b/Rakefile
index 414f5aa..4560ed1 100644
--- a/Rakefile
+++ b/Rakefile
@@ -71,7 +71,7 @@ def build_converter(mode = :pretty)
     Asciidoctor::TemplatesCompiler::RevealjsSlim.compile_converter(
       templates_dir: TEMPLATES_DIR,
       class_name: 'Asciidoctor::Revealjs::Converter',
-      register_for: ['revealjs'],
+      register_for: ['revealjs', 'reveal.js'],
       backend_info: {
         basebackend: 'html',
         outfilesuffix: '.html',
diff --git a/examples/multi-destination-content.adoc b/examples/multi-destination-content.adoc
index 1530ed6..b01b945 100644
--- a/examples/multi-destination-content.adoc
+++ b/examples/multi-destination-content.adoc
@@ -49,7 +49,7 @@ Some overview
 +-------------------------------------------------+
 ----
 
-ifdef::backend-revealjs[=== !]
+ifdef::backend-reveal.js[=== !]
 
 Detailed view
 

Meaning that registering the backend is not enough for it to change the ifdef backend syntax. So, we are silently going to accept the dotted name for the back-end to reduce surprises if someone use this but we are not going to document it or say it is the preferred form. Existing advanced users keep functionality and we reduce surprises for new users. New users who want advanced ifdef on this backend can search for the documentation if things don't work out of the box.

[mojavelinux]

I think that's a very fair description of what it should be. Users get an early win by accidentally using -b reveal.js instead of -b revealjs. But they will have to learn to use -b revealjs for it to work consistently with what the test suite is verifying.

[ggrossetie]

Yes I think that's a great compromise.

On a side note, why does an attribute name cannot contain dots? Is there an usage where it would be confusing for the parser? Spaces should definitely not be allowed.

[mojavelinux]

On a side note, why does an attribute name cannot contain dots?

Historical reasons.

We also kept it reserved just in case we want to do something like: asciidoctor/asciidoctor#528

[obilodeau]

This is resolved now that #297 is merged.