asciidoctor / asciidoctor.js

:scroll: A JavaScript port of Asciidoctor, a modern implementation of AsciiDoc
https://asciidoctor.org
MIT License
739 stars 136 forks source link

Includes with absolute paths not working if base_dir is set #273

Closed nerk closed 5 years ago

nerk commented 7 years ago

When explicitly setting the 'base_dir' option, include blocks with absolute paths are no longer working. Asciidoctor.js does not seem to check if a path is absolute or relative.

Ruby Asciidoctor is working, though.

test.ad:

= Include test

include::C:/tmp/inc.adoc[]

Testing from the command line:

asciidoctorjs -B C: test.ad

C:\Users\thomas.kern\AppData\Roaming\npm\node_modules\asciidoctor-cli\node_modules\asciidoctor.js\dist\asciidoctor.js:33252
          } else { throw $err; }
                   ^

Error: ENOENT: no such file or directory, open 'C:\C:\tmp\inc.adoc'
    at Error (native)
    at Object.fs.openSync (fs.js:549:18)
    at $File.ːinitialize (C:\Users\thomas.kern\AppData\Roaming\npm\node_modules\asciidoctor-cli\node_modules\asciidoctor.js\dist\asciidoctor.js:20294:40)
    at Opal.defn.TMP_4 [as $new] (C:\Users\thomas.kern\AppData\Roaming\npm\node_modules\asciidoctor-cli\node_modules\asciidoctor.js\node_modules\opal-runtime\src\opal.js:3139:23)
    at $PreprocessorReader.ːopen (C:\Users\thomas.kern\AppData\Roaming\npm\node_modules\asciidoctor-cli\node_modules\asciidoctor.js\dist\asciidoctor.js:20254:45)
    at $PreprocessorReader.ːpreprocess_include (C:\Users\thomas.kern\AppData\Roaming\npm\node_modules\asciidoctor-cli\node_modules\asciidoctor.js\dist\asciidoctor.js:31684:86)
    at $PreprocessorReader.ːprocess_line (C:\Users\thomas.kern\AppData\Roaming\npm\node_modules\asciidoctor-cli\node_modules\asciidoctor.js\dist\asciidoctor.js:31380:36)
    at $PreprocessorReader.ːpeek_line (C:\Users\thomas.kern\AppData\Roaming\npm\node_modules\asciidoctor-cli\node_modules\asciidoctor.js\dist\asciidoctor.js:30986:41)
    at $PreprocessorReader.ːpeek_line (C:\Users\thomas.kern\AppData\Roaming\npm\node_modules\asciidoctor-cli\node_modules\asciidoctor.js\dist\asciidoctor.js:31411:129)
    at $PreprocessorReader.ːskip_blank_lines (C:\Users\thomas.kern\AppData\Roaming\npm\node_modules\asciidoctor-cli\node_modules\asciidoctor.js\dist\asciidoctor.js:31099:41)

Using Ruby Asciidoctor works:

asciidoctor -B C: test.ad
ggrossetie commented 7 years ago

Actually this is a bug in Opal: https://github.com/opal/opal/pull/1601 This fix should be available in Opal 0.11 (not yet released)

mojavelinux commented 7 years ago

I think this is also related to https://github.com/asciidoctor/asciidoctor/pull/1898, which is coming in 1.5.6.

mojavelinux commented 6 years ago

I would say this is a bug in Opal...possibly still a bug.

The problem is, the File.expand_path implementation in Opal is not correctly expanding a Windows root. This method is used to resolve the value of the base_dir option.

Here's how it works in Ruby:

File.expand_path 'C:'
#=> C:/Users/Username/directory

File.expand_path 'C:/'
#=> C:/

File.expand_path 'D:'
#=> D:/

File.expand_path 'D:/'
#=> D:/

In other words, Ruby always leaves behind at least one slash in the path. And the PathResolver in Asciidoctor assumes this.

In contrast, here's what happens in Opal.

File.expand_path 'C:'
#=> C:/Users/Username/directory/C:

File.expand_path 'C:/'
#=> C:

File.expand_path 'D:'
#=> C:/Users/Username/directory/D:

File.expand_path 'D:/'
#=> D:

So Opal returns the correct result in 0% of the cases :wink:

We have to assume some level of consistency in Asciidoctor, and I'm going to assume the return value of File.expand_path is compliant. Specifically, it must contain at least one slash (and, bonus, it should also recognize a bare drive letter).

Having said that, there's an open question about whether the Asciidoctor PathResolver should recognize C: (without the slash) as a Windows root. If we do that, we also need to change some assumptions in PathResolver#partition_path.

mojavelinux commented 6 years ago

I just tested against Opal master and now all 4 cases returns the drive letter without the trailing slash. I'll report this upstream.

Correction: when using the nodejs module, all the results are still the same as initially reported.

mojavelinux commented 6 years ago

See https://github.com/opal/opal/issues/1794

I'm still debating whether we should handle a bare drive letter (no trailing slash). I kind of want to say no, but it also seems a little fragile not to. :thinking:

mojavelinux commented 6 years ago

This should be scheduled for 1.5.7 instead of 1.5.6.

mojavelinux commented 6 years ago

A workaround you'll be able to use is to set the docdir attribute, which does not get expanded. But you will need to include the trailing slash.

asciidoctorjs -a docdir=C:/ test.ad

See https://github.com/asciidoctor/asciidoctor/blob/6d313e87caf8c05bf8b516e654823ff00a3e263d/lib/asciidoctor/document.rb#L391-L392