mermaidjs / mermaid.cli

Development has been moved to https://github.com/mermaid-js/mermaid-cli
MIT License
1.09k stars 117 forks source link

Config file no longer seems to have any effect #29

Closed pepijnve closed 6 years ago

pepijnve commented 6 years ago

After upgrading mermaid.cli from 0.3.6 to 0.4.2 the --configFile option no longer seems to have any effect. I noticed this when the asciidoctor-diagram CI build started failing. Build #597 with 0.3.6 worked, build #598 with 0.4.2 fails.

I can reproduce this locally as well. Using the input file

sequenceDiagram
    Alice->>Bob: Hello Bob, how are you?
    Bob->>Claire: Hello Claire, how are you?
    Claire->>Doug: Hello Doug, how are you?

and the config file

{
  "sequenceDiagram": {
    "diagramMarginX": 500,
    "diagramMarginY": 500,
    "actorMargin": 500,
    "boxMargin": 500,
    "boxTextMargin": 500,
    "noteMargin": 500,
    "messageMargin": 500
  }
}

running mmdc with and without the --configFile produces identical output.

tylerlong commented 6 years ago

I am sorry but I introduced a breaking change: https://mermaidjs.github.io/breakingChanges.html

The tool has no problem but you need to update the config file.

pepijnve commented 6 years ago

You're really making me bend over backwards to maintain compatibility eh 😆

In asciidoctor-diagram I try to avoid breaking peoples existing workflows to the extent possible. A more user friendly way to do changes like this, for instance, is to still support the old key, upgrade it to the new one and log a deprecation warning. I realise this takes a little bit more work from a developer perspective, but it's much nicer to your users.

Anyway I'll add a version check in diagram and generate the correct key depending on the version.

pepijnve commented 6 years ago

FYI, https://mermaidjs.github.io/mermaidAPI.html is out of date it seems. Still says it should be sequenceDiagram.

tylerlong commented 6 years ago

What you said makes sense. I didn't think too much when I changed it. I still have the freedom to change the behavior because mermaid 8.0 is still in beta. Let me think.

tylerlong commented 6 years ago

Backwards compatibility added: https://github.com/knsv/mermaid/commit/a949c2aaec9ba8f9a7c8b83deb9b618750a95650

pepijnve commented 6 years ago

Thanks for fixing this on your end as well. I've added a version check in my code too to make sure the new key is used with mmdc >= 0.4.1.