Skip to content

Add slots to graph #1

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

Merged
merged 3 commits into from
Mar 6, 2021
Merged

Add slots to graph #1

merged 3 commits into from
Mar 6, 2021

Conversation

mtsr
Copy link
Contributor

@mtsr mtsr commented Mar 5, 2021

I've added slots to the graph. Your dot code was clean enough that I didn't want to add a dependency for that. But the itertools thing is really useful to have. Take a look and let me know what you think.

@mtsr
Copy link
Contributor Author

mtsr commented Mar 6, 2021

This is what it looks like now (note that this is not the base rendergraph, but from my project):

render_graph dot 3

Copy link
Owner

@jakobhellermann jakobhellermann left a comment

Choose a reason for hiding this comment

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

Thank you very much. I've suggested a few minor tweaks, but otherwise this is good to merge.

@jakobhellermann jakobhellermann merged commit 3669dfd into jakobhellermann:main Mar 6, 2021
@mtsr
Copy link
Contributor Author

mtsr commented Mar 6, 2021

Made your proposed changes. This is how it looks now:

render_graph dot 3

@mtsr
Copy link
Contributor Author

mtsr commented Mar 6, 2021

Note the minor layout issues with the text flowing outside the nodes. I found this issue rust-lang/rust#76500 . If I understand correctly the graphviz engine doesn't have Roboto so is failing to set the width correctly in the SVG, which is then rendered like this (by GIMP in my case).

The proposed solution is listing multiple fonts.

@jakobhellermann
Copy link
Owner

Thanks! Can you try if fontname="Roboto, Serif" or something like that works?

@mtsr
Copy link
Contributor Author

mtsr commented Mar 6, 2021

Doesn't seem to work. Not sure, but can't find good documentation about it, either.

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.

2 participants