Re: RubyForge bug fixes

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 D. [email protected]
To: [email protected]
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:[email protected]
[mailto:[email protected]] On Behalf Of Unnikrishnan
Nair
Sent: Saturday,10 May 10, 2008 22:59
To: [email protected]
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 L. (IRONRUBY) [email protected]
To: “[email protected][email protected]
Sent: Saturday, May 10, 2008 3:25:16 PM
Subject: Re: [Ironruby-core] RubyForge bug fixes

Unnikrishnan N.:

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

This forum is not affiliated to the Ruby language, Ruby on Rails framework, nor any Ruby applications discussed here.

| Privacy Policy | Terms of Service | Remote Ruby Jobs