tfpt review /shelveset:merge-3;REDMOND\jflam
Ruby only
This shelveset fixes a number of open bugs on Rubyforge and adds some features
that we need to get the latest rubinius specs running.
MatchDataOps:
- we used to alias MatchData to System.Text.RegularExpressions.Match.
However, to fully implement MatchData we need to also hold onto a reference to
the original string that we matched against. A new MatchData type was created
(MatchData.cs) which wraps the existing .NET Match object. This also touches
MutableStringOps.cs, RubyOps.cs, RubyScope.cs, SpecialGlobalVariableInfo.cs
- finished implementing all methods. 16 out of 16 specs pass
- implemented to_a which closes bug #19903
RegexpOps:
- added ctor overload to close bug #19927
- made some changes to return MatchData objects instead of Match objects
- made some changes to return boxed integers via
RuntimeHelpers.Int32ToObject() instead of explicit boxing. This also touches
MutableStringOps.cs
TimeOps:
- removed overloaded constructors to close bug #19956. But this exposes a
new problem about Type aliasing that is described by bug 20035 (referencing a
System::DateTime explicitly does not allow you to call the .NET constructors -
only the Ruby-defined constructors).
- fixed Time#- bug - #19955
ModuleOps:
- fixed some very old bugs #15996, #15995 related to including
non-sensible things (include 1, include nil). RequireNonClasses() method now
throws the correct Ruby exceptions. This also touches SingletonOps.cs
MutableStringOps:
- added an implementation of String#rindex which closes #19904. We pass
all specs except for 4 which fail because of differences between .NET regex and
Ruby regex.
Dir.cs:
- cleaned up some code here, was about to work on glob implementation
(bugs #19843 #19950, but handed off to Curt)
FileOps.cs:
- added an implementation of File#basename which closes #19905. passes all
specs except for one which is wrong, and one which is unix-specific. These
things are likely bugs in Ruby.
#File.basename(''baz.rb'', ''z.rb'').should
== ''ba'' -- bad test
#File.basename("bar.txt.exe", ".txt.exe").should ==
"bar" - unix-only, should be wrapped in platform
- added an implementation of File.file? to close #19949
MutableString.cs:
- added a static Empty MutableString
- added delegation thunks to LastIndexOf()
- fixed our Equals() implementation to correctly distinguish between
different types of strings (CLR vs. MutableString)
File.cs:
- fixes a bug related to what "w+" means in .NET. - its
FileMode.OpenOrCreate
Fixed bug #19885 by adding the -I command line switch which lets you specify the
library load path. I''ve redefined our internal alias.txt file to
generate aliases for rbd and rbx that include a pointer to where the MRI libs
are stored in our layout.
Fixed bug #17810, which was a long-standing perf bug.
Fixed bug #20007, by allowing users to pass nothing for
attr_accessor/reader/writer. This is really a bug in Ruby, but the current
implementations allow this behavior, and there are apps that actually depend on
it (RbYAML).
Thanks,
-John
-------------- next part --------------
A non-text attachment was scrubbed...
Name: merge-3.diff
Type: application/octet-stream
Size: 355608 bytes
Desc: merge-3.diff
URL:
<http://rubyforge.org/pipermail/ironruby-core/attachments/20080510/941ab20a/attachment-0001.obj>
Quick question, are these following assertions are correct?
??? should_raise(TypeError){ File.basename(1) }
??? should_raise(TypeError){ File.basename("bar.txt", 1) }
??? should_raise(TypeError){ File.basename(true) }
I was expecting all the above three should be ArgumentError than TypeError,
until unless, you expect it to be TypeError, when I run the changes I have made
for the all the above three by default you get the following
error>>> File.basename(1)
:0:in `Initialize##1'': wrong number or type of arguments for
`basename'' (Argumen
tError)
let me know if you would like me to validate and force throw TypeError
execption.
Also, in the basename_spec the test setup has wrong parameter on
File.open(@name, ''w+''), if I change it to
''w'', the test runs fine otherwise none of the test works.
Thanks,
----- Original Message ----
From: John Lam (IRONRUBY) <jflam at microsoft.com>
To: IronRuby External Code Reviewers <irbrev at microsoft.com>
Cc: "ironruby-core at rubyforge.org" <ironruby-core at
rubyforge.org>
Sent: Saturday, May 10, 2008 3:16:08 AM
Subject: [Ironruby-core] RubyForge bug fixes
tfpt review /shelveset:merge-3;REDMOND\jflam
Ruby only
This shelveset fixes a number of open bugs on Rubyforge and adds some features
that we need to get the latest rubinius specs running.
MatchDataOps:
-? ? ? we used to alias MatchData to System.Text.RegularExpressions.Match.
However, to fully implement MatchData we need to also hold onto a reference to
the original string that we matched against. A new MatchData type was created
(MatchData.cs) which wraps the existing .NET Match object. This also touches
MutableStringOps.cs, RubyOps.cs, RubyScope.cs, SpecialGlobalVariableInfo.cs
-? ? ? finished implementing all methods. 16 out of 16 specs pass
-? ? ? implemented to_a which closes bug #19903
RegexpOps:
-? ? ? added ctor overload to close bug #19927
-? ? ? made some changes to return MatchData objects instead of Match objects
-? ? ? made some changes to return boxed integers via
RuntimeHelpers.Int32ToObject() instead of explicit boxing. This also touches
MutableStringOps.cs
TimeOps:
-? ? ? removed overloaded constructors to close bug #19956. But this exposes a
new problem about Type aliasing that is described by bug 20035 (referencing a
System::DateTime explicitly does not allow you to call the .NET constructors -
only the Ruby-defined constructors).
-? ? ? fixed Time#- bug - #19955
ModuleOps:
-? ? ? fixed some very old bugs #15996, #15995 related to including non-sensible
things (include 1, include nil). RequireNonClasses() method now throws the
correct Ruby exceptions. This also touches SingletonOps.cs
MutableStringOps:
-? ? ? added an implementation of String#rindex which closes #19904. We pass all
specs except for 4 which fail because of differences between .NET regex and Ruby
regex.
Dir.cs:
-? ? ? cleaned up some code here, was about to work on glob implementation (bugs
#19843 #19950, but handed off to Curt)
FileOps.cs:
-? ? ? added an implementation of File#basename which closes #19905. passes all
specs except for one which is wrong, and one which is unix-specific. These
things are likely bugs in Ruby.
? ? #File.basename(''baz.rb'', ''z.rb'').should
== ''ba'' -- bad test
? ? #File.basename("bar.txt.exe", ".txt.exe").should ==
"bar" - unix-only, should be wrapped in platform
-? ? ? added an implementation of File.file? to close #19949
MutableString.cs:
-? ? ? added a static Empty MutableString
-? ? ? added delegation thunks to LastIndexOf()
-? ? ? fixed our Equals() implementation to correctly distinguish between
different types of strings (CLR vs. MutableString)
File.cs:
-? ? ? fixes a bug related to what "w+" means in .NET. - its
FileMode.OpenOrCreate
Fixed bug #19885 by adding the -I command line switch which lets you specify the
library load path. I''ve redefined our internal alias.txt file to
generate aliases for rbd and rbx that include a pointer to where the MRI libs
are stored in our layout.
Fixed bug #17810, which was a long-standing perf bug.
Fixed bug #20007, by allowing users to pass nothing for
attr_accessor/reader/writer. This is really a bug in Ruby, but the current
implementations allow this behavior, and there are apps that actually depend on
it (RbYAML).
Thanks,
-John
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://rubyforge.org/pipermail/ironruby-core/attachments/20080510/0a060786/attachment.html>
Unnikrishnan Nair:> Quick question, are these following assertions are correct? > > should_raise(TypeError){ File.basename(1) } > should_raise(TypeError){ File.basename("bar.txt", 1) } > should_raise(TypeError){ File.basename(true) }Yes they are. You can easily verify this in MRI yourself. In first and third cases, it hits the overload that accepts a nullable object as the first parameter. The TypeError is raised via Protocols.CastToString(). In the second case, it also hits an overload that accepts a nullable object as its second parameter, which raises TypeError via Protocols.CastToString(). FYI, this is the implementation of #basename that I have in my shelveset. It passes all of the (valid) specs - there are some legitimate bugs in our old copy of the specs (I haven''t checked with the latest version of the rubinius specs - I''ll do that next week). I''m a bit worried about how I''m handling the special cases for Windows. [RubyMethod("basename", RubyMethodAttributes.PublicSingleton)] public static MutableString/*!*/ Basename(CodeContext/*!*/ context, object/*!*/ self, [NotNull]MutableString/*!*/ path, [NotNull]MutableString/*!*/ extensionFilter) { if (path.Length == 0) return path; MutableString trimmedPath = TrimTrailingSlashes(path); // Special cases of drive letters C:\\ or C:/ if (trimmedPath.Length == 2) if (Char.IsLetter(trimmedPath.GetChar(0)) && trimmedPath.GetChar(1) == '':'') return Kernel.FlowTaint(context, path, (path.Length > 2 ? MutableString.Create(path.GetChar(2).ToString()) : MutableString.Create(String.Empty))); string trimmedPathAsString = trimmedPath.ConvertToString(); if (trimmedPathAsString == "/") return trimmedPath; string filename = System.IO.Path.GetFileName(trimmedPath.ConvertToString()); // Handle UNC host names correctly string root = System.IO.Path.GetPathRoot(trimmedPath.ConvertToString()); if (extensionFilter.Length == 0) return trimmedPathAsString == root ? MutableString.Create(root) : MutableString.Create(filename); string fileExtension = System.IO.Path.GetExtension(filename); string basename = System.IO.Path.GetFileNameWithoutExtension(filename); string result = WildcardExtensionMatch(fileExtension, extensionFilter.ConvertToString()) ? basename : filename; return Kernel.FlowTaint(context, self, (result.Equals(root) ? MutableString.Create(root) : MutableString.Create(result))); } [RubyMethod("basename", RubyMethodAttributes.PublicSingleton)] public static MutableString/*!*/ Basename(CodeContext/*!*/ context, object/*!*/ self, [NotNull]MutableString/*!*/ path) { return Basename(context, self, path, MutableString.Empty); } [RubyMethod("basename", RubyMethodAttributes.PublicSingleton)] public static MutableString/*!*/ Basename(CodeContext/*!*/ context, object/*!*/ self, object path, object extension) { return Basename(context, self, Protocols.CastToString(context, path), Protocols.CastToString(context, extension)); } [RubyMethod("basename", RubyMethodAttributes.PublicSingleton)] public static MutableString/*!*/ Basename(CodeContext/*!*/ context, object/*!*/ self, object path) { return Basename(context, self, Protocols.CastToString(context, path)); }> Also, in the basename_spec the test setup has wrong parameter on > File.open(@name, ''w+''), if I change it to ''w'', the test runs fine > otherwise none of the test works.This is correct. There is a bug in how we map the semantics of ''w+'' to .NET IO. It''s fixed in my shelveset. Thanks, -John
Hi John,
Here is my code that I have, this is the first method I attemepted since this is
one of the interesting logic. I thought I post my code for you to review as
well. This passed most of the test as well[
{
returnString = path;
}
[
{
}
[
{RubyMethod("basename",
RubyMethodAttributes.PublicSingleton)]publicstaticMutableStringBasename(CodeContext/*!*/context,
object/*!*/self, [NotNull]MutableString/*!*/path)MutableString[] tokens =
path.Split(@"\/".ToCharArray(), Int32.MaxValue,
StringSplitOptions.RemoveEmptyEntries);MutableStringreturnString =
(tokens.GetLength(0) > 0) ? tokens[tokens.GetLength(0) - 1] : path;
if((tokens.GetLength(0) > 0) &&
(returnString.GetChar(returnString.Length -
1).Equals('':'')))returnreturnString;RubyMethod("basename",
RubyMethodAttributes.PublicSingleton)]publicstaticMutableStringBasename(CodeContext/*!*/context,
object/*!*/self, object/*!*/path)returnBasename(context, self,
Protocols.CastToString(context, path));RubyMethod("basename",
RubyMethodAttributes.PublicSingleton)]publicstaticMutableStringBasename(CodeContext/*!*/context,
object/*!*/self, [NotNull]MutableString/*!*/path,
[NotNull]MutableString/*!*/filter)MutableStringfilename = Basename(context,
self,
path);//Treat ? speciallyif(filter.IndexOf(''?'') >=
0)returnfilename;//Treat .* and exetions specially using regex{
filter.Clear();
filter.Append(
}if(filter.Equals(MutableString.Create(".*")))@"(?x)(\.[^.]*$|$)");else{
filter.Append(
}
System.Text.RegularExpressions.
filename.Replace(mat.Index, mat.Length,
}
[
{
}
Let me know what do you think? Yes, I found some problems with test spec as
well, (bar.txt should be baz)
Thanks."$");RubyRegexreg = newRubyRegex(filter,
System.Text.RegularExpressions.RegexOptions.Singleline);Matchmat =
reg.Match(filename);if(mat.Success)MutableString.Create(""));returnfilename;RubyMethod("basename",
RubyMethodAttributes.PublicSingleton)]publicstaticMutableStringBasename(CodeContext/*!*/context,
object/*!*/self, object/*!*/path, object/*!*/filter)returnBasename(context,
self, Protocols.CastToString(context, path), Protocols.CastToString(context,
filter));
----- Original Message ----
From: John Lam (IRONRUBY) <jflam at microsoft.com>
To: "ironruby-core at rubyforge.org" <ironruby-core at
rubyforge.org>
Sent: Saturday, May 10, 2008 3:25:16 PM
Subject: Re: [Ironruby-core] RubyForge bug fixes
Unnikrishnan Nair:
> Quick question, are these following assertions are correct?
>
>? ? should_raise(TypeError){ File.basename(1) }
>? ? should_raise(TypeError){ File.basename("bar.txt", 1) }
>? ? should_raise(TypeError){ File.basename(true) }
Yes they are. You can easily verify this in MRI yourself.
In first and third cases, it hits the overload that accepts a nullable object as
the first parameter. The TypeError is raised via Protocols.CastToString().
In the second case, it also hits an overload that accepts a nullable object as
its second parameter, which raises TypeError via Protocols.CastToString().
FYI, this is the implementation of #basename that I have in my shelveset. It
passes all of the (valid) specs - there are some legitimate bugs in our old copy
of the specs (I haven''t checked with the latest version of the rubinius
specs - I''ll do that next week). I''m a bit worried about how
I''m handling the special cases for Windows.
[RubyMethod("basename", RubyMethodAttributes.PublicSingleton)]
public static MutableString/*!*/ Basename(CodeContext/*!*/ context, object/*!*/
self, [NotNull]MutableString/*!*/ path, [NotNull]MutableString/*!*/
extensionFilter) {
? ? if (path.Length == 0)
? ? ? ? return path;
? ? MutableString trimmedPath = TrimTrailingSlashes(path);
? ? // Special cases of drive letters C:\\ or C:/
? ? if (trimmedPath.Length == 2)
? ? ? ? if (Char.IsLetter(trimmedPath.GetChar(0)) &&
trimmedPath.GetChar(1) == '':'')
? ? ? ? ? ? return Kernel.FlowTaint(context, path, (path.Length > 2 ?
MutableString.Create(path.GetChar(2).ToString()) :
MutableString.Create(String.Empty)));
? ? string trimmedPathAsString = trimmedPath.ConvertToString();
? ? if (trimmedPathAsString == "/")
? ? ? ? return trimmedPath;
? ? string filename = System.IO.Path.GetFileName(trimmedPath.ConvertToString());
? ? // Handle UNC host names correctly
? ? string root = System.IO.Path.GetPathRoot(trimmedPath.ConvertToString());
? ? if (extensionFilter.Length == 0)
? ? ? ? return trimmedPathAsString == root ? MutableString.Create(root) :
MutableString.Create(filename);
? ? string fileExtension = System.IO.Path.GetExtension(filename);
? ? string basename = System.IO.Path.GetFileNameWithoutExtension(filename);
? ? string result = WildcardExtensionMatch(fileExtension,
extensionFilter.ConvertToString()) ? basename : filename;
? ? return Kernel.FlowTaint(context, self, (result.Equals(root) ?
MutableString.Create(root) : MutableString.Create(result)));
}
[RubyMethod("basename", RubyMethodAttributes.PublicSingleton)]
public static MutableString/*!*/ Basename(CodeContext/*!*/ context, object/*!*/
self, [NotNull]MutableString/*!*/ path) {
? ? return Basename(context, self, path, MutableString.Empty);
}
[RubyMethod("basename", RubyMethodAttributes.PublicSingleton)]
public static MutableString/*!*/ Basename(CodeContext/*!*/ context, object/*!*/
self, object path, object extension) {
? ? return Basename(context, self, Protocols.CastToString(context, path),
Protocols.CastToString(context, extension));
}
[RubyMethod("basename", RubyMethodAttributes.PublicSingleton)]
public static MutableString/*!*/ Basename(CodeContext/*!*/ context, object/*!*/
self, object path) {
? ? return Basename(context, self, Protocols.CastToString(context, path));
}
> Also, in the basename_spec the test setup has wrong parameter on
> File.open(@name, ''w+''), if I change it to
''w'', the test runs fine
> otherwise none of the test works.
This is correct. There is a bug in how we map the semantics of
''w+'' to .NET IO. It''s fixed in my shelveset.
Thanks,
-John
_______________________________________________
Ironruby-core mailing list
Ironruby-core at rubyforge.org
http://rubyforge.org/mailman/listinfo/ironruby-core
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://rubyforge.org/pipermail/ironruby-core/attachments/20080510/55c016fd/attachment.html>
Hi Unnikrishnan,
Just a couple of points you might find useful:
. A number of overloads have parameters like ..., object/*!*/ path,
... What you are saying to the Specsharp tool is that object should never be
null. Is this what you want? Can you pass nil into the methods in MRI? If
you can then you need to remove the /*!*/. If you can''t then you
probably
need to add [NotNull] attribute or handle the case where the parameter is
null within you method. Since you usually pass the value into CastToString,
which handles the null case then probably removing the /*!*/ is what is
required.
. In Public Singleton methods I believe you can guarantee that the
self parameter that gets passed in is actually the class that owns the
method and so you can type this parameter to RubyClass/*!*/ if you want.
Since the class is not being used in these methods it is not important, but
it can save you a cast in other methods.
. In some of the overloads you are using the Ruby regex classes. Do
you know if MRI uses these and if so do they call the methods on them
directly or via the Ruby method invocation process? If the latter then you
need to use Dynamic Invocation to call the methods in your overloads [so
that if someone monkey patches the Regex classes the behaviour shows up in
the File class]. I suspect this is not the case but worth writing an RSpec
for to check.
. It might be worth pulling the MutableString objects that you use
for comparison out into static readonly variables so that you don''t
have to
keep instantiating them every call. Such as:
private static readonly MutableString dotStar =
MutableString.Create(".*");
and
private static readonly MutableString empty = MutableString.CreateEmpty();
Hope that helps.
Pete
From: ironruby-core-bounces at rubyforge.org
[mailto:ironruby-core-bounces at rubyforge.org] On Behalf Of Unnikrishnan Nair
Sent: Saturday,10 May 10, 2008 22:59
To: ironruby-core at rubyforge.org
Subject: Re: [Ironruby-core] RubyForge bug fixes
Hi John,
Here is my code that I have, this is the first method I attemepted since
this is one of the interesting logic. I thought I post my code for you to
review as well. This passed most of the test as well
[RubyMethod("basename", RubyMethodAttributes.PublicSingleton)]
public static MutableString Basename(CodeContext/*!*/ context, object/*!*/
self, [NotNull]MutableString/*!*/ path)
{
MutableString[] tokens = path.Split(@"\/".ToCharArray(),
Int32.MaxValue,
StringSplitOptions.RemoveEmptyEntries);
MutableString returnString = (tokens.GetLength(0) > 0) ?
tokens[tokens.GetLength(0) - 1] : path;
if ((tokens.GetLength(0) > 0) &&
(returnString.GetChar(returnString.Length -
1).Equals('':'')))
returnString = path;
return returnString;
}
[RubyMethod("basename", RubyMethodAttributes.PublicSingleton)]
public static MutableString Basename(CodeContext/*!*/ context, object/*!*/
self, object/*!*/ path)
{
return Basename(context, self, Protocols.CastToString(context, path));
}
[RubyMethod("basename", RubyMethodAttributes.PublicSingleton)]
public static MutableString Basename(CodeContext/*!*/ context, object/*!*/
self, [NotNull]MutableString/*!*/ path, [NotNull]MutableString/*!*/ filter)
{
MutableString filename = Basename(context, self, path);
//Treat ? specially
if (filter.IndexOf(''?'') >= 0)
return filename;
//Treat .* and exetions specially using regex
if (filter.Equals(MutableString.Create(".*")))
{
filter.Clear();
filter.Append(@"(?x)(\.[^.]*$|$)");
}
else
{
filter.Append("$");
}
RubyRegex reg = new RubyRegex(filter,
System.Text.RegularExpressions.RegexOptions.Singleline);
System.Text.RegularExpressions.Match mat = reg.Match(filename);
if (mat.Success)
filename.Replace(mat.Index, mat.Length, MutableString.Create(""));
return filename;
}
[RubyMethod("basename", RubyMethodAttributes.PublicSingleton)]
public static MutableString Basename(CodeContext/*!*/ context, object/*!*/
self, object/*!*/ path, object/*!*/ filter)
{
return Basename(context, self, Protocols.CastToString(context, path),
Protocols.CastToString(context, filter));
}
Let me know what do you think? Yes, I found some problems with test spec as
well, (bar.txt should be baz)
Thanks.
----- Original Message ----
From: John Lam (IRONRUBY) <jflam at microsoft.com>
To: "ironruby-core at rubyforge.org" <ironruby-core at
rubyforge.org>
Sent: Saturday, May 10, 2008 3:25:16 PM
Subject: Re: [Ironruby-core] RubyForge bug fixes
Unnikrishnan Nair:
> Quick question, are these following assertions are correct?
>
> should_raise(TypeError){ File.basename(1) }
> should_raise(TypeError){ File.basename("bar.txt", 1) }
> should_raise(TypeError){ File.basename(true) }
Yes they are. You can easily verify this in MRI yourself.
In first and third cases, it hits the overload that accepts a nullable
object as the first parameter. The TypeError is raised via
Protocols.CastToString().
In the second case, it also hits an overload that accepts a nullable object
as its second parameter, which raises TypeError via
Protocols.CastToString().
FYI, this is the implementation of #basename that I have in my shelveset. It
passes all of the (valid) specs - there are some legitimate bugs in our old
copy of the specs (I haven''t checked with the latest version of the
rubinius
specs - I''ll do that next week). I''m a bit worried about how
I''m handling
the special cases for Windows.
[RubyMethod("basename", RubyMethodAttributes.PublicSingleton)]
public static MutableString/*!*/ Basename(CodeContext/*!*/ context,
object/*!*/ self, [NotNull]MutableString/*!*/ path,
[NotNull]MutableString/*!*/ extensionFilter) {
if (path.Length == 0)
return path;
MutableString trimmedPath = TrimTrailingSlashes(path);
// Special cases of drive letters C:\\ or C:/
if (trimmedPath.Length == 2)
if (Char.IsLetter(trimmedPath.GetChar(0)) &&
trimmedPath.GetChar(1)
== '':'')
return Kernel.FlowTaint(context, path, (path.Length > 2 ?
MutableString.Create(path.GetChar(2).ToString()) :
MutableString.Create(String.Empty)));
string trimmedPathAsString = trimmedPath.ConvertToString();
if (trimmedPathAsString == "/")
return trimmedPath;
string filename = System.IO <http://system.io.path.ge/>
.Path.GetFileName(trimmedPath.ConvertToString());
// Handle UNC host names correctly
string root = System.IO <http://system.io.path.ge/>
.Path.GetPathRoot(trimmedPath.ConvertToString());
if (extensionFilter.Length == 0)
return trimmedPathAsString == root ? MutableString.Create(root) :
MutableString.Create(filename);
string fileExtension = System.IO <http://system.io.path.ge/>
.Path.GetExtension(filename);
string basename = System.IO <http://system.io.path.ge/>
.Path.GetFileNameWithoutExtension(filename);
string result = WildcardExtensionMatch(fileExtension,
extensionFilter.ConvertToString()) ? basename : filename;
return Kernel.FlowTaint(context, self, (result.Equals(root) ?
MutableString.Create(root) : MutableString.Create(result)));
}
[RubyMethod("basename", RubyMethodAttributes.PublicSingleton)]
public static MutableString/*!*/ Basename(CodeContext/*!*/ context,
object/*!*/ self, [NotNull]MutableString/*!*/ path) {
return Basename(context, self, path, MutableString.Empty);
}
[RubyMethod("basename", RubyMethodAttributes.PublicSingleton)]
public static MutableString/*!*/ Basename(CodeContext/*!*/ context,
object/*!*/ self, object path, object extension) {
return Basename(context, self, Protocols.CastToString(context, path),
Protocols.CastToString(context, extension));
}
[RubyMethod("basename", RubyMethodAttributes.PublicSingleton)]
public static MutableString/*!*/ Basename(CodeContext/*!*/ context,
object/*!*/ self, object path) {
return Basename(context, self, Protocols.CastToString(context, path));
}
> Also, in the basename_spec the test setup has wrong parameter on
> File.open(@name, ''w+''), if I change it to
''w'', the test runs fine
> otherwise none of the test works.
This is correct. There is a bug in how we map the semantics of
''w+'' to .NET
IO. It''s fixed in my shelveset.
Thanks,
-John
_______________________________________________
Ironruby-core mailing list
Ironruby-core at rubyforge.org
http://rubyforge.org/mailman/listinfo/ironruby-core
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://rubyforge.org/pipermail/ironruby-core/attachments/20080511/dd377698/attachment-0001.html>
Hi Peter,
Fantastic point, now that I am understanding the code, it makes perfect sense, I
really appriciate the comments.
Thanks.
----- Original Message ----
From: Peter Bacon Darwin <bacondarwin at googlemail.com>
To: ironruby-core at rubyforge.org
Sent: Sunday, May 11, 2008 7:14:48 AM
Subject: Re: [Ironruby-core] RubyForge bug fixes
Hi Unnikrishnan,
Just a couple of points you might find useful:
????????? A number of overloads have parameters like ..., object/*!*/ path, ...
What you are saying to the Specsharp tool is that object should never be null.?
Is this what you want?? Can you pass nil into the methods in MRI?? If you can
then you need to remove the /*!*/.? If you can?t then you probably need to add
[NotNull] attribute or handle the case where the parameter is null within you
method.? Since you usually pass the value into CastToString, which handles the
null case then probably removing the /*!*/ is what is required.
????????? In Public Singleton methods I believe you can guarantee that the self
parameter that gets passed in is actually the class that owns the method and so
you can type this parameter to RubyClass/*!*/ if you want.? Since the class is
not being used in these methods it is not important, but it can save you a cast
in other methods.
????????? In some of the overloads you are using the Ruby regex classes.? Do you
know if MRI uses these and if so do they call the methods on them directly or
via the Ruby method invocation process?? If the latter then you need to use
Dynamic Invocation to call the methods in your overloads [so that if someone
monkey patches the Regex classes the behaviour shows up in the File class].? I
suspect this is not the case but worth writing an RSpec for to check.
????????? It might be worth pulling the MutableString objects that you use for
comparison out into static readonly variables so that you don?t have to keep
instantiating them every call.? Such as:
private static readonly MutableString dotStar =
MutableString.Create(".*"); and
private static readonly MutableString empty = MutableString.CreateEmpty();
Hope that helps.
Pete
?
From:ironruby-core-bounces at rubyforge.org [mailto:ironruby-core-bounces at
rubyforge.org] On Behalf Of Unnikrishnan Nair
Sent: Saturday,10 May 10, 2008 22:59
To: ironruby-core at rubyforge.org
Subject: Re: [Ironruby-core] RubyForge bug fixes
?
Hi John,
?
Here is my code that I have, this is the first method I attemepted since this is
one of the interesting logic. I thought I post my code for you to review as
well. This passed most of the test as well
?
[RubyMethod("basename", RubyMethodAttributes.PublicSingleton)]
publicstatic MutableString Basename(CodeContext/*!*/ context, object/*!*/ self,
[NotNull]MutableString/*!*/ path)
{
MutableString[] tokens = path.Split(@"\/".ToCharArray(),
Int32.MaxValue, StringSplitOptions.RemoveEmptyEntries);
MutableStringreturnString = (tokens.GetLength(0) > 0) ?
tokens[tokens.GetLength(0) - 1] : path;
if((tokens.GetLength(0) > 0) &&
(returnString.GetChar(returnString.Length - 1).Equals('':'')))
returnString = path;
returnreturnString;
}
[RubyMethod("basename", RubyMethodAttributes.PublicSingleton)]
publicstatic MutableString Basename(CodeContext/*!*/ context, object/*!*/ self,
object/*!*/ path)
{
returnBasename(context, self, Protocols.CastToString(context, path));
}
[RubyMethod("basename", RubyMethodAttributes.PublicSingleton)]
publicstatic MutableString Basename(CodeContext/*!*/ context, object/*!*/ self,
[NotNull]MutableString/*!*/ path, [NotNull]MutableString/*!*/ filter)
{
MutableStringfilename = Basename(context, self, path);
//Treat ? specially
if(filter.IndexOf(''?'') >= 0)
returnfilename;
//Treat .* and exetions specially using regex
if(filter.Equals(MutableString.Create(".*")))
{
filter.Clear();
filter.Append(@"(?x)(\.[^.]*$|$)");
}
else
{
filter.Append("$");
}
RubyRegexreg = new RubyRegex(filter,
System.Text.RegularExpressions.RegexOptions.Singleline);
System.Text.RegularExpressions.Match mat = reg.Match(filename);
if(mat.Success)
filename.Replace(mat.Index, mat.Length, MutableString.Create(""));
returnfilename;
}
[RubyMethod("basename", RubyMethodAttributes.PublicSingleton)]
publicstatic MutableString Basename(CodeContext/*!*/ context, object/*!*/ self,
object/*!*/ path, object/*!*/ filter)
{
returnBasename(context, self, Protocols.CastToString(context, path),
Protocols.CastToString(context, filter));
}
?
Let me know what do you think? Yes, I found some problems with test spec as
well, (bar.txt should be baz)
?
Thanks.
?
?
----- Original Message ----
From: John Lam (IRONRUBY) <jflam at microsoft.com>
To: "ironruby-core at rubyforge.org" <ironruby-core at
rubyforge.org>
Sent: Saturday, May 10, 2008 3:25:16 PM
Subject: Re: [Ironruby-core] RubyForge bug fixes
Unnikrishnan Nair:
> Quick question, are these following assertions are correct?
>
>? ? should_raise(TypeError){ File.basename(1) }
>? ? should_raise(TypeError){ File.basename("bar.txt", 1) }
>? ? should_raise(TypeError){ File.basename(true) }
Yes they are. You can easily verify this in MRI yourself.
In first and third cases, it hits the overload that accepts a nullable object as
the first parameter. The TypeError is raised via Protocols.CastToString().
In the second case, it also hits an overload that accepts a nullable object as
its second parameter, which raises TypeError via Protocols.CastToString().
FYI, this is the implementation of #basename that I have in my shelveset. It
passes all of the (valid) specs - there are some legitimate bugs in our old copy
of the specs (I haven''t checked with the latest version of the rubinius
specs - I''ll do that next week). I''m a bit worried about how
I''m handling the special cases for Windows.
[RubyMethod("basename", RubyMethodAttributes.PublicSingleton)]
public static MutableString/*!*/ Basename(CodeContext/*!*/ context, object/*!*/
self, [NotNull]MutableString/*!*/ path, [NotNull]MutableString/*!*/
extensionFilter) {
? ? if (path.Length == 0)
? ? ? ? return path;
? ? MutableString trimmedPath = TrimTrailingSlashes(path);
? ? // Special cases of drive letters C:\\ or C:/
? ? if (trimmedPath.Length == 2)
? ? ? ? if (Char.IsLetter(trimmedPath.GetChar(0)) &&
trimmedPath.GetChar(1) == '':'')
? ? ? ? ? ? return Kernel.FlowTaint(context, path, (path.Length > 2 ?
MutableString.Create(path.GetChar(2).ToString()) :
MutableString.Create(String.Empty)));
? ? string trimmedPathAsString = trimmedPath.ConvertToString();
? ? if (trimmedPathAsString == "/")
? ? ? ? return trimmedPath;
? ? string filename = System.IO.Path.GetFileName(trimmedPath.ConvertToString());
? ? // Handle UNC host names correctly
? ? string root = System.IO.Path.GetPathRoot(trimmedPath.ConvertToString());
? ? if (extensionFilter.Length == 0)
? ? ? ? return trimmedPathAsString == root ? MutableString.Create(root) :
MutableString.Create(filename);
? ? string fileExtension = System.IO.Path.GetExtension(filename);
? ? string basename = System.IO.Path.GetFileNameWithoutExtension(filename);
? ? string result = WildcardExtensionMatch(fileExtension,
extensionFilter.ConvertToString()) ? basename : filename;
? ? return Kernel.FlowTaint(context, self, (result.Equals(root) ?
MutableString.Create(root) : MutableString.Create(result)));
}
[RubyMethod("basename", RubyMethodAttributes.PublicSingleton)]
public static MutableString/*!*/ Basename(CodeContext/*!*/ context, object/*!*/
self, [NotNull]MutableString/*!*/ path) {
? ? return Basename(context, self, path, MutableString.Empty);
}
[RubyMethod("basename", RubyMethodAttributes.PublicSingleton)]
public static MutableString/*!*/ Basename(CodeContext/*!*/ context, object/*!*/
self, object path, object extension) {
? ? return Basename(context, self, Protocols.CastToString(context, path),
Protocols.CastToString(context, extension));
}
[RubyMethod("basename", RubyMethodAttributes.PublicSingleton)]
public static MutableString/*!*/ Basename(CodeContext/*!*/ context, object/*!*/
self, object path) {
? ? return Basename(context, self, Protocols.CastToString(context, path));
}
> Also, in the basename_spec the test setup has wrong parameter on
> File.open(@name, ''w+''), if I change it to
''w'', the test runs fine
> otherwise none of the test works.
This is correct. There is a bug in how we map the semantics of
''w+'' to .NET IO. It''s fixed in my shelveset.
Thanks,
-John
_______________________________________________
Ironruby-core mailing list
Ironruby-core at rubyforge.org
http://rubyforge.org/mailman/listinfo/ironruby-core
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://rubyforge.org/pipermail/ironruby-core/attachments/20080511/4f00722c/attachment-0001.html>