[Ironruby-core] Code Review: Fix for File.print misbehavior

Shri Borde Shri.Borde at microsoft.com
Fri May 15 13:06:07 EDT 2009


Thanks for the first patch, Shay! Couple of questions and comments.

Have you signed the contributor agreement mentioned at the top of http://wiki.github.com/ironruby/ironruby/contributing?

Please see http://wiki.github.com/ironruby/ironruby/modifying-the-sources for coding conventions. Opening braces should go on the same line as the "if" statement.

You should make sure that there is a RubySpec spec for your fixes. In this case, there should be a spec in Merlin\External.LCA_RESTRICTED\Languages\IronRuby\mspec\rubyspec\core\io\print_spec.rb. It does look like the example "writes each obj.to_s to the stream and appends $\\ (if any) given multiple objects" corresponds to your fix. If there is an existing spec, you should delete the corresponding tag from Merlin\External.LCA_RESTRICTED\Languages\IronRuby\mspec\ironruby-tags\core\io\print_tags.txt. Please delete the print_tags.txt file since there are not more tags left in that file.

You did run irtests.bat, right? There are currently 5 known failures from run.bat and 19 from the RubySpec specs (all of which should go away with the next push from Microsoft TFS to GIT).

Thanks again!

Shri


-----Original Message-----
From: ironruby-core-bounces at rubyforge.org [mailto:ironruby-core-bounces at rubyforge.org] On Behalf Of Shay Friedman
Sent: Friday, May 15, 2009 8:20 AM
To: ironruby-core at rubyforge.org
Subject: [Ironruby-core] Code Review: Fix for File.print misbehavior

Commit Description:
Using the File.print method after the global input record separator had
been changed generated a different output than the MRI.

For example, consider the next code block:
$\ = ">>"
file =  File.open("a.txt","w")
file.print "One ", 1, ", Two ", 2
file.close

Output file on MRI: One 1, Two 2>>
Output file on IronRuby: One >>1>>, Two >>2>>

This commit fixes this behavior.

Commit site:
http://github.com/shayfriedman/ironruby/commit/0ccacf09b53c231f21851391d616b0fa53026197

Files changed:
Merlin/Main/Languages/Ruby/Libraries.LCA_RESTRICTED/Builtins/IoOps.cs

Thanks,
Shay.
-- 
Posted via http://www.ruby-forum.com/.
_______________________________________________
Ironruby-core mailing list
Ironruby-core at rubyforge.org
http://rubyforge.org/mailman/listinfo/ironruby-core



More information about the Ironruby-core mailing list