redhataccess / ascii_binder

AsciiBinder is an AsciiDoc-based system for authoring and publishing closely related documentation sets from a single source.
https://asciibinder.net
MIT License
76 stars 39 forks source link

Invoke Asciidoctor properly #148

Closed mojavelinux closed 3 years ago

mojavelinux commented 5 years ago

This is a follow-up to a bug that was reported in Asciidoctor core. That led to the discovery that AsciiBinder is not invoking Asciidoctor correctly.

AsciiBinder passes the source document as a String instead of a File object to Asciidoctor here. As a consequence, none of the document-related attributes (e.g., docname, docfile, docdir, etc) get set.

As it turns out, AsciiBinder is already preparing a File object. But then it prematurely calls .read on the object before passing it to Asciidoctor. Asciidoctor will already do for you, but by allowing Asciidoctor to do it, the document-related attributes get set. AsciiBinder is also not closing the File object, which is going to lead to memory leaks. Again, Asciidoctor will handle this for you.

One way to fix the problem is to switch to using the Asciidoctor.load_file method and pass the filename instead (which Asciidoctor will convert internally to a File object).

Asciidoctor.load_file topic.source_path, ...

Another way would be to use the block form of File.open so Asciidoctor receives the File object:

File.open topic.source_path, 'r' do |topic_file|
  Asciidoctor.load topic_file, ...
end

In both cases, the File object will not be left open.

Additionally, the page attributes should be passed as an Hash of name/value pairs instead of as an array of name=value strings. That way, the values are properly escaped.

jboxman commented 4 years ago

@mojavelinux, when I tried to fix this with the first option, I found that include statements in adoc files break. ascii_binder expects an include to be relative to the root of the git repository, I think.

For some reason, this works with Asciidoctor.load. I thought they were using a preprocessor to allow includes relative to the documentation root, but I haven't found any indication of that so far.

mojavelinux commented 4 years ago

ascii_binder expects an include to be relative to the root of the git repository, I think.

Then set the base_dir option.

For some reason, this works with Asciidoctor.load

That's because with no context, Asciidoctor assumes everything is relative to the current directory (it doesn't know about any other location since none of the attributes I mentioned can be set).

vikram-redhat commented 3 years ago

Fixed via: https://github.com/redhataccess/ascii_binder/commit/0ec4f8dfbf15297cf415cd07122941dba96e02ff