1)
The lookup for Stream constant is slightly different in Ruby:
require 'YAML'
class Module
def const_missing name
puts "missing #{name}"
$S
end
end
module YAML
$S = Stream
remove_const "Stream"
end
p YAML::load_stream(File.new("C:\\temp\\yaml.txt"))
---
To achieve the same behavior use RubyUtils.GetConstant(context, self, "Stream",
false) instead of TryLookupConstant. This method also throws the right
exception on failure.
2)
In YAML.Stream.Emit:
return RubyYaml.DumpAll(context,
RubyUtils.GetExecutionContext(context).GetModule(typeof(YamlStreamOps)),
self.Documents, io);
Since DumpAll method doesn't use "self" parameter, it's unnecessary to create
one. It would be better to define a new method that doesn't take the argument
and call it from Emit.
3) YamlStream fields seem to be readonly, yet they are not marked as such.
Also, it seems that you expect _documents to be non-nullable (mark it by /*!*/
if so).
4) YAML::Stream could be used as a base class in Ruby:
class MyStream < YAML::Stream
end
To enable that, YamlStream class needs to have a default constructor (could be
protected).
Tomas
-----Original Message-----
From: Oleg Tkachenko
Sent: Friday, June 13, 2008 5:09 PM
To: IronRuby External Code Reviewers
Cc: [email protected]
Subject: Code Review: YamlTestsAndStream
tfpt review "/shelveset:YamlTestsAndStream;REDMOND\olegtkac"
Comment :
Adds some basic unit tests (taken from yaml spec and ruby 1.8.6).
Fixes a bug with registration of predefined ruby contructors, which allows to
load more that one yaml document.
Checks if YAML::Stream class is avaliable and fails gracefully if not.
Introduces YAML::Stream class implementation.
--
Oleg
_______________________________________________
Ironruby-core mailing list
[email protected]
http://rubyforge.org/mailman/listinfo/ironruby-core
