Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Process graphviz diagrams. #24

Merged
merged 6 commits into from
Nov 21, 2024
Merged

Process graphviz diagrams. #24

merged 6 commits into from
Nov 21, 2024

Conversation

jonathanpallant
Copy link
Member

Replaces mermaid support (which doesn't work well with reveal.js and RevealMarkdown).

I prefer this over #23 and #22. But you have to re-write your diagrams as Graphviz dot syntax.

@jonathanpallant
Copy link
Member Author

I think we can publish this because anything other than dot process blocks are left exactly as before.

@jonathanpallant jonathanpallant requested review from tshepang and Hoverbear and removed request for tshepang November 20, 2024 10:36
Copy link
Member

@Hoverbear Hoverbear left a comment

Choose a reason for hiding this comment

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

Can't give a very comprehensive review here but everything appears as I'd expect and the checks pass.

@jonathanpallant
Copy link
Member Author

Actually, I should add a test containing a dot diagram.

Replaces mermaid support (which doesn't work well with reveal.js and RevealMarkdown).
Showing me the reference file isn't useful - I have that file. I need to see the generated file in data_out.
@tshepang
Copy link
Member

running cargo test fails locally for me...

test build_slides ... FAILED

@jonathanpallant
Copy link
Member Author

What does:

$ diff -r ./tests/reference_out ./tests/data_out

say? Do you have Graphviz installed?

@tshepang
Copy link
Member

tshepang commented Nov 21, 2024

❯ diff -r ./tests/reference_out ./tests/data_out
diff --color=auto -r ./tests/reference_out/chapter_2.html ./tests/data_out/chapter_2.html
13c13
< <!-- Generated by graphviz version 2.43.0 (0)
---
> <!-- Generated by graphviz version 12.2.0 (20241103.1931)
15c15
< <!-- Title: %3 Pages: 1 -->
---
> <!-- Pages: 1 -->
19,20c19
< <title>%3</title>
< <polygon fill="white" stroke="transparent" points="-4,4 -4,-41 58,-41 58,4 -4,4"/>
---
> <polygon fill="white" stroke="none" points="-4,4 -4,-41 58,-41 58,4 -4,4"/>
25c24
< <text text-anchor="middle" x="27" y="-14.8" font-family="Times,serif" font-size="14.00">X</text>
---
> <text text-anchor="middle" x="26.88" y="-13.45" font-family="Times,serif" font-size="14.00">X</text>

yeah, it's installed... version 12.2.0

@jonathanpallant
Copy link
Member Author

That diff seems fine. I see the same.

@jonathanpallant jonathanpallant merged commit dda0dd1 into main Nov 21, 2024
9 checks passed
@jonathanpallant jonathanpallant deleted the pre-render-graphviz branch November 21, 2024 12:57
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.

3 participants