Forum: IronRuby File.new spec fixes

Announcement (2017-05-07): www.ruby-forum.com is now read-only since I unfortunately do not have the time to support and maintain the forum any more. Please see rubyonrails.org/community and ruby-lang.org/en/community for other Rails- und Ruby-related community platforms.
Jirapong N. (Guest)
on 2009-04-17 22:43
(Received via mailing list)
http://github.com/Jirapong/ironruby/commit/929e07e...

Fixes for core\file\new_spec.rb:
File.new raises an Errorno::EEXIST if the file exists when create a
new file with File::CREAT|File::EXCL
File.new raises an Errno::EINVAL error with File::APPEND
File.new raises an Errno::EINVAL error with File::RDONLY|File::APPEND

Files changed:
  • Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/ironruby-
tags/core/file/new_tags.txt
  • Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/rubyspec/
core/file/new_spec.rb
  • Merlin/Main/Languages/Ruby/Libraries.LCA_RESTRICTED/Builtins/
FileOps.cs
  • Merlin/Main/Languages/Ruby/Ruby/Builtins/File.cs

Thank you,
-Jirapong
Shri B. (Guest)
on 2009-04-18 02:56
(Received via mailing list)
Some blocks are not indented as in the second line in this example.
Could you indent those please?
+    lambda {
+    @fh = File.new(@file, File::CREAT|File::EXCL)
+  }.should raise_error(Errno::EEXIST)
0

You added try-catch to one of the overloads of RubyFileOps.CreateFile.
Could you add it to all? Btw, I discussed with Tomas about the fact that
many of the IO exceptions are defined in IronRuby.Libraries.dll, but
RubyFile needs to throw those exception from IronRuby.dll. We can move
the exceptions that are needed in IronRuby.dll into IronRuby.dll. That
will avoid having to do a try-catch to translate the exception type.  Do
you want to add Languages\Ruby\Ruby\Builtins\Errno.cs and move some of
the exception types there?

Thanks,
Shri

From: removed_email_address@domain.invalid
[mailto:removed_email_address@domain.invalid] On Behalf Of
removed_email_address@domain.invalid
Sent: Friday, April 17, 2009 11:40 AM
To: removed_email_address@domain.invalid
Subject: [Ironruby-core] File.new spec fixes

http://github.com/Jirapong/ironruby/commit/929e07e...

Fixes for core\file\new_spec.rb:

 *   File.new raises an Errorno::EEXIST if the file exists when create a
new file with File::CREAT|File::EXCL

 *   File.new raises an Errno::EINVAL error with File::APPEND
 *   File.new raises an Errno::EINVAL error with
File::RDONLY|File::APPEND

Files changed:
            *
Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/ironruby-tags/core/file/new_tags.txt
            *
Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/rubyspec/core/file/new_spec.rb
            *
Merlin/Main/Languages/Ruby/Libraries.LCA_RESTRICTED/Builtins/FileOps.cs
            * Merlin/Main/Languages/Ruby/Ruby/Builtins/File.cs

Thank you,
-Jirapong
Jirapong N. (Guest)
on 2009-04-18 10:25
(Received via mailing list)
Thank you for your review.  please find formatted version at 9942b24

I just do a pull request for 929e07 and 9942b24.

please see my answer inline.

Thank you,
-Jirapong

On Apr 18, 2009, at 5:51 AM, Shri B. wrote:

> Some blocks are not indented as in the second line in this example.
> Could you indent those please?
> +    lambda {
> +    @fh = File.new(@file, File::CREAT|File::EXCL)
> +  }.should raise_error(Errno::EEXIST)
> 0
>
> You added try-catch to one of the overloads of
> RubyFileOps.CreateFile. Could you add it to all?
Yes, It must be, but this will not longer need if we move Errno to
Ruby project.

> Btw, I discussed with Tomas about the fact that many of the IO
> exceptions are defined in IronRuby.Libraries.dll, but RubyFile needs
> to throw those exception from IronRuby.dll. We can move the
> exceptions that are needed in IronRuby.dll into IronRuby.dll. That
> will avoid having to do a try-catch to translate the exception
> type.  Do you want to add Languages\Ruby\Ruby\Builtins\Errno.cs and
> move some of the exception types there?
I'm trying to add Errno.cs with EEXIST exception in IronRuby.dll, but
I can't get its initialize code in Initializers.Generated.cs file
after run geninit.  Is it possible to define [RubyClass("EEXIST")] in
IronRuby.dll?
Shri B. (Guest)
on 2009-04-18 21:50
(Received via mailing list)
You  will need to add the exception classes in IronRuby.dll, and then
define XXXOps classes in IronRuby.Libraries.dll with the [RubyClass]
attribute. See FileNotFoundExceptionOps in
Libraries.LCA_RESTRICTED\Builtins\Errno.cs for an example of how this is
done.

From: removed_email_address@domain.invalid
[mailto:removed_email_address@domain.invalid] On Behalf Of
removed_email_address@domain.invalid
Sent: Friday, April 17, 2009 11:25 PM
To: removed_email_address@domain.invalid
Subject: Re: [Ironruby-core] File.new spec fixes

Thank you for your review.  please find formatted version at
9942b24<http://github.com/Jirapong/ironruby/commit/9942b24...

I just do a pull request for 929e07 and 9942b24.

please see my answer inline.

Thank you,
-Jirapong

On Apr 18, 2009, at 5:51 AM, Shri B. wrote:


Some blocks are not indented as in the second line in this example.
Could you indent those please?
+    lambda {
+    @fh = File.new(@file, File::CREAT|File::EXCL)
+  }.should raise_error(Errno::EEXIST)
0

You added try-catch to one of the overloads of RubyFileOps.CreateFile.
Could you add it to all?
Yes, It must be, but this will not longer need if we move Errno to Ruby
project.


Btw, I discussed with Tomas about the fact that many of the IO
exceptions are defined in IronRuby.Libraries.dll, but RubyFile needs to
throw those exception from IronRuby.dll. We can move the exceptions that
are needed in IronRuby.dll into IronRuby.dll. That will avoid having to
do a try-catch to translate the exception type.  Do you want to add
Languages\Ruby\Ruby\Builtins\Errno.cs and move some of the exception
types there?
I'm trying to add Errno.cs with EEXIST exception in IronRuby.dll, but I
can't get its initialize code in Initializers.Generated.cs file after
run geninit.  Is it possible to define [RubyClass("EEXIST")] in
IronRuby.dll?



Thanks,
Shri

From:
removed_email_address@domain.invalid<mailto:removed_email_address@domain.invalid>
[mailto:removed_email_address@domain.invalid] On Behalf Of
removed_email_address@domain.invalid<mailto:removed_email_address@domain.invalid>
Sent: Friday, April 17, 2009 11:40 AM
To: 
removed_email_address@domain.invalid<mailto:removed_email_address@domain.invalid>
Subject: [Ironruby-core] File.new spec fixes

http://github.com/Jirapong/ironruby/commit/929e07e...

Fixes for core\file\new_spec.rb:

 *   File.new raises an Errorno::EEXIST if the file exists when create a
new file with File::CREAT|File::EXCL

 *   File.new raises an Errno::EINVAL error with File::APPEND
 *   File.new raises an Errno::EINVAL error with
File::RDONLY|File::APPEND

Files changed:
            *
Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/ironruby-tags/core/file/new_tags.txt
            *
Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/rubyspec/core/file/new_spec.rb
            *
Merlin/Main/Languages/Ruby/Libraries.LCA_RESTRICTED/Builtins/FileOps.cs
            * Merlin/Main/Languages/Ruby/Ruby/Builtins/File.cs

Thank you,
-Jirapong
Jirapong N. (Guest)
on 2009-04-19 00:35
(Received via mailing list)
Hi Shri,
  Thank you for quick reply.  Could you please review it at af4acea ?

Note: VS.NET ProductVersion seems to be difference on my machine.
Thank you,
-Jirapong
Shri B. (Guest)
on 2009-04-19 01:43
(Received via mailing list)
Looks good! The only comment is that in IronRuby.dll (an in the
libraries too wherever possible), the code should be "good" C# code, and
should have as few Ruby-isms in it as possible. In RubyErrno.cs, you
added a static class IronRuby.Builtins.RubyErrno with inner exception
classes like ExistError. You would normally not declare exceptions as
inner classes, and so you should not do that here either. Instead, just
keep ExistError etc in the IronRuby.Builtins namespace (just like
SystemExit, etc in Ruby\Builtins\Exceptions.cs) or use a namespace like
"IronRuby.Builtins.RubyErrno" if you want. But using a static class
serves no useful purpose and so should be avoided.

Ofcourse, in Libraries.LCA_RESTRICTED\Builtins\Errno.cs, you will need
to use inner classes to match the Ruby class hierarchy, but that is a
different matter...

I see that you removed "if (path.Empty)" from FileOps.cs and moved it to
File.cs, which is great! Couldn't you do the same in KernelOps too? You
changed how KernelOps throws the exception, but the whole "if
(path.IsEmpty)" could be removed...

From: removed_email_address@domain.invalid
[mailto:removed_email_address@domain.invalid] On Behalf Of
removed_email_address@domain.invalid
Sent: Saturday, April 18, 2009 1:35 PM
To: removed_email_address@domain.invalid
Subject: Re: [Ironruby-core] File.new spec fixes

Hi Shri,
            Thank you for quick reply.  Could you please review it at
af4acea<http://github.com/Jirapong/ironruby/commit/af4acea...
?

Note: VS.NET ProductVersion seems to be difference on my machine.
Thank you,
-Jirapong

On Apr 19, 2009, at 12:48 AM, Shri B. wrote:


You  will need to add the exception classes in IronRuby.dll, and then
define XXXOps classes in IronRuby.Libraries.dll with the [RubyClass]
attribute. See FileNotFoundExceptionOps in
Libraries.LCA_RESTRICTED\Builtins\Errno.cs for an example of how this is
done.

From:
removed_email_address@domain.invalid<mailto:removed_email_address@domain.invalid>
[mailto:removed_email_address@domain.invalid] On Behalf Of
removed_email_address@domain.invalid<mailto:removed_email_address@domain.invalid>
Sent: Friday, April 17, 2009 11:25 PM
To: 
removed_email_address@domain.invalid<mailto:removed_email_address@domain.invalid>
Subject: Re: [Ironruby-core] File.new spec fixes

Thank you for your review.  please find formatted version at
9942b24<http://github.com/Jirapong/ironruby/commit/9942b24...

I just do a pull request for 929e07 and 9942b24.

please see my answer inline.

Thank you,
-Jirapong

On Apr 18, 2009, at 5:51 AM, Shri B. wrote:



Some blocks are not indented as in the second line in this example.
Could you indent those please?
+    lambda {
+    @fh = File.new(@file, File::CREAT|File::EXCL)
+  }.should raise_error(Errno::EEXIST)
0

You added try-catch to one of the overloads of RubyFileOps.CreateFile.
Could you add it to all?
Yes, It must be, but this will not longer need if we move Errno to Ruby
project.



Btw, I discussed with Tomas about the fact that many of the IO
exceptions are defined in IronRuby.Libraries.dll, but RubyFile needs to
throw those exception from IronRuby.dll. We can move the exceptions that
are needed in IronRuby.dll into IronRuby.dll. That will avoid having to
do a try-catch to translate the exception type.  Do you want to add
Languages\Ruby\Ruby\Builtins\Errno.cs and move some of the exception
types there?
I'm trying to add Errno.cs with EEXIST exception in IronRuby.dll, but I
can't get its initialize code in Initializers.Generated.cs file after
run geninit.  Is it possible to define [RubyClass("EEXIST")] in
IronRuby.dll?




Thanks,
Shri

From:
removed_email_address@domain.invalid<mailto:removed_email_address@domain.invalid>
[mailto:removed_email_address@domain.invalid] On Behalf Of
removed_email_address@domain.invalid<mailto:removed_email_address@domain.invalid>
Sent: Friday, April 17, 2009 11:40 AM
To: 
removed_email_address@domain.invalid<mailto:removed_email_address@domain.invalid>
Subject: [Ironruby-core] File.new spec fixes

http://github.com/Jirapong/ironruby/commit/929e07e...

Fixes for core\file\new_spec.rb:

 *   File.new raises an Errorno::EEXIST if the file exists when create a
new file with File::CREAT|File::EXCL

 *   File.new raises an Errno::EINVAL error with File::APPEND
 *   File.new raises an Errno::EINVAL error with
File::RDONLY|File::APPEND

Files changed:
            *
Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/ironruby-tags/core/file/new_tags.txt
            *
Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/rubyspec/core/file/new_spec.rb
            *
Merlin/Main/Languages/Ruby/Libraries.LCA_RESTRICTED/Builtins/FileOps.cs
            * Merlin/Main/Languages/Ruby/Ruby/Builtins/File.cs

Thank you,
-Jirapong

_______________________________________________
Ironruby-core mailing list
removed_email_address@domain.invalid<mailto:removed_email_address@domain.invalid>
http://rubyforge.org/mailman/listinfo/ironruby-core
Jirapong N. (Guest)
on 2009-04-21 03:19
(Received via mailing list)
http://github.com/Jirapong/ironruby/commit/e5dfbef...

Makes Code refactor as reviewed.  see inline answer.

Files changed:
  • Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/rubyspec/
core/kernel/open_spec.rb
  •
Merlin/Main/Languages/Ruby/Libraries.LCA_RESTRICTED/Builtins/Errno.cs
  • Merlin/Main/Languages/Ruby/Libraries.LCA_RESTRICTED/Builtins/
KernelOps.cs
  • Merlin/Main/Languages/Ruby/Libraries.LCA_RESTRICTED/
Initializers.Generated.cs
  • Merlin/Main/Languages/Ruby/Ruby/Builtins/Exceptions.cs
  • Merlin/Main/Languages/Ruby/Ruby/Builtins/RubyErrno.cs
Thanks,
-Jirapong

On Apr 19, 2009, at 4:43 AM, Shri B. wrote:

> avoided.
I have move Errno's exceptions to exceptions.cs and makes RubyErrno.cs
to be only a helper class like IronRuby.Runtime.RubyExceptions class. -
JN

>
> Ofcourse, in Libraries.LCA_RESTRICTED\Builtins\Errno.cs, you will
> need to use inner classes to match the Ruby class hierarchy, but
> that is a different matter…
>
> I see that you removed “if (path.Empty)” from FileOps.cs and moved
> it to File.cs, which is great! Couldn’t you do the same in KernelOps
> too? You changed how KernelOps throws the exception, but the whole
> “if (path.IsEmpty)” could be removed…
Done - JN
Shri B. (Guest)
on 2009-04-21 08:36
(Received via mailing list)
Looks great!

From: removed_email_address@domain.invalid
[mailto:removed_email_address@domain.invalid] On Behalf Of
removed_email_address@domain.invalid
Sent: Monday, April 20, 2009 4:19 PM
To: removed_email_address@domain.invalid
Subject: Re: [Ironruby-core] File.new spec fixes


http://github.com/Jirapong/ironruby/commit/e5dfbef...

Makes Code refactor as reviewed.  see inline answer.

Files changed:
            *
Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/rubyspec/core/kernel/open_spec.rb
            *
Merlin/Main/Languages/Ruby/Libraries.LCA_RESTRICTED/Builtins/Errno.cs
            *
Merlin/Main/Languages/Ruby/Libraries.LCA_RESTRICTED/Builtins/KernelOps.cs
            *
Merlin/Main/Languages/Ruby/Libraries.LCA_RESTRICTED/Initializers.Generated.cs
            * Merlin/Main/Languages/Ruby/Ruby/Builtins/Exceptions.cs
            * Merlin/Main/Languages/Ruby/Ruby/Builtins/RubyErrno.cs
Thanks,
-Jirapong

On Apr 19, 2009, at 4:43 AM, Shri B. wrote:


Looks good! The only comment is that in IronRuby.dll (an in the
libraries too wherever possible), the code should be "good" C# code, and
should have as few Ruby-isms in it as possible. In RubyErrno.cs, you
added a static class IronRuby.Builtins.RubyErrno with inner exception
classes like ExistError. You would normally not declare exceptions as
inner classes, and so you should not do that here either. Instead, just
keep ExistError etc in the IronRuby.Builtins namespace (just like
SystemExit, etc in Ruby\Builtins\Exceptions.cs) or use a namespace like
"IronRuby.Builtins.RubyErrno" if you want. But using a static class
serves no useful purpose and so should be avoided.
I have move Errno's exceptions to exceptions.cs and makes RubyErrno.cs
to be only a helper class like IronRuby.Runtime.RubyExceptions class.
-JN



Ofcourse, in Libraries.LCA_RESTRICTED\Builtins\Errno.cs, you will need
to use inner classes to match the Ruby class hierarchy, but that is a
different matter...

I see that you removed "if (path.Empty)" from FileOps.cs and moved it to
File.cs, which is great! Couldn't you do the same in KernelOps too? You
changed how KernelOps throws the exception, but the whole "if
(path.IsEmpty)" could be removed...
Done - JN



From:
removed_email_address@domain.invalid<mailto:removed_email_address@domain.invalid>
[mailto:removed_email_address@domain.invalid] On Behalf Of
removed_email_address@domain.invalid<mailto:removed_email_address@domain.invalid>
Sent: Saturday, April 18, 2009 1:35 PM
To: 
removed_email_address@domain.invalid<mailto:removed_email_address@domain.invalid>
Subject: Re: [Ironruby-core] File.new spec fixes

Hi Shri,
            Thank you for quick reply.  Could you please review it at
af4acea<http://github.com/Jirapong/ironruby/commit/af4acea...
?

Note: VS.NET ProductVersion seems to be difference on my machine.
Thank you,
-Jirapong

On Apr 19, 2009, at 12:48 AM, Shri B. wrote:



You  will need to add the exception classes in IronRuby.dll, and then
define XXXOps classes in IronRuby.Libraries.dll with the [RubyClass]
attribute. See FileNotFoundExceptionOps in
Libraries.LCA_RESTRICTED\Builtins\Errno.cs for an example of how this is
done.

From:
removed_email_address@domain.invalid<mailto:removed_email_address@domain.invalid>
[mailto:removed_email_address@domain.invalid] On Behalf Of
removed_email_address@domain.invalid<mailto:removed_email_address@domain.invalid>
Sent: Friday, April 17, 2009 11:25 PM
To: 
removed_email_address@domain.invalid<mailto:removed_email_address@domain.invalid>
Subject: Re: [Ironruby-core] File.new spec fixes

Thank you for your review.  please find formatted version at
9942b24<http://github.com/Jirapong/ironruby/commit/9942b24...

I just do a pull request for 929e07 and 9942b24.

please see my answer inline.

Thank you,
-Jirapong

On Apr 18, 2009, at 5:51 AM, Shri B. wrote:




Some blocks are not indented as in the second line in this example.
Could you indent those please?
+    lambda {
+    @fh = File.new(@file, File::CREAT|File::EXCL)
+  }.should raise_error(Errno::EEXIST)
0

You added try-catch to one of the overloads of RubyFileOps.CreateFile.
Could you add it to all?
Yes, It must be, but this will not longer need if we move Errno to Ruby
project.




Btw, I discussed with Tomas about the fact that many of the IO
exceptions are defined in IronRuby.Libraries.dll, but RubyFile needs to
throw those exception from IronRuby.dll. We can move the exceptions that
are needed in IronRuby.dll into IronRuby.dll. That will avoid having to
do a try-catch to translate the exception type.  Do you want to add
Languages\Ruby\Ruby\Builtins\Errno.cs and move some of the exception
types there?
I'm trying to add Errno.cs with EEXIST exception in IronRuby.dll, but I
can't get its initialize code in Initializers.Generated.cs file after
run geninit.  Is it possible to define [RubyClass("EEXIST")] in
IronRuby.dll?





Thanks,
Shri

From:
removed_email_address@domain.invalid<mailto:removed_email_address@domain.invalid>
[mailto:removed_email_address@domain.invalid] On Behalf Of
removed_email_address@domain.invalid<mailto:removed_email_address@domain.invalid>
Sent: Friday, April 17, 2009 11:40 AM
To: 
removed_email_address@domain.invalid<mailto:removed_email_address@domain.invalid>
Subject: [Ironruby-core] File.new spec fixes

http://github.com/Jirapong/ironruby/commit/929e07e...

Fixes for core\file\new_spec.rb:

 *   File.new raises an Errorno::EEXIST if the file exists when create a
new file with File::CREAT|File::EXCL

 *   File.new raises an Errno::EINVAL error with File::APPEND
 *   File.new raises an Errno::EINVAL error with
File::RDONLY|File::APPEND

Files changed:
            *
Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/ironruby-tags/core/file/new_tags.txt
            *
Merlin/External.LCA_RESTRICTED/Languages/IronRuby/mspec/rubyspec/core/file/new_spec.rb
            *
Merlin/Main/Languages/Ruby/Libraries.LCA_RESTRICTED/Builtins/FileOps.cs
            * Merlin/Main/Languages/Ruby/Ruby/Builtins/File.cs

Thank you,
-Jirapong

_______________________________________________
Ironruby-core mailing list
removed_email_address@domain.invalid<mailto:removed_email_address@domain.invalid>
http://rubyforge.org/mailman/listinfo/ironruby-core

_______________________________________________
Ironruby-core mailing list
removed_email_address@domain.invalid<mailto:removed_email_address@domain.invalid>
http://rubyforge.org/mailman/listinfo/ironruby-core
This topic is locked and can not be replied to.