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

Statedefs #145

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Statedefs #145

wants to merge 12 commits into from

Conversation

rickbsgu
Copy link

@rickbsgu rickbsgu commented Nov 11, 2017

Changes add the option to define states prior to transitions.

Allows:

  • Dot attributes to be defined for states as well as transitions
  • Transition definitions to check if state is defined. Throws an error if not.

Also adds a dot prefix definition to set attributes for the entire graph (the default is pretty ugly.) A default set is specified, but can be overridden.

Statedefs are implemented in parallel with the current state mechanism to insure that the package still works as before. The statedefs mechanism only kicks in if statedefs are defined in the fsm definition.

All of the regression tests pass (had to modify the plugin/visualize test somewhat extensively to accommodate the dot prefix mechanism.) I also have added a test file with some tests (statedefs.js). Thinking of more.

Finally, some small changes to the 'examples' executable to allow a single example to be run. Added an example 'traffic_light_statedefs' that demonstrates the dot attributes for states.

I haven't touched the documentation - want to field thoughts from you on where would be the appropriate place to document this.

Let me know what your thoughts are.

Thanks,
rickb

@rickbsgu
Copy link
Author

rickbsgu commented Nov 12, 2017

Hmm - trying this in a real world app and seeing a bug in the visualization generation. Hold off until I figure it out.

-- Ah - never mind. I had added states with the same name as the transition name. I suppose in principle, that's ok. I wonder if it would be worth enforcing not allowing that? It's something I would like to detect in my own usage...

At any rate, I caught it in the visualization (another reason I think the visualization capability is a killer feature.)

Followup:
Added the check for visualize name contained in states. Only enabled if statedefs are present.

@taoqf
Copy link

taoqf commented Feb 23, 2018

@rickbsgu contribution welcomed. https://github.com/taoqf/javascript-state-machine

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