Skip to content

Commit efc5d78

Browse files
Save and restore thread-locals in Capture#run and with_id
Three scope helpers in StdoutRouter hard-reset thread state on exit instead of restoring the value they replaced: * Capture#run set Thread.current.report_on_exception = false and never restored it, permanently disabling exception reporting on any thread that ran a capture. * Capture#run reset :cliui_current_capture to nil, so a nested capture left the still-active outer capture invisible to current_capture consumers such as in_alternate_screen. * with_id reset :cliui_output_id to nil, so a nested ID scope stripped the outer scope's output labelling for the rest of the outer block. Save the previous value and restore it in the ensure block, matching how Capture#run already handles :no_cliui_frame_inset and :cliui_output_hook. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 83cbecb commit efc5d78

2 files changed

Lines changed: 55 additions & 2 deletions

File tree

lib/cli/ui/stdout_router.rb

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -209,12 +209,14 @@ def run
209209

210210
StdoutRouter.assert_enabled!
211211

212+
prev_capture = Thread.current[:cliui_current_capture]
212213
Thread.current[:cliui_current_capture] = self
213214

214215
prev_frame_inset = Thread.current[:no_cliui_frame_inset]
215216
prev_hook = Thread.current[:cliui_output_hook]
216217

217218
if Thread.current.respond_to?(:report_on_exception)
219+
prev_report_on_exception = Thread.current.report_on_exception
218220
Thread.current.report_on_exception = false
219221
end
220222

@@ -238,7 +240,10 @@ def run
238240
ensure
239241
Thread.current[:cliui_output_hook] = prev_hook
240242
Thread.current[:no_cliui_frame_inset] = prev_frame_inset
241-
Thread.current[:cliui_current_capture] = nil
243+
Thread.current[:cliui_current_capture] = prev_capture
244+
unless prev_report_on_exception.nil?
245+
Thread.current.report_on_exception = prev_report_on_exception
246+
end
242247
end
243248

244249
#: -> String
@@ -318,6 +323,7 @@ class << self
318323
def with_id(on_streams:, &block)
319324
require 'securerandom'
320325
id = format('%05d', rand(10**5))
326+
prev_id = Thread.current[:cliui_output_id]
321327
Thread.current[:cliui_output_id] = {
322328
id: id,
323329
streams: on_streams.map do |stream|
@@ -326,7 +332,7 @@ def with_id(on_streams:, &block)
326332
}
327333
yield(id)
328334
ensure
329-
Thread.current[:cliui_output_id] = nil
335+
Thread.current[:cliui_output_id] = prev_id
330336
end
331337

332338
#: -> Hash[Symbol, (String | io_like)]?

test/cli/ui/stdout_router_test.rb

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,53 @@ def test_current_id
2424
end
2525
end
2626

27+
def test_nested_with_id_restores_outer_id
28+
StdoutRouter.with_id(on_streams: [$stdout]) do |outer_id|
29+
StdoutRouter.with_id(on_streams: [$stdout]) do |inner_id|
30+
assert_equal(inner_id, StdoutRouter.current_id&.fetch(:id))
31+
end
32+
assert_equal(outer_id, StdoutRouter.current_id&.fetch(:id))
33+
end
34+
assert_nil(StdoutRouter.current_id)
35+
end
36+
37+
def test_capture_restores_report_on_exception
38+
capture_io do
39+
StdoutRouter.with_enabled do
40+
prev = Thread.current.report_on_exception
41+
begin
42+
Thread.current.report_on_exception = true
43+
during = nil
44+
StdoutRouter::Capture.new { during = Thread.current.report_on_exception }.run
45+
assert_equal(false, during)
46+
assert_equal(true, Thread.current.report_on_exception)
47+
ensure
48+
Thread.current.report_on_exception = prev
49+
end
50+
end
51+
end
52+
end
53+
54+
def test_nested_capture_restores_outer_capture
55+
capture_io do
56+
StdoutRouter.with_enabled do
57+
inner_current = nil
58+
restored_current = nil
59+
inner = StdoutRouter::Capture.new do
60+
inner_current = StdoutRouter::Capture.current_capture
61+
end
62+
outer = StdoutRouter::Capture.new do
63+
inner.run
64+
restored_current = StdoutRouter::Capture.current_capture
65+
end
66+
outer.run
67+
assert_same(inner, inner_current)
68+
assert_same(outer, restored_current)
69+
assert_nil(StdoutRouter::Capture.current_capture)
70+
end
71+
end
72+
end
73+
2774
def test_frame_can_autoload_after_router_is_enabled
2875
script = <<~RUBY
2976
require 'stringio'

0 commit comments

Comments
 (0)