From 06d50b9d73667994375396044318d027d8087aba Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Sat, 23 Jun 2012 18:31:35 -0300 Subject: Remove some not used variables and improve code a bit --- activesupport/lib/active_support/number_helper.rb | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/activesupport/lib/active_support/number_helper.rb b/activesupport/lib/active_support/number_helper.rb index c736041066..8d1b07da24 100644 --- a/activesupport/lib/active_support/number_helper.rb +++ b/activesupport/lib/active_support/number_helper.rb @@ -116,8 +116,7 @@ module ActiveSupport number = number.respond_to?("abs") ? number.abs : number.sub(/^-/, '') end - formatted_number = format.gsub('%n', self.number_to_rounded(number, options)).gsub('%u', unit) - formatted_number + format.gsub('%n', self.number_to_rounded(number, options)).gsub('%u', unit) end # Formats a +number+ as a percentage string (e.g., 65%). You can @@ -160,9 +159,7 @@ module ActiveSupport options = defaults.merge!(options) format = options[:format] || "%n%" - - formatted_number = format.gsub('%n', self.number_to_rounded(number, options)) - formatted_number + format.gsub('%n', self.number_to_rounded(number, options)) end # Formats a +number+ with grouped thousands using +delimiter+ @@ -242,10 +239,9 @@ module ActiveSupport # number_to_rounded(1111.2345, precision: 2, separator: ',', delimiter: '.') # # => 1.111,23 def number_to_rounded(number, options = {}) - options = options.symbolize_keys - return number unless valid_float?(number) - number = Float(number) + number = Float(number) + options = options.symbolize_keys defaults = format_translations('precision', options[:locale]) options = defaults.merge!(options) @@ -254,7 +250,7 @@ module ActiveSupport significant = options.delete :significant strip_insignificant_zeros = options.delete :strip_insignificant_zeros - if significant and precision > 0 + if significant && precision > 0 if number == 0 digits, rounded_number = 1, 0 else @@ -263,10 +259,10 @@ module ActiveSupport digits = (Math.log10(rounded_number.abs) + 1).floor # After rounding, the number of digits may have changed end precision -= digits - precision = precision > 0 ? precision : 0 #don't let it be negative + precision = 0 if precision < 0 # don't let it be negative else rounded_number = BigDecimal.new(number.to_s).round(precision).to_f - rounded_number = rounded_number.zero? ? rounded_number.abs : rounded_number #prevent showing negative zeros + rounded_number = rounded_number.abs if rounded_number.zero? # prevent showing negative zeros end formatted_number = self.number_to_delimited("%01.#{precision}f" % rounded_number, options) if strip_insignificant_zeros @@ -527,6 +523,5 @@ module ActiveSupport false end private_module_and_instance_method :valid_float? - end end -- cgit v1.2.3 From 77b89c293594e6b7d1bf9092244a43ae1e2bd67c Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Sat, 23 Jun 2012 18:46:29 -0300 Subject: Do not propagate the :raise option to AS number helpers ActiveSupport::NumberHelper does not make use of :raise, so there's no need to propagate it down. --- .../lib/action_view/helpers/number_helper.rb | 26 ++++++++++++++++------ 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/actionpack/lib/action_view/helpers/number_helper.rb b/actionpack/lib/action_view/helpers/number_helper.rb index 8f97d1f014..9720e90429 100644 --- a/actionpack/lib/action_view/helpers/number_helper.rb +++ b/actionpack/lib/action_view/helpers/number_helper.rb @@ -59,7 +59,7 @@ module ActionView return unless number options = options.symbolize_keys - parse_float(number, true) if options[:raise] + parse_float(number, true) if options.delete(:raise) ERB::Util.html_escape(ActiveSupport::NumberHelper.number_to_phone(number, options)) end @@ -109,7 +109,9 @@ module ActionView return unless number options = escape_unsafe_delimiters_and_separators(options.symbolize_keys) - wrap_with_output_safety_handling(number, options[:raise]){ ActiveSupport::NumberHelper.number_to_currency(number, options) } + wrap_with_output_safety_handling(number, options.delete(:raise)) { + ActiveSupport::NumberHelper.number_to_currency(number, options) + } end # Formats a +number+ as a percentage string (e.g., 65%). You can @@ -152,7 +154,9 @@ module ActionView return unless number options = escape_unsafe_delimiters_and_separators(options.symbolize_keys) - wrap_with_output_safety_handling(number, options[:raise]){ ActiveSupport::NumberHelper.number_to_percentage(number, options) } + wrap_with_output_safety_handling(number, options.delete(:raise)) { + ActiveSupport::NumberHelper.number_to_percentage(number, options) + } end # Formats a +number+ with grouped thousands using +delimiter+ @@ -187,7 +191,9 @@ module ActionView def number_with_delimiter(number, options = {}) options = escape_unsafe_delimiters_and_separators(options.symbolize_keys) - wrap_with_output_safety_handling(number, options[:raise]){ ActiveSupport::NumberHelper.number_to_delimited(number, options) } + wrap_with_output_safety_handling(number, options.delete(:raise)) { + ActiveSupport::NumberHelper.number_to_delimited(number, options) + } end # Formats a +number+ with the specified level of @@ -234,7 +240,9 @@ module ActionView def number_with_precision(number, options = {}) options = escape_unsafe_delimiters_and_separators(options.symbolize_keys) - wrap_with_output_safety_handling(number, options[:raise]){ ActiveSupport::NumberHelper.number_to_rounded(number, options) } + wrap_with_output_safety_handling(number, options.delete(:raise)) { + ActiveSupport::NumberHelper.number_to_rounded(number, options) + } end @@ -288,7 +296,9 @@ module ActionView def number_to_human_size(number, options = {}) options = escape_unsafe_delimiters_and_separators(options.symbolize_keys) - wrap_with_output_safety_handling(number, options[:raise]){ ActiveSupport::NumberHelper.number_to_human_size(number, options) } + wrap_with_output_safety_handling(number, options.delete(:raise)) { + ActiveSupport::NumberHelper.number_to_human_size(number, options) + } end # Pretty prints (formats and approximates) a number in a way it @@ -392,7 +402,9 @@ module ActionView def number_to_human(number, options = {}) options = escape_unsafe_delimiters_and_separators(options.symbolize_keys) - wrap_with_output_safety_handling(number, options[:raise]){ ActiveSupport::NumberHelper.number_to_human(number, options) } + wrap_with_output_safety_handling(number, options.delete(:raise)) { + ActiveSupport::NumberHelper.number_to_human(number, options) + } end private -- cgit v1.2.3 From 36d863ce51f4c6526f9bbbd871c2c5f5de1565fb Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Sat, 23 Jun 2012 18:52:17 -0300 Subject: Move constants to the top, remove freeze --- activesupport/lib/active_support/number_helper.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/activesupport/lib/active_support/number_helper.rb b/activesupport/lib/active_support/number_helper.rb index 8d1b07da24..99f6489adb 100644 --- a/activesupport/lib/active_support/number_helper.rb +++ b/activesupport/lib/active_support/number_helper.rb @@ -7,9 +7,14 @@ module ActiveSupport module NumberHelper extend self + DECIMAL_UNITS = { 0 => :unit, 1 => :ten, 2 => :hundred, 3 => :thousand, 6 => :million, 9 => :billion, 12 => :trillion, 15 => :quadrillion, + -1 => :deci, -2 => :centi, -3 => :mili, -6 => :micro, -9 => :nano, -12 => :pico, -15 => :femto } + DEFAULT_CURRENCY_VALUES = { :format => "%u%n", :negative_format => "-%u%n", :unit => "$", :separator => ".", :delimiter => ",", :precision => 2, :significant => false, :strip_insignificant_zeros => false } + STORAGE_UNITS = [:byte, :kb, :mb, :gb, :tb] + # Formats a +number+ into a US phone number (e.g., (555) # 123-9876). You can customize the format in the +options+ hash. # @@ -273,8 +278,6 @@ module ActiveSupport end end - STORAGE_UNITS = [:byte, :kb, :mb, :gb, :tb].freeze - # Formats the bytes in +number+ into a more understandable # representation (e.g., giving it 1500 yields 1.5 KB). This # method is useful for reporting file sizes to users. You can @@ -352,9 +355,6 @@ module ActiveSupport end end - DECIMAL_UNITS = {0 => :unit, 1 => :ten, 2 => :hundred, 3 => :thousand, 6 => :million, 9 => :billion, 12 => :trillion, 15 => :quadrillion, - -1 => :deci, -2 => :centi, -3 => :mili, -6 => :micro, -9 => :nano, -12 => :pico, -15 => :femto}.freeze - # Pretty prints (formats and approximates) a number in a way it # is more readable by humans (eg.: 1200000000 becomes "1.2 # Billion"). This is useful for numbers that can get very large -- cgit v1.2.3 From feec54b781fc59b9771c6b5aeaf424914a165bff Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Sat, 23 Jun 2012 18:55:46 -0300 Subject: Refactor AV number helper tests with invalid numbers Just make use of the returning exception from assert_raise, instead of calling the method again with a rescue clause to test the saved exception number. --- actionpack/test/template/number_helper_test.rb | 58 +++++++------------------- 1 file changed, 14 insertions(+), 44 deletions(-) diff --git a/actionpack/test/template/number_helper_test.rb b/actionpack/test/template/number_helper_test.rb index 057cb47f53..d8fffe75ed 100644 --- a/actionpack/test/template/number_helper_test.rb +++ b/actionpack/test/template/number_helper_test.rb @@ -361,69 +361,39 @@ class NumberHelperTest < ActionView::TestCase end def test_number_helpers_should_raise_error_if_invalid_when_specified - assert_raise InvalidNumberError do + exception = assert_raise InvalidNumberError do number_to_human("x", :raise => true) end - begin - number_to_human("x", :raise => true) - rescue InvalidNumberError => e - assert_equal "x", e.number - end + assert_equal "x", exception.number - assert_raise InvalidNumberError do - number_to_human_size("x", :raise => true) - end - begin + exception = assert_raise InvalidNumberError do number_to_human_size("x", :raise => true) - rescue InvalidNumberError => e - assert_equal "x", e.number end + assert_equal "x", exception.number - assert_raise InvalidNumberError do + exception = assert_raise InvalidNumberError do number_with_precision("x", :raise => true) end - begin - number_with_precision("x", :raise => true) - rescue InvalidNumberError => e - assert_equal "x", e.number - end + assert_equal "x", exception.number - assert_raise InvalidNumberError do + exception = assert_raise InvalidNumberError do number_to_currency("x", :raise => true) end - begin - number_with_precision("x", :raise => true) - rescue InvalidNumberError => e - assert_equal "x", e.number - end + assert_equal "x", exception.number - assert_raise InvalidNumberError do + exception = assert_raise InvalidNumberError do number_to_percentage("x", :raise => true) end - begin - number_to_percentage("x", :raise => true) - rescue InvalidNumberError => e - assert_equal "x", e.number - end + assert_equal "x", exception.number - assert_raise InvalidNumberError do - number_with_delimiter("x", :raise => true) - end - begin + exception = assert_raise InvalidNumberError do number_with_delimiter("x", :raise => true) - rescue InvalidNumberError => e - assert_equal "x", e.number end + assert_equal "x", exception.number - assert_raise InvalidNumberError do + exception = assert_raise InvalidNumberError do number_to_phone("x", :raise => true) end - begin - number_to_phone("x", :raise => true) - rescue InvalidNumberError => e - assert_equal "x", e.number - end - + assert_equal "x", exception.number end - end -- cgit v1.2.3 From dc766c9e7b727cbd0b8ff5c6c706b955b6dfa632 Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Sat, 23 Jun 2012 19:36:00 -0300 Subject: Move number helper i18n related tests to AS They also make more sense here since all the related logic with I18n is handled by AS::NumberHelper, and not by AV anymore. --- .../test/template/number_helper_i18n_test.rb | 122 -------------------- activesupport/test/number_helper_i18n_test.rb | 124 +++++++++++++++++++++ 2 files changed, 124 insertions(+), 122 deletions(-) delete mode 100644 actionpack/test/template/number_helper_i18n_test.rb create mode 100644 activesupport/test/number_helper_i18n_test.rb diff --git a/actionpack/test/template/number_helper_i18n_test.rb b/actionpack/test/template/number_helper_i18n_test.rb deleted file mode 100644 index d6e9de9555..0000000000 --- a/actionpack/test/template/number_helper_i18n_test.rb +++ /dev/null @@ -1,122 +0,0 @@ -require 'abstract_unit' - -class NumberHelperTest < ActionView::TestCase - tests ActionView::Helpers::NumberHelper - - def setup - I18n.backend.store_translations 'ts', - :number => { - :format => { :precision => 3, :delimiter => ',', :separator => '.', :significant => false, :strip_insignificant_zeros => false }, - :currency => { :format => { :unit => '&$', :format => '%u - %n', :negative_format => '(%u - %n)', :precision => 2 } }, - :human => { - :format => { - :precision => 2, - :significant => true, - :strip_insignificant_zeros => true - }, - :storage_units => { - :format => "%n %u", - :units => { - :byte => "b", - :kb => "k" - } - }, - :decimal_units => { - :format => "%n %u", - :units => { - :deci => {:one => "Tenth", :other => "Tenths"}, - :unit => "u", - :ten => {:one => "Ten", :other => "Tens"}, - :thousand => "t", - :million => "m" , - :billion =>"b" , - :trillion =>"t" , - :quadrillion =>"q" - } - } - }, - :percentage => { :format => {:delimiter => '', :precision => 2, :strip_insignificant_zeros => true} }, - :precision => { :format => {:delimiter => '', :significant => true} } - }, - :custom_units_for_number_to_human => {:mili => "mm", :centi => "cm", :deci => "dm", :unit => "m", :ten => "dam", :hundred => "hm", :thousand => "km"} - end - - def test_number_to_i18n_currency - assert_equal("&$ - 10.00", number_to_currency(10, :locale => 'ts')) - assert_equal("(&$ - 10.00)", number_to_currency(-10, :locale => 'ts')) - assert_equal("-10.00 - &$", number_to_currency(-10, :locale => 'ts', :format => "%n - %u")) - end - - def test_number_to_currency_with_clean_i18n_settings - clean_i18n do - assert_equal("$10.00", number_to_currency(10)) - assert_equal("-$10.00", number_to_currency(-10)) - end - end - - def test_number_to_currency_without_currency_negative_format - clean_i18n do - I18n.backend.store_translations 'ts', :number => { :currency => { :format => { :unit => '@', :format => '%n %u' } } } - assert_equal("-10.00 @", number_to_currency(-10, :locale => 'ts')) - end - end - - def test_number_with_i18n_precision - #Delimiter was set to "" - assert_equal("10000", number_with_precision(10000, :locale => 'ts')) - - #Precision inherited and significant was set - assert_equal("1.00", number_with_precision(1.0, :locale => 'ts')) - - end - - def test_number_with_i18n_delimiter - #Delimiter "," and separator "." - assert_equal("1,000,000.234", number_with_delimiter(1000000.234, :locale => 'ts')) - end - - def test_number_to_i18n_percentage - # to see if strip_insignificant_zeros is true - assert_equal("1%", number_to_percentage(1, :locale => 'ts')) - # precision is 2, significant should be inherited - assert_equal("1.24%", number_to_percentage(1.2434, :locale => 'ts')) - # no delimiter - assert_equal("12434%", number_to_percentage(12434, :locale => 'ts')) - end - - def test_number_to_i18n_human_size - #b for bytes and k for kbytes - assert_equal("2 k", number_to_human_size(2048, :locale => 'ts')) - assert_equal("42 b", number_to_human_size(42, :locale => 'ts')) - end - - def test_number_to_human_with_default_translation_scope - #Using t for thousand - assert_equal "2 t", number_to_human(2000, :locale => 'ts') - #Significant was set to true with precision 2, using b for billion - assert_equal "1.2 b", number_to_human(1234567890, :locale => 'ts') - #Using pluralization (Ten/Tens and Tenth/Tenths) - assert_equal "1 Tenth", number_to_human(0.1, :locale => 'ts') - assert_equal "1.3 Tenth", number_to_human(0.134, :locale => 'ts') - assert_equal "2 Tenths", number_to_human(0.2, :locale => 'ts') - assert_equal "1 Ten", number_to_human(10, :locale => 'ts') - assert_equal "1.2 Ten", number_to_human(12, :locale => 'ts') - assert_equal "2 Tens", number_to_human(20, :locale => 'ts') - end - - def test_number_to_human_with_custom_translation_scope - #Significant was set to true with precision 2, with custom translated units - assert_equal "4.3 cm", number_to_human(0.0432, :locale => 'ts', :units => :custom_units_for_number_to_human) - end - - private - def clean_i18n - load_path = I18n.load_path.dup - I18n.load_path.clear - I18n.reload! - yield - ensure - I18n.load_path = load_path - I18n.reload! - end -end diff --git a/activesupport/test/number_helper_i18n_test.rb b/activesupport/test/number_helper_i18n_test.rb new file mode 100644 index 0000000000..bc29b4b2d9 --- /dev/null +++ b/activesupport/test/number_helper_i18n_test.rb @@ -0,0 +1,124 @@ +require 'abstract_unit' +require 'active_support/number_helper' + +module ActiveSupport + class NumberHelperI18nTest < ActiveSupport::TestCase + include ActiveSupport::NumberHelper + + def setup + I18n.backend.store_translations 'ts', + :number => { + :format => { :precision => 3, :delimiter => ',', :separator => '.', :significant => false, :strip_insignificant_zeros => false }, + :currency => { :format => { :unit => '&$', :format => '%u - %n', :negative_format => '(%u - %n)', :precision => 2 } }, + :human => { + :format => { + :precision => 2, + :significant => true, + :strip_insignificant_zeros => true + }, + :storage_units => { + :format => "%n %u", + :units => { + :byte => "b", + :kb => "k" + } + }, + :decimal_units => { + :format => "%n %u", + :units => { + :deci => {:one => "Tenth", :other => "Tenths"}, + :unit => "u", + :ten => {:one => "Ten", :other => "Tens"}, + :thousand => "t", + :million => "m", + :billion =>"b", + :trillion =>"t" , + :quadrillion =>"q" + } + } + }, + :percentage => { :format => {:delimiter => '', :precision => 2, :strip_insignificant_zeros => true} }, + :precision => { :format => {:delimiter => '', :significant => true} } + }, + :custom_units_for_number_to_human => {:mili => "mm", :centi => "cm", :deci => "dm", :unit => "m", :ten => "dam", :hundred => "hm", :thousand => "km"} + end + + def test_number_to_i18n_currency + assert_equal("&$ - 10.00", number_to_currency(10, :locale => 'ts')) + assert_equal("(&$ - 10.00)", number_to_currency(-10, :locale => 'ts')) + assert_equal("-10.00 - &$", number_to_currency(-10, :locale => 'ts', :format => "%n - %u")) + end + + def test_number_to_currency_with_clean_i18n_settings + clean_i18n do + assert_equal("$10.00", number_to_currency(10)) + assert_equal("-$10.00", number_to_currency(-10)) + end + end + + def test_number_to_currency_without_currency_negative_format + clean_i18n do + I18n.backend.store_translations 'ts', :number => { :currency => { :format => { :unit => '@', :format => '%n %u' } } } + assert_equal("-10.00 @", number_to_currency(-10, :locale => 'ts')) + end + end + + def test_number_with_i18n_precision + #Delimiter was set to "" + assert_equal("10000", number_to_rounded(10000, :locale => 'ts')) + + #Precision inherited and significant was set + assert_equal("1.00", number_to_rounded(1.0, :locale => 'ts')) + end + + def test_number_with_i18n_delimiter + #Delimiter "," and separator "." + assert_equal("1,000,000.234", number_to_delimited(1000000.234, :locale => 'ts')) + end + + def test_number_to_i18n_percentage + # to see if strip_insignificant_zeros is true + assert_equal("1%", number_to_percentage(1, :locale => 'ts')) + # precision is 2, significant should be inherited + assert_equal("1.24%", number_to_percentage(1.2434, :locale => 'ts')) + # no delimiter + assert_equal("12434%", number_to_percentage(12434, :locale => 'ts')) + end + + def test_number_to_i18n_human_size + #b for bytes and k for kbytes + assert_equal("2 k", number_to_human_size(2048, :locale => 'ts')) + assert_equal("42 b", number_to_human_size(42, :locale => 'ts')) + end + + def test_number_to_human_with_default_translation_scope + #Using t for thousand + assert_equal "2 t", number_to_human(2000, :locale => 'ts') + #Significant was set to true with precision 2, using b for billion + assert_equal "1.2 b", number_to_human(1234567890, :locale => 'ts') + #Using pluralization (Ten/Tens and Tenth/Tenths) + assert_equal "1 Tenth", number_to_human(0.1, :locale => 'ts') + assert_equal "1.3 Tenth", number_to_human(0.134, :locale => 'ts') + assert_equal "2 Tenths", number_to_human(0.2, :locale => 'ts') + assert_equal "1 Ten", number_to_human(10, :locale => 'ts') + assert_equal "1.2 Ten", number_to_human(12, :locale => 'ts') + assert_equal "2 Tens", number_to_human(20, :locale => 'ts') + end + + def test_number_to_human_with_custom_translation_scope + #Significant was set to true with precision 2, with custom translated units + assert_equal "4.3 cm", number_to_human(0.0432, :locale => 'ts', :units => :custom_units_for_number_to_human) + end + + private + def clean_i18n + load_path = I18n.load_path.dup + I18n.load_path.clear + I18n.reload! + yield + ensure + I18n.load_path = load_path + I18n.reload! + end + end +end -- cgit v1.2.3 From d6aea126de689767ee6a1454884d7bf2d0ad4387 Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Sat, 23 Jun 2012 19:39:59 -0300 Subject: Get rid of the clear_i18n hack by using a different locale Use a different and very specific locale for testing currency negative format, and an empty store for currency defaults. --- activesupport/test/number_helper_i18n_test.rb | 30 +++++++++------------------ 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/activesupport/test/number_helper_i18n_test.rb b/activesupport/test/number_helper_i18n_test.rb index bc29b4b2d9..e07198027b 100644 --- a/activesupport/test/number_helper_i18n_test.rb +++ b/activesupport/test/number_helper_i18n_test.rb @@ -49,18 +49,19 @@ module ActiveSupport assert_equal("-10.00 - &$", number_to_currency(-10, :locale => 'ts', :format => "%n - %u")) end - def test_number_to_currency_with_clean_i18n_settings - clean_i18n do - assert_equal("$10.00", number_to_currency(10)) - assert_equal("-$10.00", number_to_currency(-10)) - end + def test_number_to_currency_with_empty_i18n_store + I18n.backend.store_translations 'empty', {} + + assert_equal("$10.00", number_to_currency(10, :locale => 'empty')) + assert_equal("-$10.00", number_to_currency(-10, :locale => 'empty')) end def test_number_to_currency_without_currency_negative_format - clean_i18n do - I18n.backend.store_translations 'ts', :number => { :currency => { :format => { :unit => '@', :format => '%n %u' } } } - assert_equal("-10.00 @", number_to_currency(-10, :locale => 'ts')) - end + I18n.backend.store_translations 'no_negative_format', :number => { + :currency => { :format => { :unit => '@', :format => '%n %u' } } + } + + assert_equal("-10.00 @", number_to_currency(-10, :locale => 'no_negative_format')) end def test_number_with_i18n_precision @@ -109,16 +110,5 @@ module ActiveSupport #Significant was set to true with precision 2, with custom translated units assert_equal "4.3 cm", number_to_human(0.0432, :locale => 'ts', :units => :custom_units_for_number_to_human) end - - private - def clean_i18n - load_path = I18n.load_path.dup - I18n.load_path.clear - I18n.reload! - yield - ensure - I18n.load_path = load_path - I18n.reload! - end end end -- cgit v1.2.3