asciidoctor / asciidoctor.js

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

Unexpected behavior of include directive in 2.2.7 when parsing malformed title #1736

Open gladykov opened 2 weeks ago

gladykov commented 2 weeks ago

I used this file https://gist.github.com/mojavelinux/4402636#file-test-asciidoc-txt as source of test of ascii doc converter. Per comment there, it has wrong title, but it never bothered us. But seems since 2.2.7 it causes unexpected behavior (present in 3.0.4)


import asciidoctor from '@asciidoctor/core'

const Asciidoctor = asciidoctor()

const badInput = `Asciidoctor Demo
================

include::markdown_2_in.md[]`

// Use this variable in order to genearte good output 
const goodInput = `include::markdown_2_in.md[]`

const expectedOutput = `<div class="paragraph">
<p><a href="markdown_2_in.md" class="bare include">markdown_2_in.md</a></p>
</div>`

const actualOutput = Asciidoctor.convert(badInput)

console.log("'" + expectedOutput + "'")
console.log("'" + actualOutput + "'")

console.assert(actualOutput === expectedOutput)

// Actual wrong output:
// <div class="paragraph">
// <p><a href="markdown_2_in.md">role=include</a></p>
// </div>

Removing either title Asciidoctor Demo or ================ from badInput will cause output line <p><a href="markdown_2_in.md">role=include</a></p> to render properly into <p><a href="markdown_2_in.md" class="bare include">markdown_2_in.md</a></p> Meaning you need both of them to trigger unexpected behavior.

gladykov commented 2 weeks ago

PS. At this point I'm not sure what is the correct title syntax. This may as well classify as other bug, or me not understanding standard enough:

Such title: = Asciidoctor Demo will cause input 'emphasis' to be rendered as 'emphasis' - unexpected :yellow_circle:

Such title:

Asciidoctor Demo
================

will cause input 'emphasis' to be rendered as <em>emphasis</em> - expected :green_circle:


This does not affect *bold* input, which will always render as <strong>bold</strong>

Let me know if you will need more examples / testing on my side.

ggrossetie commented 2 weeks ago

If you use setext-style document title it will implicitly enable compatibility mode, see: https://docs.asciidoctor.org/asciidoctor/latest/migrate/asciidoc-py/#compatibility-mode

If you are writing modern AsciiDoc, you should use = Asciidoctor Demo and _emphasis_.

I can reproduce this first issue though: https://runkit.com/mogztter/66706c610b72fa00085f9fea

@mojavelinux any idea? it seems that something has changed since I cannot reproduce on an older version of Asciidoctor.js (for instance 2.2.6):

var core = require("@asciidoctor/core@2.2.6")

var Asciidoctor = core()

console.log(Asciidoctor.convert(`Asciidoctor Demo
================

include::markdown_2_in.md[]`))

// "<div class=\"paragraph\">\n<p><a href=\"markdown_2_in.md\" class=\"bare\">markdown_2_in.md</a></p>\n</div>"
mojavelinux commented 2 weeks ago

As far as I can tell, this works as expected with Asciidoctor (Ruby). The file is included.

I think the API example you provided is not working because you didn't enable safe mode. That's why the include ends up as a link.

Note that using setext headings in AsciiDoc is considered deprecated and thus we aren't addressing any issues related to them in core. They are also being removed from the language in the specification.

ggrossetie commented 2 weeks ago

As far as I can tell, this works as expected with Asciidoctor (Ruby). The file is included.

I think the issue is specifically when include is disabled.

I think the API example you provided is not working because you didn't enable safe mode. That's why the include ends up as a link

It should produce a link but the output is wrong:

<p><a href="markdown_2_in.md">role=include</a></p>

The expected output is:

<p><a href="markdown_2_in.md" class="bare">markdown_2_in.md</a></p>

Note that using setext headings in AsciiDoc is considered deprecated and thus we aren't addressing any issues related to them in core. They are also being removed from the language in the specification.

👍🏻

mojavelinux commented 2 weeks ago

Oh, I see what it is. The include processor now creates a link with a role when the include is disabled...and in compat mode that syntax is not parsed. I'm on the fence about whether to do anything about that. I suppose we could only add the role when the document is not in compat mode.

ggrossetie commented 1 week ago

I suppose we could only add the role when the document is not in compat mode

I think that would be nice especially since compat mode can be implicitly enabled. Should I transfer this issue in asciidoctor/asciidoctor?

mojavelinux commented 1 week ago

I'm open to addressing it in core, though I'd prefer a fresh issue rather than a transferred one.