Add a warn form that takes a callback#7685
Conversation
headius
force-pushed
the
warning_callback_for_message
branch
from
cd1bf63 to
536b068
Compare
enebo
left a comment
enebo
left a comment
There was a problem hiding this comment.
I think the ID should get removed. The idea of it is nice and was originally useful for ruby-ide but we have long stopped using our main parser for that. We do not use ID for anything internally and most new warns do what you just did and mark it miscellaneous.
If we do ever have a need for a progmatic warn system I think we should consider completely rewriting our warn system.
I do really like the notion of making all warns lazy like this to reduce accidental overhead of forumlating a warn string to just not use it because warn is off.
|
@enebo This does avoid building the string until the last moment, but it introduces the construction of a lambda object. The callback form could be expanded to pass in a few key objects to avoid that allocation, like we do for Hash and lock callbacks elsewhere. I'll see if I can cut the ID stuff out of the new code. |
headius
force-pushed
the
warning_callback_for_message
branch
from
536b068 to
004f5dd
Compare
Callback receives the warning's stack trace element and must return a formatted message. This allows reusing the same single trace element rather than generating it twice to build the message elsewhere. Fixes jruby#7675
Usually all we need to build a message for the warning is the execution context (usually will be ThreadContext) and some state (a Ruby object in this case). Pass them through so the lambda can be stateless.
headius
force-pushed
the
warning_callback_for_message
branch
from
06f7b7d to
8cf5130
Compare
|
My commits got a little mixed up; see new hashes. |
2 participants
Callback receives the warning's stack trace element and must return a formatted message. This allows reusing the same single trace element rather than generating it twice to build the message elsewhere.