Use #prepend rather than using 2 aliases #19752

Merged
merged 1 commit into from Apr 14, 2015

Conversation

Projects
None yet
8 participants
@yuki24
Contributor

yuki24 commented Apr 14, 2015

This is a follow-up pull request to #19434. Now that Ruby 2.2.2 is out and we can use #prepend anywhere.

cc/ @rafaelfranca @kirs

yuki24 added a commit to yuki24/rails that referenced this pull request Apr 14, 2015

Use Ruby 2.2.2 on travis
This is required to run rails#19752 successfully.

@yuki24 yuki24 referenced this pull request Apr 14, 2015

Merged

Use Ruby 2.2.2 on travis #19754

@yuki24

This comment has been minimized.

Show comment
Hide comment
@yuki24

yuki24 Apr 14, 2015

Contributor

For some reason the build status didn't show up here, but it's green on travis.

Contributor

yuki24 commented Apr 14, 2015

For some reason the build status didn't show up here, but it's green on travis.

@kirs

This comment has been minimized.

Show comment
Hide comment
@kirs

kirs Apr 14, 2015

Member

Nice catch! Looks good for me.

Member

kirs commented Apr 14, 2015

Nice catch! Looks good for me.

- else
- include_without_range?(value)
+module ActiveSupport
+ module IncludeWithRange

This comment has been minimized.

@carlosantoniodasilva

carlosantoniodasilva Apr 14, 2015

Member

I think we should nodoc this constant as well, it will be weird to have it showing up in the docs.

@carlosantoniodasilva

carlosantoniodasilva Apr 14, 2015

Member

I think we should nodoc this constant as well, it will be weird to have it showing up in the docs.

This comment has been minimized.

@yuki24

yuki24 Apr 14, 2015

Contributor

done.

@yuki24

yuki24 Apr 14, 2015

Contributor

done.

+ if value.is_a?(::Range)
+ # 1...10 includes 1..9 but it does not include 1..10.
+ operator = exclude_end? && !value.exclude_end? ? :< : :<=
+ super(value.first) && value.last.send(operator, last)

This comment has been minimized.

@carlosantoniodasilva

carlosantoniodasilva Apr 14, 2015

Member

Sorry, I just noticed it now, but this indent seems weird doesn't it?

@carlosantoniodasilva

carlosantoniodasilva Apr 14, 2015

Member

Sorry, I just noticed it now, but this indent seems weird doesn't it?

This comment has been minimized.

@yuki24

yuki24 Apr 14, 2015

Contributor

I'm not sure, I'm happy to remove the line break here, though. WDYT?

@yuki24

yuki24 Apr 14, 2015

Contributor

I'm not sure, I'm happy to remove the line break here, though. WDYT?

This comment has been minimized.

@carlosantoniodasilva

carlosantoniodasilva Apr 14, 2015

Member

The second line has extra indent, I think it should be indented like this:

        operator = exclude_end? && !value.exclude_end? ? :< : :<=
        super(value.first) && value.last.send(operator, last)
@carlosantoniodasilva

carlosantoniodasilva Apr 14, 2015

Member

The second line has extra indent, I think it should be indented like this:

        operator = exclude_end? && !value.exclude_end? ? :< : :<=
        super(value.first) && value.last.send(operator, last)

This comment has been minimized.

@yuki24

yuki24 Apr 14, 2015

Contributor

Thanks, I missed that. the commit has been updated.

@yuki24

yuki24 Apr 14, 2015

Contributor

Thanks, I missed that. the commit has been updated.

@morgoth morgoth referenced this pull request Apr 14, 2015

Merged

Upgrade to Ruby 2.2.2 #19753

@rafaelfranca

This comment has been minimized.

Show comment
Hide comment

carlosantoniodasilva added a commit that referenced this pull request Apr 14, 2015

@carlosantoniodasilva carlosantoniodasilva merged commit fcd8e62 into rails:master Apr 14, 2015

@carlosantoniodasilva

This comment has been minimized.

Show comment
Hide comment
@carlosantoniodasilva

carlosantoniodasilva Apr 14, 2015

Member

thanks! 💚💛❤️💙💜

thanks! 💚💛❤️💙💜

@yuki24 yuki24 deleted the yuki24:use-prepend-rather-than-alias branch Apr 14, 2015

@baburdick

This comment has been minimized.

Show comment
Hide comment
@baburdick

baburdick Jan 29, 2016

This logic breaks expressions like this:

case number
when   0...400            then 100.0
when 400..Float::INFINITY then 200.0
else raise StandardError
end

Why not instead feed the start and end values of the passed range to #include_without_range?

In the meantime, it's possible to workaround the breakage in my example by using Float::MAX instead:

case number
when   0...400       then 100.0
when 400..Float::MAX then 200.0
else raise StandardError
end

This logic breaks expressions like this:

case number
when   0...400            then 100.0
when 400..Float::INFINITY then 200.0
else raise StandardError
end

Why not instead feed the start and end values of the passed range to #include_without_range?

In the meantime, it's possible to workaround the breakage in my example by using Float::MAX instead:

case number
when   0...400       then 100.0
when 400..Float::MAX then 200.0
else raise StandardError
end

This comment has been minimized.

Show comment
Hide comment
@albertosaurus

albertosaurus Feb 1, 2016

@baburdick is this what you have in mind?

def include?(value)
  if value.is_a?(::Range)
    super(value.first) && super(value.last)
  else
    super
  end
end

@baburdick is this what you have in mind?

def include?(value)
  if value.is_a?(::Range)
    super(value.first) && super(value.last)
  else
    super
  end
end

This comment has been minimized.

Show comment
Hide comment
@baburdick

baburdick Feb 1, 2016

It appears that that would do it.

It appears that that would do it.

This comment has been minimized.

Show comment
Hide comment
@albertosaurus

albertosaurus Feb 2, 2016

The problem with that approach is that (x...y).include?(x...y) should evaluate to true, but won't.

The problem with that approach is that (x...y).include?(x...y) should evaluate to true, but won't.

This comment has been minimized.

Show comment
Hide comment
@lorefnon

lorefnon Feb 14, 2016

@baburdick In your example, for what value of number is the result unexpected ?

@baburdick In your example, for what value of number is the result unexpected ?

This comment has been minimized.

Show comment
Hide comment
@nathell

nathell Mar 1, 2016

@lorefnon I don't see it either. Because case calls === rather than include? under the hood, it would appear that the example is not relevant. In fact, it works for me on Rails master.

@lorefnon I don't see it either. Because case calls === rather than include? under the hood, it would appear that the example is not relevant. In fact, it works for me on Rails master.

This comment has been minimized.

Show comment
Hide comment
@baburdick

baburdick Mar 1, 2016

I apologize. I was beginning to think I can no longer produce this, and that it must have been a strange interaction between a recent rails version (prior to 4.1.14.1) and a gem. But I recovered my notes on this. The problem manifests when the number is a BigDecimal. Rails console example (Rails 4.1.14.2, Ruby v2.1.2):

2.1.2 :013 > number = BigDecimal.new 1000000
 => #<BigDecimal:660fd00,'0.1E7',9(27)> 
2.1.2 :019 > number < Float::MAX
 => true 
2.1.2 :020 > number < Float::INFINITY
FloatDomainError: Infinity
    from (irb):20:in `to_r'
    from (irb):20:in `<'
    from (irb):20
    from /usr/local/rvm/gems/ruby-2.1.2@my_service/gems/railties-4.1.14.2/lib/rails/commands/console.rb:90:in `start'
    from /usr/local/rvm/gems/ruby-2.1.2@my_service/gems/railties-4.1.14.2/lib/rails/commands/console.rb:9:in `start'
    from /usr/local/rvm/gems/ruby-2.1.2@my_service/gems/railties-4.1.14.2/lib/rails/commands/commands_tasks.rb:69:in `console'
    from /usr/local/rvm/gems/ruby-2.1.2@my_service/gems/railties-4.1.14.2/lib/rails/commands/commands_tasks.rb:40:in `run_command!'
    from /usr/local/rvm/gems/ruby-2.1.2@my_service/gems/railties-4.1.14.2/lib/rails/commands.rb:17:in `<top (required)>'
    from bin/rails:4:in `require'
    from bin/rails:4:in `<main>'

Sorry to have wasted your time. With luck I'll remember to post more environment details with such reports.

I apologize. I was beginning to think I can no longer produce this, and that it must have been a strange interaction between a recent rails version (prior to 4.1.14.1) and a gem. But I recovered my notes on this. The problem manifests when the number is a BigDecimal. Rails console example (Rails 4.1.14.2, Ruby v2.1.2):

2.1.2 :013 > number = BigDecimal.new 1000000
 => #<BigDecimal:660fd00,'0.1E7',9(27)> 
2.1.2 :019 > number < Float::MAX
 => true 
2.1.2 :020 > number < Float::INFINITY
FloatDomainError: Infinity
    from (irb):20:in `to_r'
    from (irb):20:in `<'
    from (irb):20
    from /usr/local/rvm/gems/ruby-2.1.2@my_service/gems/railties-4.1.14.2/lib/rails/commands/console.rb:90:in `start'
    from /usr/local/rvm/gems/ruby-2.1.2@my_service/gems/railties-4.1.14.2/lib/rails/commands/console.rb:9:in `start'
    from /usr/local/rvm/gems/ruby-2.1.2@my_service/gems/railties-4.1.14.2/lib/rails/commands/commands_tasks.rb:69:in `console'
    from /usr/local/rvm/gems/ruby-2.1.2@my_service/gems/railties-4.1.14.2/lib/rails/commands/commands_tasks.rb:40:in `run_command!'
    from /usr/local/rvm/gems/ruby-2.1.2@my_service/gems/railties-4.1.14.2/lib/rails/commands.rb:17:in `<top (required)>'
    from bin/rails:4:in `require'
    from bin/rails:4:in `<main>'

Sorry to have wasted your time. With luck I'll remember to post more environment details with such reports.

This comment has been minimized.

Show comment
Hide comment

This comment has been minimized.

Show comment
Hide comment
@baburdick

baburdick Mar 1, 2016

Looks like the most appropriate current workaround is along these lines:

(400..BigDecimal("Infinity")) === number
# => true 

Looks like the most appropriate current workaround is along these lines:

(400..BigDecimal("Infinity")) === number
# => true 
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment