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

When header_footer is set to true and safeMode is set to safe or unsafe, building .html files fails. #116

Closed tsiiskonen closed 9 years ago

tsiiskonen commented 9 years ago

Hi,

I'm using Asciidoctor.js and grunt to configure it. I would like to use includes, when safeMode should be set to safe or unsafe (instead of secure that is default), but after I change that, building .html files fails. Only error that is given is to log is "undefined is not a function". If I remove option "header_footer: true" from configuration .html files are built, but I'm missing all the information that is related to styling and also Table of Contents is missing after that. Any Ideas, what I'm doing wrong?

Here are my configuration parameters in Gruntfile:

    asciidoctor: {
        default_options: {
            options: {
                cwd: 'app/docs/',
                showTitle: true,
                showNumberedHeadings: true,
                showToc: true,
                safeMode: 'unsafe',
                //header_footer: true,
                doctype: 'article',
                backend: 'html5'
            },
            files: {
                'app/docs_output/': ['**/*.adoc', '!includes/**/*.adoc', '**/*.md']
            }
        }
    },

I also have other problem, where table of contents is not placed to right block of the pace. Even if I add these line to .adoc file:

:toc: right

nothing happens.

ggrossetie commented 9 years ago

Hi @tsiiskonen, could you post the complete error message please ? (or is "undefined is not a function" the only error message ?)

mojavelinux commented 9 years ago

In addition, please supply a sample AsciiDoc document that demonstrates this error. You can either past it inside fenced code blocks in a command or link to a Gist or file in a GitHub repo.

tsiiskonen commented 9 years ago

"Undefined is not a function" really is the only error message. Here is everything that is printed to log related to asciidoctor task:

Running "asciidoctor:default_options" (asciidoctor) task
Warning: undefined is not a function Use --force to continue.

Aborted due to warnings.

I can reproduce it with for example this kind of .adoc files:

First file, named adocument.adoc:

:toc: right

# Documentation

Here is some text.

image::architecture.png[]

include::architecture.adoc[leveloffset=+1]

## Next header
## Header
## Header
## Header

File that is included contains only one title (architechture.adoc):

# Architecture

And grunt file is having these parameters when error occurs:

        asciidoctor: {
            default_options: {
                options: {
                    cwd: 'app/docs/',
                    showTitle: true,
                    showNumberedHeadings: true,
                    showToc: true,
                    safeMode: 'unsafe',
                    header_footer: true,
                    doctype: 'article',
                    backend: 'html5'
                },
                files: {
                    'app/docs_output/': ['**/*.adoc', '!includes/**/*.adoc', '**/*.md']
                }
            }
        },

And if I comment line

header_footer: true,

Build works, but generated html file does not have any links to css files, so styles are not working.

mojavelinux commented 9 years ago

Why did you close the issue? Isn't this still an issue?

mojavelinux commented 9 years ago

If you comment out header_footer: true, then the result is fundamentally a different thing. The header_footer: true enables a standalone (full) HTML document whereas without this option is just outputs the contents of the body.

tsiiskonen commented 9 years ago

Oh, sorry. It was not meant to happen.

tsiiskonen commented 9 years ago

Yes, and I don't want to comment it, but build is not working when I'm using header_footer and use safeMode: safe. And I have to use safeMode: safe to make includes work.

mojavelinux commented 9 years ago

Cool. Solving that is definitely the goal here :)

mojavelinux commented 9 years ago

I found the likely culprit. It's probably related to the issue that @Mogztter is reporting here: https://github.com/asciidoctor/asciidoctor/pull/1354. Asciidoctor.js can't use the IO.read method, so it can't read the stylesheet data. I found that when I changed that call to File.read, then it still had a problem finding the stylesheet (despite being included in the npm module).

The workaround right now is to set the linkcss attribute and copy the default stylesheet to the output directory manually.

You'll need to add the following entry to the options map:

attributes: 'linkcss'
mojavelinux commented 9 years ago

The focus of this issue should be on getting Asciidoctor.js to support the header_footer option when the linkcss attribute is not set.

tsiiskonen commented 9 years ago

Thank you! That solution worked and I'm able to build project correctly now.

I'm still having that one problem with table of contents. Line

:toc: right

is not placing the toc to right side of the view. It is still on the top right next after the first level header.

mojavelinux commented 9 years ago

is not placing the toc to right side of the view. It is still on the top right next after the first level header.

It's very likely this is a resolution issue. The toc only appears on the side if there is enough room. There is a media query in the stylesheet that returns the toc to the center when the window gets too narrow.

tsiiskonen commented 9 years ago

Hmm... My screen resolution is 1920 x 1080 so there should be space for toc at right side of the screen too. At least there is guite much empty space when document is rendered.

mojavelinux commented 9 years ago

Aha! I bet you the problem is this line:

showToc: true,

That is very likely setting the toc attribute in the API and overriding what you have in the document. This is likely an issue in the Grunt plugin...Yep.

https://github.com/asciidoctor/asciidoctor-grunt-plugin/blob/master/tasks/asciidoctor.js#L49

That's a bug that needs to be reported against the Grunt plugin. It needs to allow the toc to be set to any of the allowed values. Currently, it is hard-coding the value preamble if showToc is set to true and disabling the toc if showToc is set to false.

Btw, here are the allowed values:

mojavelinux commented 9 years ago

In summary, the value in your document is ignored.

tsiiskonen commented 9 years ago

Ok, thanks. I reported this issue against the Grunt plugin. Hopefully it can be fixed.

mojavelinux commented 9 years ago

:+1: You can count on it :)

anthonny commented 9 years ago

I think we can close this issue now :)

mojavelinux commented 9 years ago

Not just yet. We still need to support embedding the default stylesheet. Unless you'd like to open another issue with a more specific title.

There are two tasks:

ggrossetie commented 9 years ago

@mojavelinux Do you know why toc=preamble@ is not working ? I though @ was suppose to set a default value unless the document defined another value ?

mojavelinux commented 9 years ago

If that were the case, then it would work. But if you look closely, the @ is not there.

https://github.com/asciidoctor/asciidoctor-grunt-plugin/blob/master/tasks/asciidoctor.js#L49

One fix would be to add @. However, technically toc is not a boolean option, so really it should be changed to be an enum.

ggrossetie commented 9 years ago

Sorry I meant, I tried to modify that line to add @ but that's not working. So you say this should work... hmm I will try to find out why.

mojavelinux commented 9 years ago

Ah, I see what you're saying. The answer is, it's tricky in this situation. The problem is, we are supporting a lot of different legacy combinations here and one of them is conflicting. In this case, it's the force unset of toc2!. That should really be removed. That was added when enabling the toc would cause the processor to crash. We are beyond that, so we can remove these hacks. Let's just start clean.

ggrossetie commented 9 years ago

There are two tasks:

  • Map IO.read to File.read (we can wait for this to happen in core or add a patch)
  • Figure out why the default stylesheet path can't be resolved when using Asciidoctor.js via npm.

Another task is to implement File.readable.

With Asciidoctor.js, DATA_PATH resolves to undefined, STYLESHEETS_DATA_PATH resolves to stylesheets and the default stylesheet name is asciidoctor-default.css. So we are trying to load the file stylesheets\asciidoctor-default.css (resolved to c:\stylesheets\asciidoctor-default.css on Windows) which doesn't exist.

But we can use stylesheet and stylesdir attributes to override the stylesheet directory and the stylesheet name. With the following values (and an implementation of File.readable) I can read and load the default stylesheet with asciidoctorjs:

'stylesheet=asciidoctor.css stylesdir=' + __dirname + '/../node_modules/asciidoctor.js/dist/css'
mojavelinux commented 9 years ago

I wonder if for now we should just disable the whole section of the code that deals with copying stylesheets when running in JavaScript. One quick way to disable it now is to simply unset the copycss attribute (which is enabled by default). Then, you can do whatever you need to do on the JavaScript side.

mojavelinux commented 9 years ago

...though I guess that's only relevant when the linkcss attribute is set. Though, I think if you unset the stylesheet attribute, then it doesn't do anything. See:

https://github.com/asciidoctor/asciidoctor/blob/master/lib/asciidoctor/converter/html5.rb#L54-L71

pavelhoral commented 9 years ago

Can I ask you guys for a summary on the status for this issue... I am kind of lost reading comments as the discussion got "hijacked" many times... maybe a new issue should be created dedicated to generating HTML with inlined CSS?

ggrossetie commented 9 years ago

Hello @pavelhoral

I am kind of lost reading comments as the discussion got "hijacked" many times...

I agree.

maybe a new issue should be created dedicated to generating HTML with inlined CSS?

Can you describe your issue and/or what you want to achieve ?

pavelhoral commented 9 years ago

Can you describe your issue and/or what you want to achieve ?

I want CSS to be inlined in the created HTML (which is the default behavior of AsciiDoctor). In other words generating HTML with just safe: 'unsafe' should inline the default CSS file in the result HTML. Right now it ends with:

/home/horal/foo/bar/node_modules/gulp-asciidoctor/node_modules/asciidoctor.js/dist/npm/asciidoctor-core.min.js:9
primary_stylesheet_data=(null==(d=a.Object._scope.IO)?a.cm("IO"):d).$read((nul
                                                                    ^
TypeError: undefined is not a function
    at g.j.$primary_stylesheet_data (/home/horal/Documents/repository/jcu/jcu-idm/node_modules/gulp-asciidoctor/node_modules/asciidoctor.js/dist/npm/asciidoctor-core.min.js:9:18735)
    at g.j.$embed_primary_stylesheet (/home/horal/Documents/repository/jcu/jcu-idm/node_modules/gulp-asciidoctor/node_modules/asciidoctor.js/dist/npm/asciidoctor-core.min.js:9:18927)
    at h.j.$document (/home/horal/Documents/repository/jcu/jcu-idm/node_modules/gulp-asciidoctor/node_modules/asciidoctor.js/dist/npm/asciidoctor-core.min.js:4:18691)
    at h.$opal.defn.TMP_1 (/home/horal/Documents/repository/jcu/jcu-idm/node_modules/gulp-asciidoctor/node_modules/asciidoctor.js/node_modules/opal-npm-wrapper/index.js:1793:21)
    at h.i.$convert (/home/horal/Documents/repository/jcu/jcu-idm/node_modules/gulp-asciidoctor/node_modules/asciidoctor.js/dist/npm/asciidoctor-core.min.js:4:7408)
    at i.o.$convert (/home/horal/Documents/repository/jcu/jcu-idm/node_modules/gulp-asciidoctor/node_modules/asciidoctor.js/dist/npm/asciidoctor-core.min.js:6:15859)
    at OpalModule.b._proto.$convert (/home/horal/Documents/repository/jcu/jcu-idm/node_modules/gulp-asciidoctor/node_modules/asciidoctor.js/dist/npm/asciidoctor-core.min.js:10:24778)
    at Transform._transform (/home/horal/Documents/repository/jcu/jcu-idm/node_modules/gulp-asciidoctor/index.js:47:30)
    at Transform._read (_stream_transform.js:179:10)
    at Transform._write (_stream_transform.js:167:12)
ggrossetie commented 9 years ago

@pavelhoral Thanks

I will try to address this issue in Asciidoctor.js. There's an open pull request on Asciidoctor core to resolve the first part of this issue "Map IO.read to File.read" https://github.com/asciidoctor/asciidoctor/pull/1354

Now that we now more about this issue maybe we could just rename the issue to something more meaningful ?

mojavelinux commented 8 years ago

Nice!

@Mogztter could you update the issue to set the resolved version? Probably set the label to bug too.