Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Actions adore keyword arguments #174

Merged
merged 3 commits into from
Dec 2, 2016
Merged

Conversation

JoshCheek
Copy link
Contributor

Consider this task:

task :t, [:arg] do |task, arg: 'default_value'|
  emotion = "\e[32m^_^\e[0m"
  emotion = "\e[31mt.t\e[0m" if ARGV[0] =~ /t\[(.*)\]$/ && $1 != arg
  puts "task #{task.name.inspect} got #{arg.inspect} #{emotion}"
end

Run this on your machine right now and you'll see the task is in tears. But this commit turns tearful eyes into smiling ones, observe:

$ rake t
task "t" got "default_value" ^_^
$ rake t[custom_value]
task "t" got "default_value" t.t

$ ruby -Ilib exe/rake t
task "t" got "default_value" ^_^
$ ruby -Ilib exe/rake t[custom_value]
task "t" got "custom_value" ^_^

It accomplishes this by always invoking the action in the same way. Previously, Rake invoke the action with different argument structures based the arity.

case act.arity
when 1
  act.call(self)
else
  act.call(self, args)
end

I assume it's expecting one of these:

proc { |task| }.arity           # => 1
proc { |task, options| }.arity  # => 2

However this leads numerous task signatures to be invoked incorrectly. Eg, consider the case of our tearful task above, here is why it cries.

proc { |task, arg: 'default_value'| }.arity # => 1

The solution is nice and easy: always invoke the block the same way.

Why was this ever a thing at all, then? Looks like it was added in 2007 about a month after Ruby 1.8.6 was released. 925fbb8 Probably something was just different back then, eg the (difference) between lambdas and procs was whether you did prc.call or prc.yield

Consider this task:

```ruby
task :t, [:arg] do |task, arg: 'default_value'|
  emotion = "\e[32m^_^\e[0m"
  emotion = "\e[31mt.t\e[0m" if ARGV[0] =~ /t\[(.*)\]$/ && $1 != arg
  puts "task #{task.name.inspect} got #{arg.inspect} #{emotion}"
end
```

Run this on your machine right now, you'll see the task is in tears.
But this commit turns tearful eyes into smiling ones, observe:

```sh
$ rake t
task "t" got "default_value" ^_^
$ rake t[custom_value]
task "t" got "default_value" t.t

$ ruby -Ilib exe/rake t
task "t" got "default_value" ^_^
$ ruby -Ilib exe/rake t[custom_value]
task "t" got "custom_value" ^_^
```

It accomplishes this by always invoking the action in the same way.
Previously, Rake invoke the action with different argument structures based the arity
https://github.com/ruby/rake/blob/59856100815b841624269f817c815f54921bf321/lib/rake/task.rb#L251-L256

```ruby
case act.arity
when 1
  act.call(self)
else
  act.call(self, args)
end
```

I assume it's expecting one of these:

```ruby
proc { |task| }.arity           # => 1
proc { |task, options| }.arity  # => 2
```

However this leads numerous task signatures to be invoked incorrectly.
Eg, consider the case of our tearful task above, here is why it cries.

```ruby
proc { |task, arg: 'default_value'| }.arity # => 1
```

The solution is nice and easy: always invoke the block the same way.
Why was this ever a thing at all, then?
Looks like it was added in 2007 about a month after Ruby 1.8.6 was released.
ruby@925fbb8
Probably something was just different back then, eg the difference
between lambdas and procs was whether you did `prc.call` or `prc.yield`
https://github.com/ruby/ruby/blob/9b383bd/eval.c#L8356-L8415
# A brutish trick to avoid parsing. Remove it once support for 1.9 and 2.0 is dropped
# https://ci.appveyor.com/project/ruby/rake/build/1.0.301
skip 'Keywords aren\'t a feature in this version' if RUBY_VERSION =~ /^1|^2\.0/
eval <<-RUBY, binding, __FILE__, __LINE__
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need +1 to __LINE__

@nobu
Copy link
Member

nobu commented Nov 29, 2016

A failure in jruby-head seems a bug of jruby.

@n054
Copy link

n054 commented Nov 30, 2016

@JoshCheek let me see if I can help with Travis intigration. @n054

@JoshCheek
Copy link
Contributor Author

@n054 I'd appreciate it ^_^

@n054
Copy link

n054 commented Nov 30, 2016

@JoshCheek investigating the Travis test shows test#482.1-482.7 are passing.
Test#482.8&482.9 show TestRakeTaskWithArguments#test_actions_adore_keywords:
ArgumentError:missing keyword: reqr

@JoshCheek
Copy link
Contributor Author

Looks like it's a JRuby bug. Surprising, maybe I just use keywords a lot more than other people?

Here's the program:

$ cat f.rb
p proc { |a, b:| [a, b] }.call("a-val", b: "b-val")

MRI handles it.

$ chruby 2.3.2
$ ruby -v
ruby 2.3.2p217 (2016-11-15 revision 56796) [x86_64-darwin15]
$ ruby f.rb
["a-val", "b-val"]

Rbx handles it

$ chruby rbx
$ rbx -v
rubinius 3.60 (2.3.1 5d5e49f7 2016-09-21 3.9.0) [x86_64-darwin15.6.0]
$ rbx f.rb
["a-val", "b-val"]

JRuby blows up.

$ chruby jruby
$ jruby -v
jruby 9.1.5.0 (2.3.1) 2016-09-07 036ce39 Java HotSpot(TM) 64-Bit Server VM 25.71-b15 on 1.8.0_71-b15 +jit [darwin-x86_64]
$ jruby f.rb
ArgumentError: missing keyword: b
  block in f.rb at f.rb:-1
         <main> at f.rb:1

@JoshCheek
Copy link
Contributor Author

Looks @nobu beat me to reporting: jruby/jruby#4344

@hsbt hsbt merged commit 4dc6630 into ruby:master Dec 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants