-
Notifications
You must be signed in to change notification settings - Fork 31
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
Support for make, go, defer, and channels #178
Merged
Merged
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
6bd19b3
spawn, make, go, defer, channels
myzie b98f020
gofumpt
myzie 6672771
Test cleanup; add test for deferred file close
myzie 7fedb03
Remove unused var
myzie 009824c
Cleanup and more tests
myzie dd38a66
Misc fixes
myzie ebae9e3
Fix bug with free variables
myzie d9a0e1d
Examples
myzie 76da143
vscode extension update
myzie 7f0df6f
Add .spawn method to builtins and functions
myzie 0dc8ad6
Remove junk files
myzie File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it'd be very useful if the
go
keyword was an expression instead of a statement.And have it return the Risor
thread
object.Because then you could have:
In other words, it would be semantically identical to:
but with nicer syntax. Because then using Risor to do some quick parallel things becomes super easy. Such as:
And at that point, you don't really need to expose the
spawn
builtin function, but instead only rely on thego
keyword.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I definitely considered this. I do think there's something to be said for matching Go's behavior for the
go
keyword exactly though. It seems like a plus if most or allgo
statements in Risor are also valid in Go. There's an element of making it the least surprising to a Go programmer. This is why I chose to keep it as a statement for now.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd argue the other way around. That Go syntax and semantics could be expected to work in Risor, but then Risor could add stuff on top to make scripting easier.
Changing from a statement to an expression only expands its usage. So Go semantics are still valid.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
risor
is not go and if it provides some functionality that is similar but not exactly the same it only makes things more confusing, I'd even argue it would be better to stick tospawn
andthread
and not use thego
keyword at all.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just find the
spawn
syntax so inconvenient. Similar to Risor'stry
. Switching between function calling syntax is confusing and I commonly trip on it while writing Risor code.You're fully right @luisdavim that reusing the
go
keyword does bring over a lot of assumptions from the programmer.Maybe a different keyword could be used, like just using the word
spawn
Or what if
spawn
was a builtin property on all functions?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added support for this:
I did keep the
go
keyword as a statement, so it doesn't return the thread handle. If you want the thread handle you should usespawn
.This decision keeps the
go
behavior very similar to Go's for now, and doesn't preclude us from switching it to an expression down the road.