search

mfontanini / presenterm

A markdown terminal slideshow tool
https://mfontanini.github.io/presenterm/
BSD 2-Clause "Simplified" License
1.19k stars 29 forks source link

feat: add support for rendering mermaid diagrams #268

Closed mfontanini closed 3 months ago

mfontanini commented 3 months ago

This adds support for rendering mermaidjs diagrams by using the +render modifier. This requires the mermaid cli and accepts a few parameters in the theme and config file.

In the theme:

In the config file:

The following presentation:

# Stats we all care about

```mermaid +render
pie title Pets adopted by volunteers
    "Dogs" : 386
    "Cats" : 85
    "Rats" : 15
```

Gets rendered like this:

image

Fixes #266

PadenZach commented 3 months ago

Wow that was fast! Looks very promising, however, I wasn't able to get this one working. I get a slide with the following info:

                                                     Error loading presentation: 

                         parse error at 18:1: invalid code block: language Unknown does not support rendering

Additionally, this may break mmdc (not that bad maybe? It seems to work on github and vscode).

To try this out this is what I ran:

Presenterm mermaid tests

To test ran:

cargo install --git https://github.com/mfontanini/presenterm/ --branch feat/mermaid-graphs

yielding:

    Updating git repository `https://github.com/mfontanini/presenterm/`
  Installing presenterm v0.7.0 (https://github.com/mfontanini/presenterm/?branch=feat/mermaid-graphs#5dc93f28)
    Updating crates.io index
     Locking 185 packages to latest compatible versions

PR Example

Stats we care about

pie title Pets adopted by volunteers
    "Dogs" : 386
    "Cats" : 85
    "Rats" : 15

User Journey

journey
    title My working day
    section Go to work
      Make tea: 5: Me
      Go upstairs: 3: Me
      Do work: 1: Me, Cat
    section Go home
      Go downstairs: 5: Me
      Sit down: 5: Me

Simple Flow chart

flowchart TB
    A --> C
    A --> D
    B --> C
    B --> D

flow chart with style

%%{init: {"flowchart": {"htmlLabels": false}} }%%
flowchart LR
    markdown["`This **is** _Markdown_`"]
    newLines["`Line1
    Line 2
    Line 3`"]
    markdown --> newLines

Mind map!

mindmap
  root((mindmap))
    Origins
      Long history
      ::icon(fa fa-book)
      Popularisation
        British popular psychology author Tony Buzan
    Research
      On effectiveness<br/>and features
      On Automatic creation
        Uses
            Creative techniques
            Strategic planning
            Argument mapping
    Tools
      Pen and paper
      Mermaid
mfontanini commented 3 months ago

Are you sure you are running the right presenterm version? I know you're showing the cargo install command but the error you're showing is exactly what you'd get if you didn't install it.

this may break mmdc

Not sure I understand. What do you mean by this?

mfontanini commented 3 months ago

I just pushed a commit. Now instead of "language Unknown does not support rendering" you should see "language Unknown(\"mermaid\") does not support rendering". Can you re-run?

PadenZach commented 3 months ago

Oops, Figured out I had a homebrew version of presenterm conflicting the cargo installed one. I was able to test successfully, but it doesnt always seem consistent:

                          Error loading presentation:

        typst render failed: io: No such file or directory (os error 2)

And

  Error loading presentation:

                 typst render failed: 'mmdc' execution failed:

       Error: Protocol error: Connection closed. Most likely the page has
       been closed.
           at assert
       (file:///opt/homebrew/lib/node_modules/@mermaid-js/mermaid-cli/nod
       e_modules/puppeteer-core/lib/esm/puppeteer/util/assert.js:25:15)
           at CDPPage.close
       (file:///opt/homebrew/lib/node_modules/@mermaid-js/mermaid-cli/nod
       e_modules/puppeteer-core/lib/esm/puppeteer/common/Page.js:756:9)
       /index.js:192:3)ebrew/lib/node_modules/@mermaid-js/mermaid-cli/src

These seem to be related to performance issues

this may break mmdc mmdc is the mermaid diagram cli. There's an option to convert a markdown of mermaid charts to one with image links. The +render flag in the mermaid blocks seems to break this integration.

EG:

➜  ~ mmdc -i presenterm_mermaid_test.md
No mermaid charts found in Markdown input

Performance

From poking around myself, seems like this is largely a mermaid side issue. Seems like their next release should improve performance some; however, looking at the code here I'm wondering if we're hitting a really bad mmdc bottleneck. It looks like the mmdc -i ... -o ... command for each mermaid chart. I think this makes it hit the worst part of mmdc (starting up a headless chromium instance) for each chart.

mfontanini commented 3 months ago

There's an option to convert a markdown of mermaid charts to one with image links. The +render flag in the mermaid blocks seems to break this integration.

This is not using a markdown file, it's writing each mermaid diagram snippet into a separate .mmd file so the +render flag being non-standard is not an issue.

Seems like their next release should improve performance some

This seems related to when you're using markdown, which is not the case here so it won't improve anything here.

starting up a headless chromium instance

I hadn't seen how this worked... So in order to convert text into an image from the command line this spins up an entire browser... Software engineering has gone too far.

Having said that, I can't reproduce the issue you have. I'm using the example presentation you pasted above and it works every time. You're saying sometimes it works and sometimes it doesn't? If you run mmdc directly on a .mmd file do you see this issue? Maybe run it multiple times? I don't see how presenterm could cause this, this is just spinning a separate process.

mikavilpas commented 3 months ago

I tried this out, and was able to render the "Stats we all care about" presentation with the dogs,cats,rats piechart!

I hadn't seen how this worked... So in order to convert text into an image from the command line this spins up an entire browser... Software engineering has gone too far.

😄 I agree. This approach is quite cursed - but I think it can be improved. Here are some suggestions:

  1. Caching could be used, for example based on a hash of the mermaid source itself.
    • Already rendered images could be saved in a directory.
    • This would make editing the other parts of the presentation much faster. Right now mermaid seems to bog down presenterm quite a lot
  2. Asynchronous rendering could be used for mermaid diagrams.
    • Maybe could give some kind of simple indication that the diagram is being rendered, and then replace it with the rendered image when it's done.
mikavilpas commented 3 months ago

I also experimented a little with having some sort of background process that could help avoid starting and closing the render context (the browser). I tried out https://github.com/TomWright/mermaid-server but it doesn't seem to have incredible performance.

Here's a demo of me calling it via a curl script and rendering directly to wezterm:

https://github.com/mfontanini/presenterm/assets/300791/11870a06-f588-44ae-bc19-bb6ff4d03f32

As you can see, the performance isn't incredible.

mfontanini commented 3 months ago

Caching could be used, for example based on a hash of the mermaid source itself.

Yeah, images are already being cached in memory, it's just that the mermaid rendering isn't using it. I'll make a note to have them use it, this will improve the experience when developing a presentation.

Asynchronous rendering could be used for mermaid diagrams

And yes, this is something I considered when I created this PR. Looks like it will be a requirement here since this is incredibly slow.

I'd suggest people go complain on this issue https://github.com/mermaid-js/mermaid/issues/3650 as we're ultimately bound by their implementation. We can parallelize as much as we can but we'll never render charts faster than it takes to render a single one.

I'm going to merge this as-is as the functionality is there. The 2 points above are QOL improvements but they're not blockers for having this in. I'll see if I can make up some time and work on them this week.