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

Suggest `next` when trying to break from captured block #7406

Merged
merged 5 commits into from Mar 12, 2019

Conversation

Projects
None yet
6 participants
@r00ster91
Copy link
Contributor

commented Feb 10, 2019

When doing this:

def method(&block)
  block.call
end

method do
  break
end

then you get Error in line 6: can't break from captured block.
Now this makes the error message suggest next because when using break in a captured block you most likely tried to escape from the captured block and next makes that possible.
Also when you instead try to return from a captured block, it gives you a helpful error message: Error in line 6: can't return from captured block, use next so why not suggest next in case of break too?

This also downcases next's and break's error message just like all the other error messages.

@straight-shoota

This comment has been minimized.

Copy link
Member

commented Feb 10, 2019

In general, this is probably a good feature. But I'm not sure if "use next" is the best advice. It's not always as simple as that, because next and break/return have different semantics. If the block is called multiple times, next might not give you what you want. So I think it should be a more relaxed suggestion.

@asterite

This comment has been minimized.

Copy link
Member

commented Feb 10, 2019

@straight-shoota Right on! I was thinking about did you mean 'next'? But that doesn't give much info either.

In general, I think we should improve all of our error messages to look a bit more like Elm, where they take a lot of time to explain what went wrong and suggestions with their explanations.

@r00ster91

This comment has been minimized.

Copy link
Contributor Author

commented Feb 10, 2019

What about sometimes linking to the docs?

can't break from captured block.
If you want to exit the captured block, use `next`, however `next` will have behave differently when used in a block or a loop inside of the captured block.
For more information see https://crystal-lang.org/reference/syntax_and_semantics/next.html.
@Sija

This comment has been minimized.

Copy link
Contributor

commented Feb 10, 2019

Actually, why it's not possible to use break in captured blocks?

@asterite

This comment has been minimized.

Copy link
Member

commented Feb 11, 2019

@Sija break in non-captured blocks means "exit the enclosing method". For a capture block it can't work.

def capture(&block)
  block
end

def foo
  # This break looks like it will exit foo, but that can't happen
  block = capture { break }
  puts 1
  block
end

block = foo
block.call
r00ster91
@r00ster91

This comment has been minimized.

Copy link
Contributor Author

commented Feb 11, 2019

I hope this is clear enough now. I don't think it needs to be as long as I proposed. In the end it might rather confuse the user.

@straight-shoota

This comment has been minimized.

Copy link
Member

commented Feb 11, 2019

Indeed, that is way to long and too much detail. A simple "Maybe you can use next instead?`" would suffice.

r00ster91
r00ster91
@Sija

This comment has been minimized.

Copy link
Contributor

commented Feb 11, 2019

@asterite Now it makes sense, thanks for explanation!

@sdogruyol
Copy link
Member

left a comment

Thank you @r00ster91 👍

@straight-shoota straight-shoota merged commit 89ce57a into crystal-lang:master Mar 12, 2019

5 checks passed

ci/circleci: check_format Your tests passed on CircleCI!
Details
ci/circleci: test_darwin Your tests passed on CircleCI!
Details
ci/circleci: test_linux Your tests passed on CircleCI!
Details
ci/circleci: test_linux32 Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@straight-shoota straight-shoota added this to the 0.28.0 milestone Mar 12, 2019

urde-graven pushed a commit to urde-graven/crystal that referenced this pull request Mar 20, 2019

Urde Graven
Merge branch 'master' of github.com:crystal-lang/crystal
* 'master' of github.com:crystal-lang/crystal:
  Change the font-weight used in Playground (crystal-lang#7552)
  Fix formatting absolute paths (crystal-lang#7560)
  Refactor IO::Syscall as IO::Evented (crystal-lang#7505)
  YAML: fix test checking serialization of slices for libyaml 0.2.2 (crystal-lang#7555)
  Let Array#sort only use `<=>`, and let `<=>` return `nil` for partial comparability (crystal-lang#6611)
  Update `to_s` and `inspect` to have similar signatures accross the stdlib (crystal-lang#7528)
  Fix restriction of valid type vs free vars (crystal-lang#7536)
  Rewrite macro spec without executing a shell command (crystal-lang#6962)
  Suggest `next` when trying to break from captured block  (crystal-lang#7406)
  Fix GenericClassType vs GenericClassInstanceType restrictions (crystal-lang#7537)
  Add human readable formatting for numbers (crystal-lang#6314)
  Add command and args to execvp error message (crystal-lang#7511)
  Implement Set#add? method (crystal-lang#7495)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.