From 7394d12dc7c4bd6c1604e482042efab23f455794 Mon Sep 17 00:00:00 2001 From: Eloy Duran Date: Thu, 11 Dec 2008 14:12:06 +0100 Subject: Fixed ActiveSupport::OrderedHash #delete_if, #reject!, and #reject, which did not sync the @keys after the operation. This probably holds true for other mutating methods as well. Signed-off-by: Michael Koziarski --- activesupport/lib/active_support/ordered_hash.rb | 25 ++++++++++++++++++++++++ activesupport/test/ordered_hash_test.rb | 22 ++++++++++++++++++++- 2 files changed, 46 insertions(+), 1 deletion(-) (limited to 'activesupport') diff --git a/activesupport/lib/active_support/ordered_hash.rb b/activesupport/lib/active_support/ordered_hash.rb index 1ed7737017..fa68db5604 100644 --- a/activesupport/lib/active_support/ordered_hash.rb +++ b/activesupport/lib/active_support/ordered_hash.rb @@ -25,6 +25,25 @@ module ActiveSupport super end + def delete_if + super + sync_keys! + self + end + + def reject! + super + sync_keys! + self + end + + def reject(&block) + dup.reject!(&block) + end + + alias_method :super_keys, :keys + private :super_keys + def keys @keys end @@ -48,6 +67,12 @@ module ActiveSupport def each keys.each {|key| yield [key, self[key]]} end + + private + + def sync_keys! + (@keys - super_keys).each { |k| @keys.delete(k) } + end end end end diff --git a/activesupport/test/ordered_hash_test.rb b/activesupport/test/ordered_hash_test.rb index 094f9316d6..ca5fce6267 100644 --- a/activesupport/test/ordered_hash_test.rb +++ b/activesupport/test/ordered_hash_test.rb @@ -83,4 +83,24 @@ class OrderedHashTest < Test::Unit::TestCase def test_each_with_index @ordered_hash.each_with_index { |pair, index| assert_equal [@keys[index], @values[index]], pair} end -end + + def test_delete_if + (copy = @ordered_hash.dup).delete('pink') + assert_equal copy, @ordered_hash.delete_if { |k, _| k == 'pink' } + assert !@ordered_hash.keys.include?('pink') + end + + def test_reject! + (copy = @ordered_hash.dup).delete('pink') + @ordered_hash.reject! { |k, _| k == 'pink' } + assert_equal copy, @ordered_hash + assert !@ordered_hash.keys.include?('pink') + end + + def test_reject + copy = @ordered_hash.dup + new_ordered_hash = @ordered_hash.reject { |k, _| k == 'pink' } + assert_equal copy, @ordered_hash + assert !new_ordered_hash.keys.include?('pink') + end +end \ No newline at end of file -- cgit v1.2.3 From 4dcd8f01afe5800baa67bbdf72832afb0d627755 Mon Sep 17 00:00:00 2001 From: Frederick Cheung Date: Thu, 11 Dec 2008 15:19:03 +0000 Subject: Make delete_if/reject faster and fix other mutators [#1559 state:committed] Signed-off-by: Jeremy Kemper --- activesupport/lib/active_support/ordered_hash.rb | 37 +++++++++++++++--------- activesupport/test/ordered_hash_test.rb | 37 ++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 13 deletions(-) (limited to 'activesupport') diff --git a/activesupport/lib/active_support/ordered_hash.rb b/activesupport/lib/active_support/ordered_hash.rb index fa68db5604..3def0be639 100644 --- a/activesupport/lib/active_support/ordered_hash.rb +++ b/activesupport/lib/active_support/ordered_hash.rb @@ -18,19 +18,13 @@ module ActiveSupport end def delete(key) - array_index = has_key?(key) && index(key) - if array_index - @keys.delete_at(array_index) + if has_key? key + index = @keys.index(key) + @keys.delete_at index end super end - def delete_if - super - sync_keys! - self - end - def reject! super sync_keys! @@ -41,9 +35,6 @@ module ActiveSupport dup.reject!(&block) end - alias_method :super_keys, :keys - private :super_keys - def keys @keys end @@ -68,10 +59,30 @@ module ActiveSupport keys.each {|key| yield [key, self[key]]} end + alias_method :each_pair, :each + + def clear + super + @keys.clear + self + end + + def shift + k = @keys.first + v = delete(k) + [k, v] + end + + def merge(other_hash) + result = dup + other_hash.each {|k,v| result[k]=v} + result + end + private def sync_keys! - (@keys - super_keys).each { |k| @keys.delete(k) } + @keys.delete_if {|k| !has_key?(k)} end end end diff --git a/activesupport/test/ordered_hash_test.rb b/activesupport/test/ordered_hash_test.rb index ca5fce6267..0e2aa4543d 100644 --- a/activesupport/test/ordered_hash_test.rb +++ b/activesupport/test/ordered_hash_test.rb @@ -36,9 +36,11 @@ class OrderedHashTest < Test::Unit::TestCase @ordered_hash[key] = value assert_equal @keys.length + 1, @ordered_hash.length + assert_equal @ordered_hash.keys.length, @ordered_hash.length assert_equal value, @ordered_hash.delete(key) assert_equal @keys.length, @ordered_hash.length + assert_equal @ordered_hash.keys.length, @ordered_hash.length assert_nil @ordered_hash.delete(bad_key) end @@ -84,6 +86,17 @@ class OrderedHashTest < Test::Unit::TestCase @ordered_hash.each_with_index { |pair, index| assert_equal [@keys[index], @values[index]], pair} end + def test_each_pair + values = [] + keys = [] + @ordered_hash.each_pair do |key, value| + keys << key + values << value + end + assert_equal @values, values + assert_equal @keys, keys + end + def test_delete_if (copy = @ordered_hash.dup).delete('pink') assert_equal copy, @ordered_hash.delete_if { |k, _| k == 'pink' } @@ -103,4 +116,28 @@ class OrderedHashTest < Test::Unit::TestCase assert_equal copy, @ordered_hash assert !new_ordered_hash.keys.include?('pink') end + + def test_clear + @ordered_hash.clear + assert_equal [], @ordered_hash.keys + end + + def test_merge + other_hash = ActiveSupport::OrderedHash.new + other_hash['purple'] = '800080' + other_hash['violet'] = 'ee82ee' + merged = @ordered_hash.merge other_hash + assert_equal merged.length, @ordered_hash.length + other_hash.length + assert_equal @keys + ['purple', 'violet'], merged.keys + + @ordered_hash.merge! other_hash + assert_equal @ordered_hash, merged + assert_equal @ordered_hash.keys, merged.keys + end + + def test_shift + pair = @ordered_hash.shift + assert_equal [@keys.first, @values.first], pair + assert !@ordered_hash.keys.include?(pair.first) + end end \ No newline at end of file -- cgit v1.2.3 From 7c18518105e98ccfd89fe64194ede27824dfe8b3 Mon Sep 17 00:00:00 2001 From: Frederick Cheung Date: Mon, 15 Dec 2008 11:49:08 -0600 Subject: Properly parenthasize calls to defined?(Rails) in 75fa82418 [#1563 state:resolved] Signed-off-by: Joshua Peek --- activesupport/lib/active_support/dependencies.rb | 2 +- activesupport/lib/active_support/deprecation.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'activesupport') diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb index 23b7aee514..7ce9adec2c 100644 --- a/activesupport/lib/active_support/dependencies.rb +++ b/activesupport/lib/active_support/dependencies.rb @@ -559,7 +559,7 @@ module ActiveSupport #:nodoc: # Old style environment.rb referenced this method directly. Please note, it doesn't # actually *do* anything any more. def self.root(*args) - if defined? Rails && Rails.logger + if defined?(Rails) && Rails.logger Rails.logger.warn "Your environment.rb uses the old syntax, it may not continue to work in future releases." Rails.logger.warn "For upgrade instructions please see: http://manuals.rubyonrails.com/read/book/19" end diff --git a/activesupport/lib/active_support/deprecation.rb b/activesupport/lib/active_support/deprecation.rb index f18ea197a5..25b26e9c96 100644 --- a/activesupport/lib/active_support/deprecation.rb +++ b/activesupport/lib/active_support/deprecation.rb @@ -13,7 +13,7 @@ module ActiveSupport $stderr.puts callstack.join("\n ") if debug }, 'development' => Proc.new { |message, callstack| - logger = defined? Rails ? Rails.logger : Logger.new($stderr) + logger = defined?(Rails) ? Rails.logger : Logger.new($stderr) logger.warn message logger.debug callstack.join("\n ") if debug } -- cgit v1.2.3 From 262fef7ed57520b857605a0105fe7ba9265654f6 Mon Sep 17 00:00:00 2001 From: Frederick Cheung Date: Sun, 14 Dec 2008 10:07:06 +0000 Subject: Make constantize look into ancestors [#410 state:resolved] Signed-off-by: Jeremy Kemper --- activesupport/lib/active_support/inflector.rb | 65 ++++++++++----------------- activesupport/test/inflector_test.rb | 17 ++++++- 2 files changed, 40 insertions(+), 42 deletions(-) (limited to 'activesupport') diff --git a/activesupport/lib/active_support/inflector.rb b/activesupport/lib/active_support/inflector.rb index 4921b99677..9fb7540ea2 100644 --- a/activesupport/lib/active_support/inflector.rb +++ b/activesupport/lib/active_support/inflector.rb @@ -330,47 +330,30 @@ module ActiveSupport underscore(demodulize(class_name)) + (separate_class_name_and_id_with_underscore ? "_id" : "id") end - # Ruby 1.9 introduces an inherit argument for Module#const_get and - # #const_defined? and changes their default behavior. - if Module.method(:const_get).arity == 1 - # Tries to find a constant with the name specified in the argument string: - # - # "Module".constantize # => Module - # "Test::Unit".constantize # => Test::Unit - # - # The name is assumed to be the one of a top-level constant, no matter whether - # it starts with "::" or not. No lexical context is taken into account: - # - # C = 'outside' - # module M - # C = 'inside' - # C # => 'inside' - # "C".constantize # => 'outside', same as ::C - # end - # - # NameError is raised when the name is not in CamelCase or the constant is - # unknown. - def constantize(camel_cased_word) - names = camel_cased_word.split('::') - names.shift if names.empty? || names.first.empty? - - constant = Object - names.each do |name| - constant = constant.const_defined?(name) ? constant.const_get(name) : constant.const_missing(name) - end - constant - end - else - def constantize(camel_cased_word) #:nodoc: - names = camel_cased_word.split('::') - names.shift if names.empty? || names.first.empty? - - constant = Object - names.each do |name| - constant = constant.const_get(name, false) || constant.const_missing(name) - end - constant - end + # Tries to find a constant with the name specified in the argument string: + # + # "Module".constantize # => Module + # "Test::Unit".constantize # => Test::Unit + # + # The name is assumed to be the one of a top-level constant, no matter whether + # it starts with "::" or not. No lexical context is taken into account: + # + # C = 'outside' + # module M + # C = 'inside' + # C # => 'inside' + # "C".constantize # => 'outside', same as ::C + # end + # + # NameError is raised when the name is not in CamelCase or the constant is + # unknown. + def constantize(camel_cased_word) + names = camel_cased_word.split('::') + names.shift if names.empty? || names.first.empty? + + constant = Object + names.each { |name| constant = constant.const_get(name) } + constant end # Turns a number into an ordinal string used to denote the position in an diff --git a/activesupport/test/inflector_test.rb b/activesupport/test/inflector_test.rb index d8c93dc9ae..2c422ebe72 100644 --- a/activesupport/test/inflector_test.rb +++ b/activesupport/test/inflector_test.rb @@ -2,8 +2,21 @@ require 'abstract_unit' require 'inflector_test_cases' module Ace + module Extension + def self.included(base) + base.extend(ClassMethods) + end + + module ClassMethods + def mission_accomplished? + false + end + end + end + module Base class Case + include Extension end end end @@ -167,7 +180,9 @@ class InflectorTest < Test::Unit::TestCase end def test_constantize_does_lexical_lookup - assert_raises(NameError) { ActiveSupport::Inflector.constantize("Ace::Base::InflectorTest") } + assert_equal InflectorTest, ActiveSupport::Inflector.constantize("Ace::Base::InflectorTest") + assert_nothing_raised { Ace::Base::Case::ClassMethods } + assert_nothing_raised { assert_equal Ace::Base::Case::ClassMethods, ActiveSupport::Inflector.constantize("Ace::Base::Case::ClassMethods") } end def test_ordinal -- cgit v1.2.3 From 19be3d35b38b6685789d8d343617d465a3652717 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Mon, 15 Dec 2008 18:20:18 -0800 Subject: Revert "Make constantize look into ancestors" [#410 state:open] This reverts commit 262fef7ed57520b857605a0105fe7ba9265654f6. --- activesupport/lib/active_support/inflector.rb | 65 +++++++++++++++++---------- activesupport/test/inflector_test.rb | 17 +------ 2 files changed, 42 insertions(+), 40 deletions(-) (limited to 'activesupport') diff --git a/activesupport/lib/active_support/inflector.rb b/activesupport/lib/active_support/inflector.rb index 9fb7540ea2..4921b99677 100644 --- a/activesupport/lib/active_support/inflector.rb +++ b/activesupport/lib/active_support/inflector.rb @@ -330,30 +330,47 @@ module ActiveSupport underscore(demodulize(class_name)) + (separate_class_name_and_id_with_underscore ? "_id" : "id") end - # Tries to find a constant with the name specified in the argument string: - # - # "Module".constantize # => Module - # "Test::Unit".constantize # => Test::Unit - # - # The name is assumed to be the one of a top-level constant, no matter whether - # it starts with "::" or not. No lexical context is taken into account: - # - # C = 'outside' - # module M - # C = 'inside' - # C # => 'inside' - # "C".constantize # => 'outside', same as ::C - # end - # - # NameError is raised when the name is not in CamelCase or the constant is - # unknown. - def constantize(camel_cased_word) - names = camel_cased_word.split('::') - names.shift if names.empty? || names.first.empty? - - constant = Object - names.each { |name| constant = constant.const_get(name) } - constant + # Ruby 1.9 introduces an inherit argument for Module#const_get and + # #const_defined? and changes their default behavior. + if Module.method(:const_get).arity == 1 + # Tries to find a constant with the name specified in the argument string: + # + # "Module".constantize # => Module + # "Test::Unit".constantize # => Test::Unit + # + # The name is assumed to be the one of a top-level constant, no matter whether + # it starts with "::" or not. No lexical context is taken into account: + # + # C = 'outside' + # module M + # C = 'inside' + # C # => 'inside' + # "C".constantize # => 'outside', same as ::C + # end + # + # NameError is raised when the name is not in CamelCase or the constant is + # unknown. + def constantize(camel_cased_word) + names = camel_cased_word.split('::') + names.shift if names.empty? || names.first.empty? + + constant = Object + names.each do |name| + constant = constant.const_defined?(name) ? constant.const_get(name) : constant.const_missing(name) + end + constant + end + else + def constantize(camel_cased_word) #:nodoc: + names = camel_cased_word.split('::') + names.shift if names.empty? || names.first.empty? + + constant = Object + names.each do |name| + constant = constant.const_get(name, false) || constant.const_missing(name) + end + constant + end end # Turns a number into an ordinal string used to denote the position in an diff --git a/activesupport/test/inflector_test.rb b/activesupport/test/inflector_test.rb index 2c422ebe72..d8c93dc9ae 100644 --- a/activesupport/test/inflector_test.rb +++ b/activesupport/test/inflector_test.rb @@ -2,21 +2,8 @@ require 'abstract_unit' require 'inflector_test_cases' module Ace - module Extension - def self.included(base) - base.extend(ClassMethods) - end - - module ClassMethods - def mission_accomplished? - false - end - end - end - module Base class Case - include Extension end end end @@ -180,9 +167,7 @@ class InflectorTest < Test::Unit::TestCase end def test_constantize_does_lexical_lookup - assert_equal InflectorTest, ActiveSupport::Inflector.constantize("Ace::Base::InflectorTest") - assert_nothing_raised { Ace::Base::Case::ClassMethods } - assert_nothing_raised { assert_equal Ace::Base::Case::ClassMethods, ActiveSupport::Inflector.constantize("Ace::Base::Case::ClassMethods") } + assert_raises(NameError) { ActiveSupport::Inflector.constantize("Ace::Base::InflectorTest") } end def test_ordinal -- cgit v1.2.3