search

dry-rb / dry-container

A simple, configurable object container implemented in Ruby
https://dry-rb.org/gems/dry-container
MIT License
335 stars 41 forks source link

Use #parameters instead of #arity #18

Closed flash-gordon closed 8 years ago

AMHOL commented 8 years ago

Thanks @flash-gordon, not sure why this is needed as the arity check was there to check whether the block/proc argument takes no arguments?

Also the build is broken on jruby, not sure what the problems is though

flash-gordon commented 8 years ago

damn! I tested every mri since 2.0 manually. I guess jruby 1.7 doesn't support keywords in block, I'll figure that out.

When you pass a block to a method it is converted to a proc (not lambda) object that has some weird behavior with #arity:

2.3.0 :001 > (-> * {}).arity
 => -1
2.3.0 :002 > (-> ** {}).arity
 => -1
2.3.0 :003 > (proc { |*| }).arity
 => -1
2.3.0 :004 > (proc { |**| }).arity
 => 0

and

2.3.0 :001 > def foo(&block)
2.3.0 :002?>   block
2.3.0 :003?>   end
 => :foo
2.3.0 :004 > (foo { |**| }).arity
 => 0

I use keyword arguments quite extensively with dry-transaction and it works fine except this case because dry-container tries to call the block prematurely.

solnic commented 8 years ago

We don't support jruby 1.7

On Sat, Apr 30, 2016 at 3:51 PM Nikita Shilnikov notifications@github.com wrote:

damn! I tested every mri since 2.0 manually. I guess jruby 1.7 doesn't support keywords in block, I'll figure that out.

When you pass a block to a method it is converted to a proc (not lambda) object that has some weird behavior with #arity:

2.3.0 :001 > (-> * {}).arity => -12.3.0 :002 > (-> * {}).arity => -12.3.0 :003 > (proc { || }).arity => -12.3.0 :004 > (proc { || }).arity => 0

and

2.3.0 :001 > def foo(&block)2.3.0 :002?> block2.3.0 :003?> end => :foo2.3.0 :004 > (foo { |**| }).arity => 0

I use keyword arguments quite extensively with dry-transaction and it works fine except this case because dry-container tries to call the block prematurely.

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub https://github.com/dry-rb/dry-container/pull/18#issuecomment-215964378

flash-gordon commented 8 years ago

@solnic OK, I updated travis config, we'll see what happens

flash-gordon commented 8 years ago

Doesn't work either :-/ Going to take a look later today

flash-gordon commented 8 years ago

It works on my machine with jruby-9000 and fails on CI because travis installs jruby-9000.pre1 instead of released version due to unfixed rvm issue. Travis has workaround for that but it slows the build down (compare this and that).

flash-gordon commented 8 years ago

@AMHOL your turn :)

flash-gordon commented 8 years ago

btw I used workaround from here

AMHOL commented 8 years ago

Sorry @flash-gordon updated to jruby-9000 in travis.yml, I missed that the first time, specs are failing again but they actually run

flash-gordon commented 8 years ago

@AMHOL I tried that. Now the build uses jruby-9000-pre1 again :)

$ java -Xmx32m -version
java version "1.7.0_76"
Java(TM) SE Runtime Environment (build 1.7.0_76-b13)
Java HotSpot(TM) 64-Bit Server VM (build 24.76-b04, mixed mode)
$ javac -J-Xmx32m -version
javac 1.7.0_76
$ ruby --version
ruby --version
jruby 9.0.0.0.pre1 (2.2.0p0) 2015-01-20 d537cab Java HotSpot(TM) 64-Bit Server VM 24.76-b04 on 1.7.0_76-b13 [linux-amd64]

The only way is to update rvm & install jruby manually.

AMHOL commented 8 years ago

That sucks, removed my commit

flash-gordon commented 8 years ago

Yep, it does. IIUIC travis doesn't update rvm for god-knows-why reason.