asciidoctor / asciidoctor-diagram

:left_right_arrow: Asciidoctor diagram extension, with support for AsciiToSVG, BlockDiag (BlockDiag, SeqDiag, ActDiag, NwDiag), Ditaa, Erd, GraphViz, Mermaid, Msc, PlantUML, Shaape, SvgBob, Syntrax, UMLet, Vega, Vega-Lite and WaveDrom.
http://asciidoctor.org
MIT License
442 stars 107 forks source link

Mermaid: document mmdc attribute #296

Closed ahus1 closed 1 year ago

ahus1 commented 4 years ago

Thanks for providing release 2.0.5 of this library!

I've tested the Mermaid functionality in release 2.0.5 and after some trial-and error I found out what to put into mmdc. Apparently I needed to put in the JavaScript file of the CLI in there, as it is passed to the node executable directly.

First I tried to pass in the CLI executable (the mmdc.cmd on Windows), but then Node complained that it couldn't parse a CMD file. After looking at the source code I figured out that passing on the JavaScript file mentioned in the mmdc.cmd would probably be the thing to do.

You find my setup in this issue's comment: https://github.com/asciidoctor/asciidoctor-intellij-plugin/issues/534#issuecomment-703832304

I wonder:

pepijnve commented 4 years ago

You're right, that isn't really obvious anymore. Combined with the error stating you should set the mermaid attribute the situation is definitely less than ideal.

To be honest I only really develop/test on macos, windows is a bit under tested. On UNIX-like systems the mmdc attribute can still point to the binary wrapper since that's just a JS file as well that starts with #!/usr/bin/env node. I had not considered the windows .cmd case. I'll see what I can do about this.

ahus1 commented 4 years ago

I am happy that there is the new release with the greatly improved mermaid support. Adding this small workaround on Windows is fine for me.

pepijnve commented 4 years ago

The tricky thing is that in order to make the node process terminate on error I have to inject the --unhandled-rejections=strict node parameter somehow (see #257 for the details). I could detect a .cmd file, try to parse/scrape it using a regex to figure out where the main script is located and then launch node. That would make things 'just work' but it feels a bit brittle since the slightest change in the batch scripts node generates might end up breaking the mermaid integration on Windows again.

mojavelinux commented 4 years ago

I'm pretty sure you can pass additional flags to the node invocation using the NODE_OPTIONS environment variable. See https://nodejs.org/api/cli.html#cli_node_options_options

pepijnve commented 4 years ago

I'm pretty sure you can pass additional flags to the node invocation using the NODE_OPTIONS environment variable.

That is a much, much better solution than what I came up with. Thanks Dan! I see a 2.0.6 release in our near future 😄

ahus1 commented 4 years ago

@pepijnve - I see that you opened issue https://github.com/mermaidjs/mermaid.cli/issues/77 with mermaid. As mermaid moved to a new GitHub repo:

pepijnve commented 4 years ago

Yep, the PR fixes the underlying root cause. That #77 issue is still relevant for the old version though, so it might be best to just leave the issue in place. I left a comment on issue #77 pointing to the PR, the maintainers can close the issue if they like. No idea why the mermaid developers made such a mess of their repos though; it's needlessly confusing for everyone.