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 support for Fiber.try(_) #835

Merged
merged 3 commits into from Dec 3, 2020
Merged

Add support for Fiber.try(_) #835

merged 3 commits into from Dec 3, 2020

Conversation

mode777
Copy link
Contributor

@mode777 mode777 commented Nov 4, 2020

I noticed that wren has Fiber.call(_) but not Fiber.try(_). I needed this because I want to pass cancellation tokens to Fibers. Implementation is straightforward. Honestly I think it just was forgotten. I added an additional test try_value.wren

@avivbeeri
Copy link
Contributor

I think this PR implementation looks good. Don't forget to add documentation for the new version of Fiber.try(_).

@mode777
Copy link
Contributor Author

mode777 commented Nov 4, 2020

I think this PR implementation looks good. Don't forget to add documentation for the new version of Fiber.try(_).

Done.

@avivbeeri
Copy link
Contributor

I assume it's fine, but is the argument passed to try(_) available if the fiber yields/suspends, and is then resumed?

@ChayimFriedman2
Copy link
Contributor

Yes, just like call(_).

@mode777
Copy link
Contributor Author

mode777 commented Nov 4, 2020

I assume it's fine, but is the argument passed to try(_) available if the fiber yields/suspends, and is then resumed?

Yes, I added another test to show that. In the implementation call() call(_) and try() all just call runFiber so there should be no difference.

@ruby0x1 ruby0x1 merged commit f533999 into wren-lang:main Dec 3, 2020
@ruby0x1
Copy link
Member

ruby0x1 commented Dec 3, 2020

@mode777 don't forget you can add yourself to AUTHORS file in a separate PR.

Also note you generally want to PR from a branch (and not main branch), so that you can have more than one PR open and not be blocked from changing main. When you change the branch on your end, changes end up here in the PR which isn't what you want.

@mode777
Copy link
Contributor Author

mode777 commented Dec 3, 2020

@mode777 don't forget you can add yourself to AUTHORS file in a separate PR.

Also note you generally want to PR from a branch (and not main branch), so that you can have more than one PR open and not be blocked from changing main. When you change the branch on your end, changes end up here in the PR which isn't what you want.

yep, I noticed that too - I was actually waiting for this to get merged so I can have a look at some other things.

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.

None yet

4 participants