-
Notifications
You must be signed in to change notification settings - Fork 121
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
Warn and do nothing if block is passed to measure command #813
Conversation
puts "#{added[0]} is added." if added | ||
else | ||
if block_given? | ||
IRB.conf[:MEASURE] = true |
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.
Changing IRB.conf[:MEASURE]
is moved to IRB.set_measure_calback
and IRB.unset_measure_calback
.
This refactor also makes IRB.set_measure_calback do end
to be a complete alternative of measure do end
@@ -235,6 +236,7 @@ def IRB.unset_measure_callback(type = nil) | |||
type_sym = type.upcase.to_sym | |||
IRB.conf[:MEASURE_CALLBACKS].reject!{ |t, | t == type_sym } | |||
end | |||
IRB.conf[:MEASURE] = nil if IRB.conf[:MEASURE_CALLBACKS].empty? |
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.
measure :off, :measure_type
had a bug
# lib/irb/cmd/measure.rb:20
when :off
IRB.conf[:MEASURE] = nil
IRB.unset_measure_callback(arg)
That disables all measure even if arg is specified.
irb> measure :foo
"FOO" is added
irb> measure :bar
"BAR" is added
irb> measure :off, :foo
# removes :foo and turns all measure off
irb> 42
# no measure
irb> measure
"TIME" added. (and BAR is re-enabled)
2f79d19
to
4bda1a3
Compare
IRB.unset_measure_callback(arg) | ||
when :list | ||
IRB.conf[:MEASURE_CALLBACKS].each do |type_name, _, arg_val| | ||
puts "- #{type_name}" + (arg_val ? "(#{arg_val.inspect})" : '') | ||
end | ||
when :on | ||
IRB.conf[:MEASURE] = true | ||
added = IRB.set_measure_callback(type, arg) | ||
added = IRB.set_measure_callback(arg) |
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.
set_measure_callback(:on, arg)
had no effect (unless IRB.conf[:MEASURE_PROC][:ON] is set)
This will fix a bug that measure :on
was enabling TIME without showing "TIME" is added.
message.
This will also allow measure :on, :foobar
, an opposite of measure :off, :foobar
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.
Ruby 3.3 is a good opportunity to add a warning message for measure command with block.
(Although, I think no one reads the warning because (maybe) no one use it.)
I searched the commit that added measure with block but it had no desciption. #183 #184
Measure command with block is
measure{heavy_logic_to_be_measured}
.Usage of measure with block was
Alternative API of measure with block
Alternative workaround for measure with block
Measure with block will register a session-only measure method that you don't need to write to `.irbrc`` for reusing. For this purpose, these workaround is enough and easier than using measure command with block. That's why I think no one use it.