Skip to content

Conversation

ms-afk
Copy link

@ms-afk ms-afk commented Aug 28, 2025

Changes

Fixes and enhances #44:

  • fixed some of the options added in Add some common params for mmdc #44 and added some new ones
  • added the possibility to use relative paths (see README for details)
  • used lisp-case for the filter's options
  • fixed the tests that were broken after Add some common params for mmdc #44, and included the new options in tests
  • documented new behaviour in the README

Clarifications regarding #31

This may close #31, but given what @reagle asked for, I think there needs to be some clarification.

First I'm not aware that Mermaid will read a mermaid-config.json automatically, so to change Mermaid's configuration for the entirety pandoc execution, you should create a mermaid configuration file and add it to the options of this filter.

For example, if we name the file mermaid.json and execute pandoc in the same directory where this file resides, this would be the metadata block needed:

---
diagram:
  engine:
    mermaid:
      config-file: mermaid.json
---

If you need to change Mermaid's configuration for a single diagram, then you will need to use Mermaid's frontmatter configuration, which is independent from this filter and built into Mermaid.

Note that the first configuration has to be written in json, whilst the second one is yaml.

@tarleb
Copy link
Member

tarleb commented Sep 2, 2025

Thank you! I didn't find the time yet to review this, but it's on my list!

Copy link
Member

@tarleb tarleb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, thanks a lot!

I've added a few inline comments, but nothing major. Just let me know if you don't have time to fix it yourself, then I'll merge as-is and do the changes later.

local outfile = tmpdir .. '/diagram.' .. file_extension
--- Configure options for mmdc based on engine options
local args = {'--pdfFit', '--input', infile, '--output', outfile}
setmetatable(args, {__index = {insert = table.insert}})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using pandoc.List {...} in the line above would make this line unnecessary.

local args = {'--pdfFit', '--input', infile, '--output', outfile}
setmetatable(args, {__index = {insert = table.insert}})
if self.opt then
if self.opt['theme'] then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The repeated if statements seems a bit clunky to me. How about defining a table that maps option keys to command line arguments, and then loop over that table in a for loop? Something along the lines of

local options = { theme = '--theme', ...}
for opt, cliopt in pairs(options) do
  ...
end

)
return read_file(outfile), mime_type
end)
local infile = tmpdir .. '/diagram.mmd'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use pandoc.path.join here to ensure that we are using the correct directory separator. It probably won't matter, but it's good style.

@ms-afk
Copy link
Author

ms-afk commented Sep 7, 2025

I'll try to work on it this week

@ms-afk
Copy link
Author

ms-afk commented Sep 18, 2025

Hello, sorry for the wait, I've applied the changed as requested.

Now the only issue is the GitHub workflow failing.
This seems to be happening because the new metadata block in test-mermaid.yaml sets a different Puppeteer configuration file than the one set in the workflow file and the new configuration doesn't contain the options necessary to make Puppeteer work in the GitHub container.

A possible solution would be to set the tests/puppetteerConfig.json content to the one required by the container, like this:

diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
index ac493a7..f3f8eb5 100644
--- a/.github/workflows/ci.yaml
+++ b/.github/workflows/ci.yaml
@@ -81,7 +81,6 @@ jobs:
       image: pandoc/core:${{ matrix.pandoc }}

     env:
-      MERMAID_BIN: /usr/local/bin/mmdc-test
       PUPPETEER_EXECUTABLE_PATH: /usr/bin/chromium-browser

     steps:
@@ -93,10 +92,6 @@ jobs:
         run: |
           apk update && apk add chromium chromium-chromedriver make npm
           npm install -g @mermaid-js/mermaid-cli
-          printf '{"args":["--no-sandbox","--disable-setuid-sandbox", "--disable-gpu"]}' > \
-              /etc/puppeteer-conf.json
-          printf '#!/bin/sh\nmmdc -p /etc/puppeteer-conf.json $@' > $MERMAID_BIN
-          chmod +x $MERMAID_BIN

       - name: Test
         run: 'make test-mermaid'
diff --git a/test/puppeteerConfig.json b/test/puppeteerConfig.json
index 0967ef4..5d14311 100644
--- a/test/puppeteerConfig.json
+++ b/test/puppeteerConfig.json
@@ -1 +1 @@
-{}
+{"args":["--no-sandbox","--disable-setuid-sandbox", "--disable-gpu"]}

This would also make some code unnecessary in the workflow file, that's why I've removed it in the code block above.

The only problem with this is that this configuration, which disables the sandbox security feature of chrome, would also be applied to tests executed in developers' computers, but I don't think this is a problem considering we control the code that the browser executes.

Let me know if you want me to commit this.

@tarleb
Copy link
Member

tarleb commented Sep 22, 2025

Thank you! I think my preferred method would be to remove test/puppeteerConfig.json from the repo. We can then add another CI test that writes the config to that file instead of /etc/puppeteer-conf.json. This way we don't interfere with local setups, but still make sure that everything works ok.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

.mermaid-config.json not being used
3 participants