asciidoctor / docgist

Render AsciiDoc documents from Gists, GitHub, DropBox and other remote sources in the browser.
http://gist.asciidoctor.org
57 stars 79 forks source link

Images support for GitHub File #14

Closed jmini closed 9 years ago

jmini commented 9 years ago

I try to render the example-manual.adoc page from the maven example repository with gist.asciidoctor.org.

Url of the file:

https://github.com/asciidoctor/asciidoctor-maven-examples/blob/master/asciidoc-to-html-example/src/docs/asciidoc/example-manual.adoc

Links:

In the Images section, the image is broken. It looks like this:

Images section screenshot

The src attribute of the img tag is wrong:

<h2 id="_images"><a class="anchor" href="#_images"></a>Images</h2>
<div class="sectionbody">
<div class="imageblock thumb">
<div class="content">
<img src="images/sunset.jpg" alt="sunset.jpg">
</div>
</div>
</div>
</div>

It should be:

<img src="https://raw.githubusercontent.com/asciidoctor/asciidoctor-maven-examples/master/asciidoc-to-html-example/src/docs/asciidoc/images/sunset.jpg" alt="sunset.jpg">
nawroth commented 9 years ago

We should make that work -- I'll have a look at it.

mojavelinux commented 9 years ago

If you know you're rendering a file from GitHub in docgist (which I'm pretty sure is true), you can set the imagesdir to a qualified URL. Though, you'd need to read in the imagesdir from the header (just like we do with source-highlighter), prepend the URL to that value, then pass the new attribute value to the API. This way, you respect the relative imagesdir path set in the document.

nawroth commented 9 years ago

I did an attempt at it, see: http://nawroth.github.io/docgist/?github-asciidoctor%2Fasciidoctor-maven-examples%2F%2Fasciidoc-to-html-example%2Fsrc%2Fdocs%2Fasciidoc%2Fexample-manual.adoc

@mojavelinux Could you check if I got the logic around imagesdir right?

nawroth commented 9 years ago

In the case where the imagesdir attribute hasn't been set in the document things work fine: http://nawroth.github.io/docgist/?github-nawroth%2Fdocgist%2F%2Fgists%2Fexample.adoc But as soon as it has been set in the document I'm failing to override it: http://nawroth.github.io/docgist/?github-nawroth%2Fdocgist%2F%2Fgists%2Fimages.adoc The code is here: https://github.com/nawroth/docgist/blob/master/js/docgist.js#L32 As soon as the rendering is invoked, the attribute is reverted back to the value set in the document.

On a brighter note, I got Prettify support in place: http://nawroth.github.io/docgist/?github-nawroth%2Fdocgist%2F%2Fgists%2Fprettify.adoc You can see the difference compared to highlight.js: http://nawroth.github.io/docgist/?github-nawroth%2Fdocgist%2F%2Fgists%2Fexample.adoc#_source_code_highlighting

mojavelinux commented 9 years ago

As soon as the rendering is invoked, the attribute is reverted back to the value set in the document.

I think I know what has to be done. I'll look at it and come back with an explanation.

I got Prettify support in place

Fantastic! There's no doubt that prettify does a more accurate job of highlighting the major languages, so this is a very nice improvement!

mojavelinux commented 9 years ago

The reason the imagesdir attribute is being reset is because, prior to the convert step, the attributes defined up to the end of the header are restored to their original values (see https://github.com/asciidoctor/asciidoctor/blob/master/lib/asciidoctor/document.rb#L1007). This is an issue in the Asciidoctor API that needs to be addressed (see https://github.com/asciidoctor/asciidoctor/issues/1255).

There are two workarounds to this problem. I'll describe the simpler, quicker fix first, then explain how we might do this in a more formal (yet slightly slower) way.

Modify the original attributes

The quick fix is to make modifications directly to the original_attributes instance variable on the document. That way, when the attributes are restored, it includes the modifications made by the API. Here's how that would be done:

var attributes = doc.attributes.map;
doc.original_attributes.$update(Opal.hash({imagesdir: imageBasePath + attributes.imagesdir}))

You must use the $update function to make changes to the Hash (not []) given how Opal stores the Hash internally.

Load in two phases

The more formal way to solve this problem (until we are able to correct the API), is to load in two phases.

You begin by loading the header only.

doc = Opal.Asciidoctor.$load(content, Opal.hash({parse_header_only: true}))

From the doc, you can read the attributes that are set up to the end of the header. You can cherry pick the attributes you want, modify them as needed, then pass them to the API as overrides. In fact, at this point, you don't even have to call $load again, you could just proceed directly to Asciidoctor.$convert. Since you are passing attributes directly to the API, they will override anything in the document.

Make sense?

nawroth commented 9 years ago

@mojavelinux I went with the two-phase option, as modifying the original attributes had the same problem as my first attempt (that is, they are still reset to the original value).

mojavelinux commented 9 years ago

Probably for the best that we're using the public API :)

mojavelinux commented 9 years ago

Nice job!

mojavelinux commented 9 years ago

\o/

nawroth commented 9 years ago

@jmini The example you gave now works.

jmini commented 9 years ago

Thank you a lot. ;-)