Skip to content

Commit

Permalink
Refactor and fix echo()
Browse files Browse the repository at this point in the history
* Error shall be raised when [echo_enabled, echo_stderr] = [false,
true], however, the previous version could set this combination by
invoking echo(false, false), then, echo(nil, true). This issue is fixed
with this commit.
  • Loading branch information
fenrir-naru committed Aug 29, 2018
1 parent 77254ab commit 5973181
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 22 deletions.
45 changes: 24 additions & 21 deletions lib/rinruby.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,6 @@ class RinRuby

require 'socket'

attr_reader :interactive
attr_reader :readline
# Exception for closed engine
EngineClosed=Class.new(Exception)
# Parse error
Expand All @@ -75,7 +73,9 @@ class RinRuby
RinRuby_Env = ".RinRuby"
RinRuby_Endian = ([1].pack("L").unpack("C*")[0] == 1) ? (:little) : (:big)

attr_accessor :echo_enabled
attr_reader :interactive
attr_reader :readline
attr_reader :echo_enabled
attr_reader :executable
attr_reader :port_number
attr_reader :port_width
Expand Down Expand Up @@ -119,6 +119,7 @@ def initialize(*args)
[:port_width, :executable, :hostname, :interactive, [:echo, :echo_enabled]].each{|k_src, k_dst|
Kernel.eval("@#{k_dst || k_src} = @opts[:#{k_src}]", binding)
}
@echo_stderr = false

while true
@port_number = @opts[:port_number] + rand(@opts[:port_width])
Expand All @@ -130,7 +131,6 @@ def initialize(*args)
end
end

@echo_stderr = false
@platform = case RUBY_PLATFORM
when /mswin/, /mingw/, /bccwin/ then 'windows'
when /cygwin/ then 'windows-cygwin'
Expand Down Expand Up @@ -161,8 +161,9 @@ def initialize(*args)
EOF
@socket = nil
[:socket_io, :get_value, :pull, :check].each{|fname| self.send("r_rinruby_#{fname}")}

echo(nil, true) if @platform =~ /.*-java/ # Redirect error messages on the Java platform

# Echo setup; redirect error messages on the Java platform
(@platform =~ /.*-java/) ? echo(true, true) : echo()
end

#The quit method will properly close the bridge between Ruby and R, freeing up system resources. This method does not need to be run when a Ruby script ends.
Expand Down Expand Up @@ -456,23 +457,25 @@ def pull(string, singletons=false)
#
#* stderr: Setting stderr to true will force messages, warnings, and errors from R to be routed through stdout. Using stderr redirection is typically not needed for the C implementation of Ruby and is thus not not enabled by default for this implementation. It is typically necessary for jRuby and is enabled by default in this case. This redirection works well in practice but it can lead to interleaving output which may confuse RinRuby. In such cases, stderr redirection should not be used. Echoing must be enabled when using stderr redirection.

def echo(enable=nil,stderr=nil)
if ( enable == false ) && ( stderr == true )
raise "You can only redirect stderr if you are echoing is enabled."
end
if ( enable != nil ) && ( enable != @echo_enabled )
echo(nil,false) if ! enable
@echo_enabled = ! @echo_enabled
def echo(enable=nil, stderr=nil)
next_enabled = (enable == nil) ? @echo_enabled : (enable ? true : false)
next_stderr = case stderr
when nil
(next_enabled ? @echo_stderr : false)
else
(stderr ? true : false)
end
if @echo_enabled && ( stderr != nil ) && ( stderr != @echo_stderr )
@echo_stderr = ! @echo_stderr
if @echo_stderr
eval "sink(stdout(),type='message')"
else
eval "sink(type='message')"
end

if (next_enabled == false) && (next_stderr == true) then # prohibited combination
raise "You can only redirect stderr if you are echoing is enabled."
end
[ @echo_enabled, @echo_stderr ]

eval "sink(#{'stdout(),' if next_stderr}type='message')" if @echo_stderr != next_stderr
[@echo_enabled = next_enabled, @echo_stderr = next_stderr]
end

def echo_enabled=(enable)
echo(enable).first
end

private
Expand Down
6 changes: 5 additions & 1 deletion spec/rinruby_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,11 @@
describe "on init" do
after{(r.quit rescue nil) if defined?(r)}
it "should accept parameters as specified on Dahl & Crawford(2009)" do
expect(r.echo_enabled).to be_falsy
if r.instance_variable_get(:@platform) =~ /.*-java/
expect(r.echo_enabled).to be_truthy # For jruby, echo will be overridden as true
else
expect(r.echo_enabled).to be_falsy
end
expect(r.interactive).to be_falsy
case r.instance_variable_get(:@platform)
when /^windows/ then
Expand Down

0 comments on commit 5973181

Please sign in to comment.