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

run actions #49

Merged
merged 4 commits into from
Apr 29, 2024
Merged

run actions #49

merged 4 commits into from
Apr 29, 2024

Conversation

Vulpesx
Copy link
Contributor

@Vulpesx Vulpesx commented Apr 26, 2024

Spinner can do tricks now.

You can change the theme, style and name through a mpsc channel.

Had to make spinner own the Style and Theme to make this work, granted i didn't put any effort into this

The question at hand is what to do about themes, currently I just put it in a box to get this working, but that's not a good solution.
We could:

  • not allow changing themes,
  • require a static reference,
  • do some unsafe magic, should be safe as long as spinner doesn't access it after the other thread ends.
  • Rc or Arc, but that still has the same problem as box

I don't think owning the Style is nearly as much of a problem but should prob do the same with that as with Theme

chore: update example
@Vulpesx
Copy link
Contributor Author

Vulpesx commented Apr 26, 2024

ill focus more on #48 so this one might take a while

@Vulpesx
Copy link
Contributor Author

Vulpesx commented Apr 29, 2024

This version uses references and unsafe magic
I forced the references to live longer than the scope, which means it has to reference data outside the closure you pass to run.
If you make that data mutable u can change the theme and style without sending which kinda violates the borrow checker

In this particular case it isn't unsafe since it's mutating in place and the spinner doesn't hold on to a reference to any data that could be on the heap that I'm aware of.

However, if that's uncomfortable which it probably should be, it's really easy to disallow that

@jdx jdx merged commit ed9fcf9 into jdx:main Apr 29, 2024
1 check passed
@Vulpesx Vulpesx mentioned this pull request Apr 30, 2024
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