Kernel#capture replaced by version which can catch output from subprocesses #7763

Merged
merged 1 commit into from Nov 9, 2012

Projects

None yet

4 participants

@route
Contributor
route commented Sep 26, 2012

After this seattlerb/minitest#161 Aaron asked to replace capture with new version.

@route
Contributor
route commented Sep 26, 2012
@mhuggins mhuggins and 1 other commented on an outdated diff Sep 26, 2012
...pport/lib/active_support/core_ext/kernel/reporting.rb
def capture(stream)
- begin
- stream = stream.to_s
- eval "$#{stream} = StringIO.new"
- yield
- result = eval("$#{stream}").string
- ensure
- eval("$#{stream} = #{stream.upcase}")
- end
+ captured_stream = Tempfile.new(stream.to_s)
+ stream_io = eval("$#{stream}")
@mhuggins
mhuggins Sep 26, 2012

You're essentially calling stream.to_s two lines in a row. Why not leave the original line that reads stream = stream.to_s?

@route
route Sep 26, 2012 Contributor

Good catch, updated

@route
Contributor
route commented Oct 2, 2012

@tenderlove just ping!

@frodsan
Contributor
frodsan commented Oct 26, 2012

This needs a rebase.

@route
Contributor
route commented Nov 1, 2012

@frodsan done

@carlosantoniodasilva carlosantoniodasilva merged commit b67a03c into rails:master Nov 9, 2012
@carlosantoniodasilva

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment