What do you think about adding a couple of snapshot tests too?

What you have so far looks good to me.

What do you think about adding a couple of snapshot tests too?

Great point, working on it rn.

@lukewilliamboswell added snapshot testing by adding analogous examples in files that previously contained only the task bang suffix, lmk if you"d rather I split out into different files

@sekerez I"m not Luke Boswell, but I"d vote for splitting out into different files. It makes the testing more single responsibility, and we may have the ? and ! behave differently in the future, though I don"t find it likely.

However, I"d understand wanting to keep them in the same file, in that we want any tests updated for ! to also get updated for ?, as their behavior is currently expected to be basically identical.

Just a heads-up: I"m changing the parsing logic a little bit here: #6851
In particular, EmptyDefsFinal will be dropped

their behavior is currently expected to be basically identical

What about ResultTryQuestion as a statement? Should it be forbidden or implement early return?

I mean:

and = \resultA, resultB -> 
    resultA? # return resultA if it"s Result.err
    resultB? # otherwise return resultB

What about ResultTryQuestion as a statement? Should it be forbidden or implement early return?

@kdziamura great point - I feel like it should be forbidden, as if resultB is of type Result T, then returning resultB? would return Err T if it"s an error, and T otherwise... whereas it should instead return Ok T in that case, hence the ? should be omitted from the return.

Just to check my understanding, returning a value with ? should result in a type error, right? And that"s downstream from AST generation? Hence the AST should still generate, right?

edit: I"m a little surprised that code like this compiles:

main =
    Stdout.line! "Hello, World!" # returns {} | Task.Err [...] 

and behaves identically to

main =
    Stdout.line "Hello, World!" # returns Task.ok | Task.Err [...] 

given that in my mind the return type differs, based on the same reasoning for ?.

Is my understanding incorrect? Should we as a matter of convention allow returning resultB?, and wrap it in an Ok if it"s not Err?

@smores56 that makes sense - I"ll split out into separate files, and changes the original files" name to taskbang from suffixed.

@kdziamura also rebasing on your changes.

and = \resultA, resultB ->
    resultA? # return resultA if it"s Result.err
    resultB? # otherwise return resultB

It will be desugared into

and = \resultA, resultB ->
    Result.try resultA \{} ->
        resultB

Which makes sense for consistency (! and ? are interchangeable, it’s only one for Tasks and the other for Results).

So, we can indeed fully piggyback the TaskAwaitBang implementation

Thank you for your contribution! Sometimes PRs end up staying open for a long time without activity, which can make the list of open PRs get long and time-consuming to review. To keep things manageable for reviewers, this bot automatically closes PRs that haven’t had activity in 60 days. This PR hasn’t had activity in 30 days, so it will be automatically closed if there is no more activity in the next 30 days. Keep in mind that PRs marked Closed are not deleted, so no matter what, the PR will still be right here in the repo. You can always access it and reopen it anytime you like!

@sekerez I realize that I have some spare development capacity, and I see that this PR hasn"t had progress in about a month and a half. Would you mind if I chipped in on getting this out the door?

I was able to implement this by hijacking the ! -> Task.await code: #7000