<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv=Content-Type content="text/html; charset=us-ascii">
<meta name=Generator content="Microsoft Word 12 (filtered medium)">
<style>
<!--
/* Font Definitions */
@font-face
{font-family:Wingdings;
panose-1:5 0 0 0 0 0 0 0 0 0;}
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:Tahoma;
panose-1:2 11 6 4 3 5 4 4 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman","serif";}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
p
{mso-style-priority:99;
mso-margin-top-alt:auto;
margin-right:0in;
mso-margin-bottom-alt:auto;
margin-left:0in;
font-size:12.0pt;
font-family:"Times New Roman","serif";}
p.MsoListParagraph, li.MsoListParagraph, div.MsoListParagraph
{mso-style-priority:34;
margin-top:0in;
margin-right:0in;
margin-bottom:0in;
margin-left:.5in;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman","serif";}
span.EmailStyle18
{mso-style-type:personal-reply;
font-family:"Calibri","sans-serif";
color:#1F497D;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@page Section1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.Section1
{page:Section1;}
/* List Definitions */
@list l0
{mso-list-id:1968776566;
mso-list-type:hybrid;
mso-list-template-ids:-1848463048 134807553 134807555 134807557 134807553 134807555 134807557 134807553 134807555 134807557;}
@list l0:level1
{mso-level-number-format:bullet;
mso-level-text:\F0B7;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;
font-family:Symbol;}
@list l0:level2
{mso-level-number-format:bullet;
mso-level-text:o;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;
font-family:"Courier New";}
ol
{margin-bottom:0in;}
ul
{margin-bottom:0in;}
-->
</style>
<!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang=EN-GB link=blue vlink=purple>
<div class=Section1>
<p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'>Hi Unnikrishnan,<o:p></o:p></span></p>
<p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'>Just a couple of points you might find useful:<o:p></o:p></span></p>
<p class=MsoListParagraph style='text-indent:-.25in;mso-list:l0 level1 lfo1'><![if !supportLists]><span
style='font-size:11.0pt;font-family:Symbol;color:#1F497D'><span
style='mso-list:Ignore'>·<span style='font:7.0pt "Times New Roman"'>
</span></span></span><![endif]><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'>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.<o:p></o:p></span></p>
<p class=MsoListParagraph style='text-indent:-.25in;mso-list:l0 level1 lfo1'><![if !supportLists]><span
style='font-size:11.0pt;font-family:Symbol;color:#1F497D'><span
style='mso-list:Ignore'>·<span style='font:7.0pt "Times New Roman"'>
</span></span></span><![endif]><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'>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.<o:p></o:p></span></p>
<p class=MsoListParagraph style='text-indent:-.25in;mso-list:l0 level1 lfo1'><![if !supportLists]><span
style='font-size:11.0pt;font-family:Symbol;color:#1F497D'><span
style='mso-list:Ignore'>·<span style='font:7.0pt "Times New Roman"'>
</span></span></span><![endif]><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'>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.<o:p></o:p></span></p>
<p class=MsoListParagraph style='text-indent:-.25in;mso-list:l0 level1 lfo1'><![if !supportLists]><span
style='font-size:11.0pt;font-family:Symbol;color:#1F497D'><span
style='mso-list:Ignore'>·<span style='font:7.0pt "Times New Roman"'>
</span></span></span><![endif]><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'>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:<br>
private static readonly MutableString dotStar = MutableString.Create(".*");
and<br>
private static readonly MutableString empty = MutableString.CreateEmpty();<br>
<br>
<o:p></o:p></span></p>
<p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'>Hope that helps.<o:p></o:p></span></p>
<p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'>Pete<o:p></o:p></span></p>
<p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";
color:#1F497D'><o:p> </o:p></span></p>
<div>
<div style='border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in'>
<p class=MsoNormal><b><span lang=EN-US style='font-size:10.0pt;font-family:
"Tahoma","sans-serif"'>From:</span></b><span lang=EN-US style='font-size:10.0pt;
font-family:"Tahoma","sans-serif"'> ironruby-core-bounces@rubyforge.org
[mailto:ironruby-core-bounces@rubyforge.org] <b>On Behalf Of </b>Unnikrishnan
Nair<br>
<b>Sent:</b> Saturday,10 May 10, 2008 22:59<br>
<b>To:</b> ironruby-core@rubyforge.org<br>
<b>Subject:</b> Re: [Ironruby-core] RubyForge bug fixes<o:p></o:p></span></p>
</div>
</div>
<p class=MsoNormal><o:p> </o:p></p>
<div>
<p>Hi John,<o:p></o:p></p>
<p> <o:p></o:p></p>
<p><span style='font-size:10.0pt'>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</span><o:p></o:p></p>
<p> <o:p></o:p></p>
<p><span style='font-size:10.0pt'>[<span style='color:#2B91AF'>RubyMethod</span>(<span
style='color:#A31515'>"basename"</span>, <span style='color:#2B91AF'>RubyMethodAttributes</span>.PublicSingleton)]<o:p></o:p></span></p>
<p><span style='font-size:10.0pt;color:blue'>public</span><span
style='font-size:10.0pt'> <span style='color:blue'>static</span> <span
style='color:#2B91AF'>MutableString</span> Basename(<span style='color:#2B91AF'>CodeContext</span><span
style='color:green'>/*!*/</span> context, <span style='color:blue'>object</span><span
style='color:green'>/*!*/</span> self, [<span style='color:#2B91AF'>NotNull</span>]<span
style='color:#2B91AF'>MutableString</span><span style='color:green'>/*!*/</span>
path)<o:p></o:p></span></p>
<p><span style='font-size:10.0pt'>{<o:p></o:p></span></p>
<p><span style='font-size:10.0pt;color:#2B91AF'>MutableString</span><span
style='font-size:10.0pt'>[] tokens = path.Split(<span style='color:#A31515'>@"\/"</span>.ToCharArray(),
<span style='color:#2B91AF'>Int32</span>.MaxValue, <span style='color:#2B91AF'>StringSplitOptions</span>.RemoveEmptyEntries);<o:p></o:p></span></p>
<p><span style='font-size:10.0pt;color:#2B91AF'>MutableString</span><span
style='font-size:10.0pt'> returnString = (tokens.GetLength(0) > 0) ?
tokens[tokens.GetLength(0) - 1] : path; <o:p></o:p></span></p>
<p><span style='font-size:10.0pt;color:blue'>if</span><span style='font-size:
10.0pt'> ((tokens.GetLength(0) > 0) &&
(returnString.GetChar(returnString.Length - 1).Equals(<span style='color:#A31515'>':'</span>)))<o:p></o:p></span></p>
<p><span style='font-size:10.0pt'>returnString = path;<o:p></o:p></span></p>
<p><span style='font-size:10.0pt;color:blue'>return</span><span
style='font-size:10.0pt'> returnString;<o:p></o:p></span></p>
<p><span style='font-size:10.0pt'>}<o:p></o:p></span></p>
<p><span style='font-size:10.0pt'>[<span style='color:#2B91AF'>RubyMethod</span>(<span
style='color:#A31515'>"basename"</span>, <span style='color:#2B91AF'>RubyMethodAttributes</span>.PublicSingleton)]<o:p></o:p></span></p>
<p><span style='font-size:10.0pt;color:blue'>public</span><span
style='font-size:10.0pt'> <span style='color:blue'>static</span> <span
style='color:#2B91AF'>MutableString</span> Basename(<span style='color:#2B91AF'>CodeContext</span><span
style='color:green'>/*!*/</span> context, <span style='color:blue'>object</span><span
style='color:green'>/*!*/</span> self, <span style='color:blue'>object</span><span
style='color:green'>/*!*/</span> path)<o:p></o:p></span></p>
<p><span style='font-size:10.0pt'>{<o:p></o:p></span></p>
<p><span style='font-size:10.0pt;color:blue'>return</span><span
style='font-size:10.0pt'> Basename(context, self, <span style='color:#2B91AF'>Protocols</span>.CastToString(context,
path));<o:p></o:p></span></p>
<p><span style='font-size:10.0pt'>}<o:p></o:p></span></p>
<p><span style='font-size:10.0pt'>[<span style='color:#2B91AF'>RubyMethod</span>(<span
style='color:#A31515'>"basename"</span>, <span style='color:#2B91AF'>RubyMethodAttributes</span>.PublicSingleton)]<o:p></o:p></span></p>
<p><span style='font-size:10.0pt;color:blue'>public</span><span
style='font-size:10.0pt'> <span style='color:blue'>static</span> <span
style='color:#2B91AF'>MutableString</span> Basename(<span style='color:#2B91AF'>CodeContext</span><span
style='color:green'>/*!*/</span> context, <span style='color:blue'>object</span><span
style='color:green'>/*!*/</span> self, [<span style='color:#2B91AF'>NotNull</span>]<span
style='color:#2B91AF'>MutableString</span><span style='color:green'>/*!*/</span>
path, [<span style='color:#2B91AF'>NotNull</span>]<span style='color:#2B91AF'>MutableString</span><span
style='color:green'>/*!*/</span> filter)<o:p></o:p></span></p>
<p><span style='font-size:10.0pt'>{<o:p></o:p></span></p>
<p><span style='font-size:10.0pt;color:#2B91AF'>MutableString</span><span
style='font-size:10.0pt'> filename = Basename(context, self, path);<o:p></o:p></span></p>
<p><span style='font-size:10.0pt;color:green'>//Treat ? specially<o:p></o:p></span></p>
<p><span style='font-size:10.0pt;color:blue'>if</span><span style='font-size:
10.0pt'> (filter.IndexOf(<span style='color:#A31515'>'?'</span>) >= 0)<o:p></o:p></span></p>
<p><span style='font-size:10.0pt;color:blue'>return</span><span
style='font-size:10.0pt'> filename;<o:p></o:p></span></p>
<p><span style='font-size:10.0pt;color:green'>//Treat .* and exetions specially
using regex<o:p></o:p></span></p>
<p><span style='font-size:10.0pt;color:blue'>if</span><span style='font-size:
10.0pt'> (filter.Equals(<span style='color:#2B91AF'>MutableString</span>.Create(<span
style='color:#A31515'>".*"</span>)))<o:p></o:p></span></p>
<p><span style='font-size:10.0pt'>{<o:p></o:p></span></p>
<p><span style='font-size:10.0pt'>filter.Clear();<o:p></o:p></span></p>
<p><span style='font-size:10.0pt'>filter.Append(<span style='color:#A31515'>@"(?x)(\.[^.]*$|$)"</span>);<o:p></o:p></span></p>
<p><span style='font-size:10.0pt'>}<o:p></o:p></span></p>
<p><span style='font-size:10.0pt;color:blue'>else<o:p></o:p></span></p>
<p><span style='font-size:10.0pt'>{<o:p></o:p></span></p>
<p><span style='font-size:10.0pt'>filter.Append(<span style='color:#A31515'>"$"</span>);<o:p></o:p></span></p>
<p><span style='font-size:10.0pt'>}<o:p></o:p></span></p>
<p><span style='font-size:10.0pt;color:#2B91AF'>RubyRegex</span><span
style='font-size:10.0pt'> reg = <span style='color:blue'>new</span> <span
style='color:#2B91AF'>RubyRegex</span>(filter, System.Text.RegularExpressions.<span
style='color:#2B91AF'>RegexOptions</span>.Singleline);<o:p></o:p></span></p>
<p><span style='font-size:10.0pt'>System.Text.RegularExpressions.<span
style='color:#2B91AF'>Match</span> mat = reg.Match(filename);<o:p></o:p></span></p>
<p><span style='font-size:10.0pt;color:blue'>if</span><span style='font-size:
10.0pt'> (mat.Success)<o:p></o:p></span></p>
<p><span style='font-size:10.0pt'>filename.Replace(mat.Index, mat.Length, <span
style='color:#2B91AF'>MutableString</span>.Create(<span style='color:#A31515'>""</span>));<o:p></o:p></span></p>
<p><span style='font-size:10.0pt;color:blue'>return</span><span
style='font-size:10.0pt'> filename;<o:p></o:p></span></p>
<p><span style='font-size:10.0pt'>}<o:p></o:p></span></p>
<p><span style='font-size:10.0pt'>[<span style='color:#2B91AF'>RubyMethod</span>(<span
style='color:#A31515'>"basename"</span>, <span style='color:#2B91AF'>RubyMethodAttributes</span>.PublicSingleton)]<o:p></o:p></span></p>
<p><span style='font-size:10.0pt;color:blue'>public</span><span
style='font-size:10.0pt'> <span style='color:blue'>static</span> <span
style='color:#2B91AF'>MutableString</span> Basename(<span style='color:#2B91AF'>CodeContext</span><span
style='color:green'>/*!*/</span> context, <span style='color:blue'>object</span><span
style='color:green'>/*!*/</span> self, <span style='color:blue'>object</span><span
style='color:green'>/*!*/</span> path, <span style='color:blue'>object</span><span
style='color:green'>/*!*/</span> filter)<o:p></o:p></span></p>
<p><span style='font-size:10.0pt'>{<o:p></o:p></span></p>
<p><span style='font-size:10.0pt;color:blue'>return</span><span
style='font-size:10.0pt'> Basename(context, self, <span style='color:#2B91AF'>Protocols</span>.CastToString(context,
path), <span style='color:#2B91AF'>Protocols</span>.CastToString(context,
filter));<o:p></o:p></span></p>
<p><span style='font-size:10.0pt'>}<o:p></o:p></span></p>
<p><span style='font-size:10.0pt'> <o:p></o:p></span></p>
<p><span style='font-size:10.0pt'>Let me know what do you think? Yes, I found
some problems with test spec as well, (bar.txt should be baz)<o:p></o:p></span></p>
<p><span style='font-size:10.0pt'> <o:p></o:p></span></p>
<p><span style='font-size:10.0pt'>Thanks.<o:p></o:p></span></p>
<p><span style='font-size:10.0pt'> <o:p></o:p></span></p>
<div>
<p class=MsoNormal style='margin-bottom:12.0pt'><o:p> </o:p></p>
<div>
<p class=MsoNormal>----- Original Message ----<br>
From: John Lam (IRONRUBY) <jflam@microsoft.com><br>
To: "ironruby-core@rubyforge.org" <ironruby-core@rubyforge.org><br>
Sent: Saturday, May 10, 2008 3:25:16 PM<br>
Subject: Re: [Ironruby-core] RubyForge bug fixes<br>
<br>
Unnikrishnan Nair:<br>
<br>
> Quick question, are these following assertions are correct?<br>
><br>
> should_raise(TypeError){ File.basename(1) }<br>
> should_raise(TypeError){ File.basename("bar.txt",
1) }<br>
> should_raise(TypeError){ File.basename(true) }<br>
<br>
Yes they are. You can easily verify this in MRI yourself.<br>
<br>
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().<br>
<br>
In the second case, it also hits an overload that accepts a nullable object as
its second parameter, which raises TypeError via Protocols.CastToString().<br>
<br>
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.<br>
<br>
[RubyMethod("basename", RubyMethodAttributes.PublicSingleton)]<br>
public static MutableString/*!*/ Basename(CodeContext/*!*/ context, object/*!*/
self, [NotNull]MutableString/*!*/ path, [NotNull]MutableString/*!*/
extensionFilter) {<br>
if (path.Length == 0)<br>
return path;<br>
<br>
MutableString trimmedPath = TrimTrailingSlashes(path);<br>
<br>
// Special cases of drive letters C:\\ or C:/<br>
if (trimmedPath.Length == 2)<br>
if (Char.IsLetter(trimmedPath.GetChar(0))
&& trimmedPath.GetChar(1) == ':')<br>
return Kernel.FlowTaint(context,
path, (path.Length > 2 ? MutableString.Create(path.GetChar(2).ToString()) :
MutableString.Create(String.Empty)));<br>
<br>
string trimmedPathAsString = trimmedPath.ConvertToString();<br>
if (trimmedPathAsString == "/")<br>
return trimmedPath;<br>
<br>
string filename = <a href="http://system.io.path.ge/"
target="_blank">System.IO</a>.Path.GetFileName(trimmedPath.ConvertToString());<br>
<br>
// Handle UNC host names correctly<br>
string root = <a href="http://system.io.path.ge/" target="_blank">System.IO</a>.Path.GetPathRoot(trimmedPath.ConvertToString());<br>
if (extensionFilter.Length == 0)<br>
return trimmedPathAsString == root ? MutableString.Create(root)
: MutableString.Create(filename);<br>
<br>
string fileExtension = <a href="http://system.io.path.ge/"
target="_blank">System.IO</a>.Path.GetExtension(filename);<br>
string basename = <a href="http://system.io.path.ge/"
target="_blank">System.IO</a>.Path.GetFileNameWithoutExtension(filename);<br>
<br>
string result = WildcardExtensionMatch(fileExtension,
extensionFilter.ConvertToString()) ? basename : filename;<br>
return Kernel.FlowTaint(context, self, (result.Equals(root) ?
MutableString.Create(root) : MutableString.Create(result)));<br>
}<br>
<br>
[RubyMethod("basename", RubyMethodAttributes.PublicSingleton)]<br>
public static MutableString/*!*/ Basename(CodeContext/*!*/ context, object/*!*/
self, [NotNull]MutableString/*!*/ path) {<br>
return Basename(context, self, path, MutableString.Empty);<br>
}<br>
<br>
[RubyMethod("basename", RubyMethodAttributes.PublicSingleton)]<br>
public static MutableString/*!*/ Basename(CodeContext/*!*/ context, object/*!*/
self, object path, object extension) {<br>
return Basename(context, self, Protocols.CastToString(context,
path), Protocols.CastToString(context, extension));<br>
}<br>
<br>
[RubyMethod("basename", RubyMethodAttributes.PublicSingleton)]<br>
public static MutableString/*!*/ Basename(CodeContext/*!*/ context, object/*!*/
self, object path) {<br>
return Basename(context, self, Protocols.CastToString(context,
path));<br>
}<br>
<br>
> Also, in the basename_spec the test setup has wrong parameter on<br>
> File.open(@name, 'w+'), if I change it to 'w', the test runs fine<br>
> otherwise none of the test works.<br>
<br>
This is correct. There is a bug in how we map the semantics of 'w+' to .NET IO.
It's fixed in my shelveset.<br>
<br>
Thanks,<br>
-John<br>
<br>
<br>
<br>
_______________________________________________<br>
Ironruby-core mailing list<br>
<a href="mailto:Ironruby-core@rubyforge.org">Ironruby-core@rubyforge.org</a><br>
<a href="http://rubyforge.org/mailman/listinfo/ironruby-core" target="_blank">http://rubyforge.org/mailman/listinfo/ironruby-core</a><o:p></o:p></p>
</div>
</div>
</div>
</div>
</body>
</html>