Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Callout not properly rendered with slim & reveal.js #65

Closed
robertpanzer opened this issue Sep 29, 2014 · 13 comments
Closed

Callout not properly rendered with slim & reveal.js #65

robertpanzer opened this issue Sep 29, 2014 · 13 comments
Assignees

Comments

@robertpanzer
Copy link
Member

When I use the slim reveal.js template a XML listing with callouts the callout is rendered wrong.
With this example:

[source,xml]
----
<?xml version="1.0" encoding="UTF-8"?>
<ivy-module version="2.0" xmlns:m="http://ant.apache.org/ivy/maven">
    <info organisation="foo"
        module="Bar"
        revision="1.0.0-SNAPSHOT">
    </info>
    <configurations> <!--1-->
        <conf name="compile"/>
        <conf name="compile-only" extends="compile"/>
        <conf name="runtime" extends="compile"/>
        <conf name="test-compile" extends="compile"/>
        <conf name="test" extends="runtime, test-compile"/>
    </configurations>
    <dependencies> <!--2-->
        <dependency org="org.jboss.spec" name="jboss-javaee-6.0" rev="${version.jboss-javaee}" conf="compile-only,test-compile->default(*)"/>
        <dependency org="org.aspectj" name="aspectjrt"  rev="${version.aspectj}"    conf="compile->default(*)"/>
        <dependency org="commons-io"  name="commons-io" rev="${version.commons-io}" conf="compile->default(*)"/>
    </dependencies>
</ivy-module>
----

the callout gets rendered like this:

<section id="_basic_structure_ivy_xml"><h2>Basic structure ivy.xml</h2><div class="listingblock"><div class="content"><pre class="highlight"><code class="xml language-xml">&lt;?xml version="1.0" encoding="UTF-8"?&gt;
&lt;ivy-module version="2.0" xmlns:m="http://ant.apache.org/ivy/maven"&gt;
    &lt;info organisation="foo"
        module="bar"
        revision="1.0.0-SNAPSHOT"&gt;
    &lt;/info&gt;
    &lt;configurations&gt; <b>(1)</b>

So <b> and </b> are visible as text in the rendered HTML.

This does not occur when I use the haml template. :-)
But it in this case the listing is not as pretty. :-(

@mojavelinux
Copy link
Member

Note, the haml templates are behind the slim templates and will soon be removed once we verify we've ported any missing bits.

I'm not surprised that callouts don't work since it requires dedicated logic in the template to get them right which may have not been done. Do you want to attempt to port the logic from the haml (if worth salvaging) and fixing it up so they look good when using the slim templates?

@robertpanzer
Copy link
Member Author

I'll try my best! :)
Can take some time though.

@mojavelinux
Copy link
Member

We'll be here to help! @obilodeau might be able to pitch in as well, but the more hackers, the merrier!

@robertpanzer
Copy link
Member Author

Well, it's rather the other way around at first look.
The haml template has no logic for that, but the slim template takes special care of it.

@mojavelinux
Copy link
Member

Check the deck.js backend. I know that it has support for such things...and a very similar output is needed.

@robertpanzer
Copy link
Member Author

Does not seem to be a problem in the templates.
With this example:

[source]
----
<xml> <!--1-->
----

the result in the HTML file is <div class="listingblock"><div class="content"><pre class="highlight"><code>&lt;xml&gt; <b>(1)</b> which is correct.
But when I open the file in the browser the content flickers and opening the DOM shows this:

<div class="listing block">
  <div class="content">
    <pre class="highlight">
      <code class="apache">
        <span class="tag">&lt;xml&gt;</span>
          <span class="tag">&lt;b&gt;</span>(1)<span class="tag">&lt;/b&gt;</span>
...

When I remove the source name from the block the error does not occur anymore, that is the <b> and </b> disappear and the number is rendered bold like it should be.

So I assume that the error is in the rather old version of highlight.js that is packaged with reveal.js.

So I downloaded a current version of highlight.js, replaced it in reveal.js/plugin/highlight and the error disappeared completely also when using the source name!

@robertpanzer
Copy link
Member Author

Closing as the issue is not related to the template.
Instead reveal.js should update its version of highlight.js if possible.

@mojavelinux
Copy link
Member

So I downloaded a current version of highlight.js, replaced it in reveal.js/plugin/highlight and the error disappeared completely also when using the source name!

Excellent! Would you be willing to add this note to the README of this repository under the reveal.js location?

when I open the file in the browser the content flickers and opening the DOM shows this:

The use of client-side source highlighters are always a little shaky when callouts are present in the source. That's because callouts are technically invalid characters in the source that the source highlighter is attempting to process. Sometimes, we get lucky and it works, as it sounds like is the case with the newer highlight.js.

In general, I recommend using a server-side source highlighter (e.g., coderay or pygments) when possible. This gives us the opportunity to insert the callouts after source highlighting is complete...thus giving us 100% accuracy. Just something to note.

@mojavelinux
Copy link
Member

Thanks for digging into this @robertpanzer!

@robertpanzer
Copy link
Member Author

Sure, can do!
Would it be worth trying to enable Coderay in the templates?
Because as far as I could see loading highlight.js is currently burnt in.

@robertpanzer robertpanzer reopened this Oct 11, 2014
@mojavelinux
Copy link
Member

Yes, it would be nice to have at least CodeRay (or one of the others like Prism or Rouge) as an option. Having said that, the CodeRay work will mostly be done in Asciidoctor core (as you can see here: https://github.com/asciidoctor/asciidoctor/blob/master/lib/asciidoctor/substitutors.rb#L1433). The template just needs to add a default highlighting theme (like the one in Asciidoctor core here: https://github.com/asciidoctor/asciidoctor/blob/master/data/stylesheets/coderay-asciidoctor.css)

mojavelinux added a commit that referenced this issue Oct 11, 2014
Addresses issue #65 add support for coderay & pygments
@robertpanzer
Copy link
Member Author

Fixed with fcf0a57
Added note for highlight.js to README.
Added support for using coderay and pygments as source-highlighters.

@mojavelinux
Copy link
Member

\o/

Thanks @robertpanzer!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants