Looks good.
Tomas
From: Jimmy S.
Sent: Wednesday, April 22, 2009 10:56 AM
To: Tomas M.; Jim D.; [email protected]
Cc: IronRuby External Code R.
Subject: RE: Code Review: FILE and $PROGRAM_NAME paths
http://github.com/jschementi/ironruby/commit/43514f8f9e980ba22dfb5b8661e5633661f50d1f
Remove Glob.CanonicalizePath (in favor of RubyUtils.CanonicalizePath)
Merlin/Main/Languages/Ruby/Libraries.LCA_RESTRICTED/Builtins/FileOps.cs
From: Tomas M.
Sent: Tuesday, April 21, 2009 5:38 PM
To: Jimmy S.; Jim D.; [email protected]
Cc: IronRuby External Code R.
Subject: RE: Code Review: FILE and $PROGRAM_NAME paths
Could you remove
public static MutableString/!/ CanonicalizePath(MutableString/!/
path) {
return RubyUtils.CanonicalizePath(path);
}
from Glob and call RubyUtils directly wherever Glob.CanonicalizePath is
used?
Other than that looks good.
Tomas
From: Jimmy S.
Sent: Tuesday, April 21, 2009 5:19 PM
To: Jim D.; [email protected]
Cc: IronRuby External Code R.
Subject: RE: Code Review: FILE and $PROGRAM_NAME paths
http://github.com/jschementi/ironruby/commit/57d18b591ba9cba92923864936f359de446e2f97
Code Review changes
-
Moves command_line/fixtures/file* and language/fixtures/file* to
fixtures/
-
Create RubyUtils.CanonicalizePath, to dependency on
IronRuby.Builtins.Glob from IronRuby.Hosting
Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/rubyspec/command_line/fixtures/file.rb
*
Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/rubyspec/command_line/fixtures/file/sub.rb
*
Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/rubyspec/command_line/program_name_spec.rb
*
Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/rubyspec/fixtures/file.rb
*
Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/rubyspec/fixtures/file/sub.rb
*
Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/rubyspec/language/file_spec.rb
*
Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/rubyspec/language/fixtures/file.rb
*
Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/rubyspec/language/fixtures/file/sub.rb
*
Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/rubyspec/language/predefined_spec.rb
* Merlin/Main/Languages/Ruby/Ruby/Builtins/Glob.cs
* Merlin/Main/Languages/Ruby/Ruby/Hosting/RubyCommandLine.cs
* Merlin/Main/Languages/Ruby/Ruby/Hosting/RubyOptionsParser.cs
* Merlin/Main/Languages/Ruby/Ruby/Runtime/RubyUtils.cs
From: Jim D.
Sent: Monday, April 20, 2009 6:53 PM
To: Jimmy S.; [email protected]
Cc: IronRuby External Code R.
Subject: RE: Code Review: FILE and $PROGRAM_NAME paths
Comment left on Github:
Can you move the fixtures up a level and combine them? (Put them in
rubyspec/fixtures instead of rubyspec/command_line/fixtures and
rubyspec/language/fixtures)
Also, shouldn’t we move Glob.CanonicalizePath to a more central
location, then use that central location for both the hosting and the
Glob implementation? Seems like this couples the hosting API to Glob,
which doesn’t seem good.
Other than that it looks good.
JD
From: Jimmy S.
Sent: Monday, April 20, 2009 3:37 PM
To: [email protected]
Cc: IronRuby External Code R.
Subject: Code Review: FILE and $PROGRAM_NAME paths
http://github.com/jschementi/ironruby/commit/3ef1b6ac9718d0c55796c2818da797ea8bab7275
Makes FILE and $PROGRAM_NAME (and $0) have canonicalized paths when
Ruby is hosted from ir.exe. However, FILE is not messed with when
including a file via require/load. Fixes
http://ironruby.codeplex.com/WorkItem/View.aspx?WorkItemId=545.