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

Add name to statemachine and make dot output stable and unique #62

Merged
merged 3 commits into from
Aug 23, 2023

Conversation

asmfreak
Copy link
Contributor

@asmfreak asmfreak commented Apr 11, 2023

This adds ability to name statemachines. I have a personal pet project that uses smlang.
It uses several ones of them. So I wanted to see them all as pictures via graphviz.
My problem with that was: smlang-rs generates only one picture (statemachine.svg).
Also I wanted to give each statemachine a unique name.
This patch addresses both issues.
I've also bumped the version as this change is a bit significant in my opinion. Please advise, if it should be revised.

Copy link
Collaborator

@ryan-summers ryan-summers left a comment

Choose a reason for hiding this comment

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

Sorry for the late review - was on vacation the last two weeks :)

Please also:

  • Note this as a change in the CHANGELOG.md

Cargo.toml Outdated Show resolved Hide resolved
examples/named_dominos.rs Show resolved Hide resolved
macros/Cargo.toml Outdated Show resolved Hide resolved
macros/src/codegen.rs Outdated Show resolved Hide resolved
macros/src/diagramgen.rs Outdated Show resolved Hide resolved
macros/src/lib.rs Show resolved Hide resolved
@asmfreak
Copy link
Contributor Author

I've implemented proposed changes.
I've also added derive functionality to states and events. Since it was conflicting with other feature (impl_display), I completely removed code for it, since the derive version has a broader scope.

ryan-summers
ryan-summers previously approved these changes Aug 23, 2023
Copy link
Collaborator

@ryan-summers ryan-summers left a comment

Choose a reason for hiding this comment

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

LGTM, just needs to have a run of cargo fmt to fix up style. One optional change requested below, but I won't block merging off that.

Thanks for taking the time on this!

macros/src/parser/mod.rs Outdated Show resolved Hide resolved
@ryan-summers
Copy link
Collaborator

ryan-summers commented Aug 23, 2023

Looks like some tests are also failing

Copy link
Collaborator

@ryan-summers ryan-summers left a comment

Choose a reason for hiding this comment

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

Thanks for pushing through on this :)

@ryan-summers ryan-summers merged commit 1bb0bfd into korken89:master Aug 23, 2023
7 checks passed
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