From fb22f93b3837d18300c41cf656cef5e9deda61a7 Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Sat, 21 Jan 2012 10:21:32 -0200 Subject: Improve getting translations for number helpers --- .../lib/action_view/helpers/number_helper.rb | 23 ++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/actionpack/lib/action_view/helpers/number_helper.rb b/actionpack/lib/action_view/helpers/number_helper.rb index fc1cbfcb14..fc41e5b72c 100644 --- a/actionpack/lib/action_view/helpers/number_helper.rb +++ b/actionpack/lib/action_view/helpers/number_helper.rb @@ -188,9 +188,8 @@ module ActionView options.symbolize_keys! - defaults = defaults_translations(options[:locale]).merge(translations_for('percentage', options[:locale])) - - options = options.reverse_merge(defaults) + defaults = format_translations('percentage', options[:locale]) + options = defaults.merge!(options) format = options[:format] || "%n%" @@ -237,7 +236,7 @@ module ActionView return number end - options = options.reverse_merge(defaults_translations(options[:locale])) + options = defaults_translations(options[:locale]).merge(options) parts = number.to_s.to_str.split('.') parts[0].gsub!(/(\d)(?=(\d\d\d)+(?!\d))/, "\\1#{options[:delimiter]}") @@ -283,9 +282,9 @@ module ActionView return number end - defaults = defaults_translations(options[:locale]).merge(translations_for('precision', options[:locale])) + defaults = format_translations('precision', options[:locale]) + options = defaults.merge!(options) - options = options.reverse_merge(defaults) # Allow the user to unset default values: Eg.: :significant => false precision = options.delete :precision significant = options.delete :significant strip_insignificant_zeros = options.delete :strip_insignificant_zeros @@ -352,9 +351,9 @@ module ActionView return number end - defaults = defaults_translations(options[:locale]).merge(translations_for('human', options[:locale])) + defaults = format_translations('human', options[:locale]) + options = defaults.merge!(options) - options = options.reverse_merge(defaults) #for backwards compatibility with those that didn't add strip_insignificant_zeros to their locale files options[:strip_insignificant_zeros] = true if not options.key?(:strip_insignificant_zeros) @@ -464,9 +463,9 @@ module ActionView return number end - defaults = defaults_translations(options[:locale]).merge(translations_for('human', options[:locale])) + defaults = format_translations('human', options[:locale]) + options = defaults.merge!(options) - options = options.reverse_merge(defaults) #for backwards compatibility with those that didn't add strip_insignificant_zeros to their locale files options[:strip_insignificant_zeros] = true if not options.key?(:strip_insignificant_zeros) @@ -504,6 +503,10 @@ module ActionView private + def format_translations(namespace, locale) + defaults_translations(locale).merge(translations_for(namespace, locale)) + end + def defaults_translations(locale) I18n.translate(:'number.format', :locale => locale, :default => {}) end -- cgit v1.2.3 From 1e78fed488d136041f44a452012a71bb1a348c2f Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Sat, 21 Jan 2012 10:34:06 -0200 Subject: Refactor float number parsing --- .../lib/action_view/helpers/number_helper.rb | 34 ++++++---------------- 1 file changed, 9 insertions(+), 25 deletions(-) diff --git a/actionpack/lib/action_view/helpers/number_helper.rb b/actionpack/lib/action_view/helpers/number_helper.rb index fc41e5b72c..2fab23e777 100644 --- a/actionpack/lib/action_view/helpers/number_helper.rb +++ b/actionpack/lib/action_view/helpers/number_helper.rb @@ -57,15 +57,11 @@ module ActionView # # => +1.123.555.1234 x 1343 def number_to_phone(number, options = {}) return unless number + options.symbolize_keys! - begin - Float(number) - rescue ArgumentError, TypeError - raise InvalidNumberError, number - end if options[:raise] + parse_float(number, true) if options[:raise] number = number.to_s.strip - options = options.symbolize_keys area_code = options[:area_code] delimiter = options[:delimiter] || "-" extension = options[:extension] @@ -75,7 +71,7 @@ module ActionView number.gsub!(/(\d{1,3})(\d{3})(\d{4}$)/,"(\\1) \\2#{delimiter}\\3") else number.gsub!(/(\d{0,3})(\d{3})(\d{4})$/,"\\1#{delimiter}\\2#{delimiter}\\3") - number.slice!(0, 1) if number.starts_with?(delimiter) && !delimiter.blank? + number.slice!(0, 1) if number.start_with?(delimiter) && !delimiter.blank? end str = [] @@ -232,9 +228,7 @@ module ActionView def number_with_delimiter(number, options = {}) options.symbolize_keys! - parse_float_number(number, options[:raise]) do - return number - end + parse_float(number, options[:raise]) or return number options = defaults_translations(options[:locale]).merge(options) @@ -278,9 +272,7 @@ module ActionView def number_with_precision(number, options = {}) options.symbolize_keys! - number = parse_float_number(number, options[:raise]) do - return number - end + number = (parse_float(number, options[:raise]) or return number) defaults = format_translations('precision', options[:locale]) options = defaults.merge!(options) @@ -347,9 +339,7 @@ module ActionView def number_to_human_size(number, options = {}) options.symbolize_keys! - number = parse_float_number(number, options[:raise]) do - return number - end + number = (parse_float(number, options[:raise]) or return number) defaults = format_translations('human', options[:locale]) options = defaults.merge!(options) @@ -459,9 +449,7 @@ module ActionView def number_to_human(number, options = {}) options.symbolize_keys! - number = parse_float_number(number, options[:raise]) do - return number - end + number = (parse_float(number, options[:raise]) or return number) defaults = format_translations('human', options[:locale]) options = defaults.merge!(options) @@ -515,14 +503,10 @@ module ActionView I18n.translate(:"number.#{namespace}.format", :locale => locale, :default => {}) end - def parse_float_number(number, raise_error) + def parse_float(number, raise_error) Float(number) rescue ArgumentError, TypeError - if raise_error - raise InvalidNumberError, number - else - yield - end + raise InvalidNumberError, number if raise_error end end end -- cgit v1.2.3 From c753e3af764f4888bc71720741158863b23d63c1 Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Sat, 21 Jan 2012 10:36:37 -0200 Subject: Refactor percentage helper --- actionpack/lib/action_view/helpers/number_helper.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/actionpack/lib/action_view/helpers/number_helper.rb b/actionpack/lib/action_view/helpers/number_helper.rb index 2fab23e777..9808178800 100644 --- a/actionpack/lib/action_view/helpers/number_helper.rb +++ b/actionpack/lib/action_view/helpers/number_helper.rb @@ -196,7 +196,8 @@ module ActionView if options[:raise] raise else - e.number.to_s.html_safe? ? format.gsub(/%n/, e.number).html_safe : format.gsub(/%n/, e.number) + formatted_number = format.gsub(/%n/, e.number) + e.number.to_s.html_safe? ? formatted_number.html_safe : formatted_number end end end -- cgit v1.2.3 From 74fabdf6c4c00827c657b4d6cef9ae6b96d3d0d2 Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Wed, 25 Jan 2012 10:30:32 -0200 Subject: Do not mutate given options hash in number helpers --- .../lib/action_view/helpers/number_helper.rb | 16 ++++++-------- actionpack/test/template/number_helper_test.rb | 25 ++++++++++++++++++++++ 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/actionpack/lib/action_view/helpers/number_helper.rb b/actionpack/lib/action_view/helpers/number_helper.rb index 9808178800..b0860f87c4 100644 --- a/actionpack/lib/action_view/helpers/number_helper.rb +++ b/actionpack/lib/action_view/helpers/number_helper.rb @@ -57,7 +57,7 @@ module ActionView # # => +1.123.555.1234 x 1343 def number_to_phone(number, options = {}) return unless number - options.symbolize_keys! + options = options.symbolize_keys parse_float(number, true) if options[:raise] @@ -118,8 +118,7 @@ module ActionView # # => 1234567890,50 £ def number_to_currency(number, options = {}) return unless number - - options.symbolize_keys! + options = options.symbolize_keys currency = translations_for('currency', options[:locale]) currency[:negative_format] ||= "-" + currency[:format] if currency[:format] @@ -181,8 +180,7 @@ module ActionView # number_to_percentage("98a", :raise => true) # => InvalidNumberError def number_to_percentage(number, options = {}) return unless number - - options.symbolize_keys! + options = options.symbolize_keys defaults = format_translations('percentage', options[:locale]) options = defaults.merge!(options) @@ -227,7 +225,7 @@ module ActionView # # number_with_delimiter("112a", :raise => true) # => raise InvalidNumberError def number_with_delimiter(number, options = {}) - options.symbolize_keys! + options = options.symbolize_keys parse_float(number, options[:raise]) or return number @@ -271,7 +269,7 @@ module ActionView # number_with_precision(1111.2345, :precision => 2, :separator => ',', :delimiter => '.') # # => 1.111,23 def number_with_precision(number, options = {}) - options.symbolize_keys! + options = options.symbolize_keys number = (parse_float(number, options[:raise]) or return number) @@ -338,7 +336,7 @@ module ActionView # number_to_human_size(1234567890123, :precision => 5) # => "1.1229 TB" # number_to_human_size(524288000, :precision => 5) # => "500 MB" def number_to_human_size(number, options = {}) - options.symbolize_keys! + options = options.symbolize_keys number = (parse_float(number, options[:raise]) or return number) @@ -448,7 +446,7 @@ module ActionView # number_to_human(0.34, :units => :distance) # => "34 centimeters" # def number_to_human(number, options = {}) - options.symbolize_keys! + options = options.symbolize_keys number = (parse_float(number, options[:raise]) or return number) diff --git a/actionpack/test/template/number_helper_test.rb b/actionpack/test/template/number_helper_test.rb index ca897a48cc..482d953907 100644 --- a/actionpack/test/template/number_helper_test.rb +++ b/actionpack/test/template/number_helper_test.rb @@ -268,6 +268,31 @@ class NumberHelperTest < ActionView::TestCase assert_nil number_to_human(nil) end + def test_number_helpers_do_not_mutate_options_hash + options = { 'raise' => true } + + number_to_phone(1, options) + assert_equal({ 'raise' => true }, options) + + number_to_currency(1, options) + assert_equal({ 'raise' => true }, options) + + number_to_percentage(1, options) + assert_equal({ 'raise' => true }, options) + + number_with_delimiter(1, options) + assert_equal({ 'raise' => true }, options) + + number_with_precision(1, options) + assert_equal({ 'raise' => true }, options) + + number_to_human_size(1, options) + assert_equal({ 'raise' => true }, options) + + number_to_human(1, options) + assert_equal({ 'raise' => true }, options) + end + def test_number_helpers_should_return_non_numeric_param_unchanged assert_equal("+1-x x 123", number_to_phone("x", :country_code => 1, :extension => 123)) assert_equal("x", number_to_phone("x")) -- cgit v1.2.3