ifErrorDo: should be onErrorDo: and deprecate with transform #6158
Conversation
1. Rename #ifErrorDo: into #onErrorDo:
2. use #onErrorDo: in deprecated methods like #ifError: and #ifErrorDo:
and move these to Deprecated90 package
3. have a transformation rule for the deprecations in #ifError: and #ifErrorDo:
4. Add a test #testOnErrorDo
Fix pharo-project#6157
|
@dionisiydk Can you please check again? Thanks in advance! |
| ifTrue: [ self deprecated: 'Use #onErrorDo: instead.' transformWith: '`@receiver ifError: `@arg' -> '`@receiver onErrorDo: `@arg' ] | ||
| ifFalse: [ self deprecated: 'Use #onErrorDo: instead.' ]. | ||
|
|
||
| ^ self onErrorDo: errorHandlerBlock |
There was a problem hiding this comment.
You need to wrap it like:
self onErrorDo: [:err | errorHandlerBlock cull: err receiver cull: err description]But I did not check in code what exact #cull:cull: it should be to be compatible.
There was a problem hiding this comment.
Why wrap? We just dispatch further on. Do you have a problematic case?
There was a problem hiding this comment.
That is original method:
ifError: errorHandlerBlock
^ self on: Error do: [:ex |
errorHandlerBlock cull: ex description cull: ex receiver]The issue is about eliminating this strange error unboxing so that the handler block will accept a normal error instance.
There was a problem hiding this comment.
what I am trying to say is that deprecated method should continue work with this strange two parameters
There was a problem hiding this comment.
Yes, the deprecated method should continue working as before :)
|
ping? |
guillep
added
Status: Need more work
…o 6157-ifErrorDo-should-be-onErrorDo-and-deprecate-with-transform
|
@guillep pong! |
Deprecated ifError: and ifErrorDo: should keep compatibility, otherwise they break the idea of a deprecation. They are called with a block expecting description and receiver as arguments => honor that. Removed the automatic rewriting as the rewriting rule for that may not be that obvious => let the user decide.
|
I've made the deprecated methods work as before to keep compatibility. Let's wait for the CI |
and move these to Deprecated90 package
Fix #6157