Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Number helpers defaults #6290

Closed
wants to merge 2 commits into from

3 participants

Carlos Antonio da Silva Rafael Mendonça França Aaron Patterson
Carlos Antonio da Silva

This uses the :en locale as default fallback for number helpers, such as number_with_precision and brothers. The overall idea is that we should always have fallbacks when using such helpers, instead of letting them raise different errors due to missing arguments. The implementation basically fetches the defaults for :en and merges the translations with the given locale.

After some discussion it seems a good path to use the default :en translations that ship with the framework as fallbacks instead of constants, to avoid the duplicated defaults. I'd like some feedback on this, thanks.

Closes #4420, #2802, #2890.

Carlos Antonio da Silva

This is going to change to use constants as defaults, instead of relying on I18n, to have them close to the code they belong to.

Aaron Patterson
Owner

I guess it's alright? :ok_hand:

Carlos Antonio da Silva

I guess so, but I'll wait this pull request that moves the helpers to AS to continue :smile:

Carlos Antonio da Silva

Now I think I'd like some feedback on this one, @josevalim @rafaelfranca @tenderlove :smile:

Carlos Antonio da Silva carlosantoniodasilva Fallback to :en locale instead of handling a constant with defaults
Action Pack already comes with a default locale fine for :en, that is
always loaded. We can just fallback to this locale for defaults, if
values for the current locale cannot be found.

Closes #4420, #2802, #2890.
f6b7149
Carlos Antonio da Silva carlosantoniodasilva Ensure I18n format values always have precedence over defaults
Always merge I18n format values, namespaced or not, over the default
ones, to ensure I18n format defaults will have precedence over our
namespaced values.

Precedence should happen like this:

    default :format
    default :namespace :format
    i18n    :format
    i18n    :namespace :format

Because we cannot allow our namespaced default to override a I18n
:format config - ie precision in I18n :format should always have higher
precedence than our default precision for a particular :namespace.

Also simplify default format options logic.
d6efe79
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Aug 11, 2012
  1. Carlos Antonio da Silva

    Fallback to :en locale instead of handling a constant with defaults

    carlosantoniodasilva authored
    Action Pack already comes with a default locale fine for :en, that is
    always loaded. We can just fallback to this locale for defaults, if
    values for the current locale cannot be found.
    
    Closes #4420, #2802, #2890.
  2. Carlos Antonio da Silva

    Ensure I18n format values always have precedence over defaults

    carlosantoniodasilva authored
    Always merge I18n format values, namespaced or not, over the default
    ones, to ensure I18n format defaults will have precedence over our
    namespaced values.
    
    Precedence should happen like this:
    
        default :format
        default :namespace :format
        i18n    :format
        i18n    :namespace :format
    
    Because we cannot allow our namespaced default to override a I18n
    :format config - ie precision in I18n :format should always have higher
    precedence than our default precision for a particular :namespace.
    
    Also simplify default format options logic.
This page is out of date. Refresh to see the latest.
5 activesupport/CHANGELOG.md
View
@@ -1,5 +1,10 @@
## Rails 4.0.0 (unreleased) ##
+* Add default values to all `ActiveSupport::NumberHelper` methods, to avoid
+ errors with empty locales or missing values.
+
+ *Carlos Antonio da Silva*
+
* `ERB::Util.html_escape` now escapes single quotes. *Santiago Pastorino*
* `Time#change` now works with time values with offsets other than UTC or the local time zone. *Andrew White*
10 activesupport/lib/active_support/locale/en.yml
View
@@ -49,7 +49,7 @@ en:
significant: false
# If set, the zeros after the decimal separator will always be stripped (eg.: 1.200 will be 1.2)
strip_insignificant_zeros: false
-
+
# Used in NumberHelper.number_to_currency()
currency:
format:
@@ -62,7 +62,7 @@ en:
precision: 2
significant: false
strip_insignificant_zeros: false
-
+
# Used in NumberHelper.number_to_percentage()
percentage:
format:
@@ -73,7 +73,7 @@ en:
# significant: false
# strip_insignificant_zeros: false
format: "%n%"
-
+
# Used in NumberHelper.number_to_rounded()
precision:
format:
@@ -83,7 +83,7 @@ en:
# precision:
# significant: false
# strip_insignificant_zeros: false
-
+
# Used in NumberHelper.number_to_human_size() and NumberHelper.number_to_human()
human:
format:
@@ -131,5 +131,3 @@ en:
billion: Billion
trillion: Trillion
quadrillion: Quadrillion
-
-
161 activesupport/lib/active_support/number_helper.rb
View
@@ -7,12 +7,108 @@ module ActiveSupport
module NumberHelper
extend self
+ DEFAULTS = {
+ # Used in number_to_delimited
+ # These are also the defaults for 'currency', 'percentage', 'precision', and 'human'
+ format: {
+ # Sets the separator between the units, for more precision (e.g. 1.0 / 2.0 == 0.5)
+ separator: ".",
+ # Delimits thousands (e.g. 1,000,000 is a million) (always in groups of three)
+ delimiter: ",",
+ # Number of decimals, behind the separator (the number 1 with a precision of 2 gives: 1.00)
+ precision: 3,
+ # If set to true, precision will mean the number of significant digits instead
+ # of the number of decimal digits (1234 with precision 2 becomes 1200, 1.23543 becomes 1.2)
+ significant: false,
+ # If set, the zeros after the decimal separator will always be stripped (eg.: 1.200 will be 1.2)
+ strip_insignificant_zeros: false
+ },
+
+ # Used in number_to_currency
+ currency: {
+ format: {
+ format: "%u%n",
+ negative_format: "-%u%n",
+ unit: "$",
+ # These five are to override number.format and are optional
+ separator: ".",
+ delimiter: ",",
+ precision: 2,
+ significant: false,
+ strip_insignificant_zeros: false
+ }
+ },
+
+ # Used in number_to_percentage
+ percentage: {
+ format: {
+ delimiter: "",
+ format: "%n%"
+ }
+ },
+
+ # Used in number_to_rounded
+ precision: {
+ format: {
+ delimiter: ""
+ }
+ },
+
+ # Used in number_to_human_size and number_to_human
+ human: {
+ format: {
+ # These five are to override number.format and are optional
+ delimiter: "",
+ precision: 3,
+ significant: true,
+ strip_insignificant_zeros: true
+ },
+ # Used in number_to_human_size
+ storage_units: {
+ # Storage units output formatting.
+ # %u is the storage unit, %n is the number (default: 2 MB)
+ format: "%n %u",
+ units: {
+ byte: "Bytes",
+ kb: "KB",
+ mb: "MB",
+ gb: "GB",
+ tb: "TB"
+ }
+ },
+ # Used in number_to_human
+ decimal_units: {
+ format: "%n %u",
+ # Decimal units output formatting
+ # By default we will only quantify some of the exponents
+ # but the commented ones might be defined or overridden
+ # by the user.
+ units: {
+ # femto: Quadrillionth
+ # pico: Trillionth
+ # nano: Billionth
+ # micro: Millionth
+ # mili: Thousandth
+ # centi: Hundredth
+ # deci: Tenth
+ unit: "",
+ # ten:
+ # one: Ten
+ # other: Tens
+ # hundred: Hundred
+ thousand: "Thousand",
+ million: "Million",
+ billion: "Billion",
+ trillion: "Trillion",
+ quadrillion: "Quadrillion"
+ }
+ }
+ }
+ }
+
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)
@@ -106,10 +202,10 @@ def number_to_currency(number, options = {})
return unless number
options = options.symbolize_keys
- currency = translations_for('currency', options[:locale])
+ currency = i18n_format_options(options[:locale], :currency)
currency[:negative_format] ||= "-" + currency[:format] if currency[:format]
- defaults = DEFAULT_CURRENCY_VALUES.merge(defaults_translations(options[:locale])).merge!(currency)
+ defaults = default_format_options(:currency).merge!(currency)
defaults[:negative_format] = "-" + options[:format] if options[:format]
options = defaults.merge!(options)
@@ -160,7 +256,7 @@ def number_to_percentage(number, options = {})
return unless number
options = options.symbolize_keys
- defaults = format_translations('percentage', options[:locale])
+ defaults = format_options(options[:locale], :percentage)
options = defaults.merge!(options)
format = options[:format] || "%n%"
@@ -197,7 +293,7 @@ def number_to_delimited(number, options = {})
return number unless valid_float?(number)
- options = defaults_translations(options[:locale]).merge(options)
+ options = format_options(options[:locale]).merge!(options)
parts = number.to_s.to_str.split('.')
parts[0].gsub!(/(\d)(?=(\d\d\d)+(?!\d))/, "\\1#{options[:delimiter]}")
@@ -248,7 +344,7 @@ def number_to_rounded(number, options = {})
number = Float(number)
options = options.symbolize_keys
- defaults = format_translations('precision', options[:locale])
+ defaults = format_options(options[:locale], :precision)
options = defaults.merge!(options)
precision = options.delete :precision
@@ -328,18 +424,18 @@ def number_to_human_size(number, options = {})
return number unless valid_float?(number)
number = Float(number)
- defaults = format_translations('human', options[:locale])
+ defaults = format_options(options[:locale], :human)
options = defaults.merge!(options)
#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)
- storage_units_format = I18n.translate(:'number.human.storage_units.format', :locale => options[:locale], :raise => true)
+ storage_units_format = translate_number_value_with_default('human.storage_units.format', :locale => options[:locale], :raise => true)
base = options[:prefix] == :si ? 1000 : 1024
if number.to_i < base
- unit = I18n.translate(:'number.human.storage_units.units.byte', :locale => options[:locale], :count => number.to_i, :raise => true)
+ unit = translate_number_value_with_default('human.storage_units.units.byte', :locale => options[:locale], :count => number.to_i, :raise => true)
storage_units_format.gsub(/%n/, number.to_i.to_s).gsub(/%u/, unit)
else
max_exp = STORAGE_UNITS.size - 1
@@ -348,7 +444,7 @@ def number_to_human_size(number, options = {})
number /= base ** exponent
unit_key = STORAGE_UNITS[exponent]
- unit = I18n.translate(:"number.human.storage_units.units.#{unit_key}", :locale => options[:locale], :count => number, :raise => true)
+ unit = translate_number_value_with_default("human.storage_units.units.#{unit_key}", :locale => options[:locale], :count => number, :raise => true)
formatted_number = self.number_to_rounded(number, options)
storage_units_format.gsub(/%n/, formatted_number).gsub(/%u/, unit)
@@ -458,7 +554,7 @@ def number_to_human(number, options = {})
return number unless valid_float?(number)
number = Float(number)
- defaults = format_translations('human', options[:locale])
+ defaults = format_options(options[:locale], :human)
options = defaults.merge!(options)
#for backwards compatibility with those that didn't add strip_insignificant_zeros to their locale files
@@ -473,7 +569,7 @@ def number_to_human(number, options = {})
when String, Symbol
I18n.translate(:"#{units}", :locale => options[:locale], :raise => true)
when nil
- I18n.translate(:"number.human.decimal_units.units", :locale => options[:locale], :raise => true)
+ translate_number_value_with_default("human.decimal_units.units", :locale => options[:locale], :raise => true)
else
raise ArgumentError, ":units must be a Hash or String translation scope."
end.keys.map{|e_name| inverted_du[e_name] }.sort_by{|e| -e}
@@ -488,34 +584,47 @@ def number_to_human(number, options = {})
when String, Symbol
I18n.translate(:"#{units}.#{DECIMAL_UNITS[display_exponent]}", :locale => options[:locale], :count => number.to_i)
else
- I18n.translate(:"number.human.decimal_units.units.#{DECIMAL_UNITS[display_exponent]}", :locale => options[:locale], :count => number.to_i)
+ translate_number_value_with_default("human.decimal_units.units.#{DECIMAL_UNITS[display_exponent]}", :locale => options[:locale], :count => number.to_i)
end
- decimal_format = options[:format] || I18n.translate(:'number.human.decimal_units.format', :locale => options[:locale], :default => "%n %u")
+ decimal_format = options[:format] || translate_number_value_with_default('human.decimal_units.format', :locale => options[:locale])
formatted_number = self.number_to_rounded(number, options)
decimal_format.gsub(/%n/, formatted_number).gsub(/%u/, unit).strip
end
- def self.private_module_and_instance_method(method_name)
+ def self.private_module_and_instance_method(method_name) #:nodoc:
private method_name
private_class_method method_name
end
private_class_method :private_module_and_instance_method
- def format_translations(namespace, locale) #:nodoc:
- defaults_translations(locale).merge(translations_for(namespace, locale))
+ def format_options(locale, namespace = nil) #:nodoc:
+ default_format_options(namespace).merge!(i18n_format_options(locale, namespace))
+ end
+ private_module_and_instance_method :format_options
+
+ def default_format_options(namespace = nil) #:nodoc:
+ options = DEFAULTS[:format].dup
+ options.merge!(DEFAULTS[namespace][:format]) if namespace
+ options
end
- private_module_and_instance_method :format_translations
+ private_module_and_instance_method :default_format_options
- def defaults_translations(locale) #:nodoc:
- I18n.translate(:'number.format', :locale => locale, :default => {})
+ def i18n_format_options(locale, namespace = nil) #:nodoc:
+ options = I18n.translate(:'number.format', locale: locale, default: {}).dup
+ if namespace
+ options.merge!(I18n.translate(:"number.#{namespace}.format", locale: locale, default: {}))
+ end
+ options
end
- private_module_and_instance_method :defaults_translations
+ private_module_and_instance_method :i18n_format_options
+
+ def translate_number_value_with_default(key, i18n_options = {}) #:nodoc:
+ default = key.split('.').reduce(DEFAULTS) { |defaults, k| defaults[k.to_sym] }
- def translations_for(namespace, locale) #:nodoc:
- I18n.translate(:"number.#{namespace}.format", :locale => locale, :default => {})
+ I18n.translate(key, { default: default, scope: :number }.merge!(i18n_options))
end
- private_module_and_instance_method :translations_for
+ private_module_and_instance_method :translate_number_value_with_default
def valid_float?(number) #:nodoc:
Float(number)
42 activesupport/test/number_helper_i18n_test.rb
View
@@ -56,6 +56,13 @@ def test_number_to_currency_with_empty_i18n_store
assert_equal("-$10.00", number_to_currency(-10, :locale => 'empty'))
end
+ def test_locale_default_format_has_precedence_over_helper_defaults
+ I18n.backend.store_translations 'ts',
+ { :number => { :format => { :separator => ";" } } }
+
+ assert_equal("&$ - 10;00", number_to_currency(10, :locale => 'ts'))
+ end
+
def test_number_to_currency_without_currency_negative_format
I18n.backend.store_translations 'no_negative_format', :number => {
:currency => { :format => { :unit => '@', :format => '%n %u' } }
@@ -72,11 +79,24 @@ def test_number_with_i18n_precision
assert_equal("1.00", number_to_rounded(1.0, :locale => 'ts'))
end
+ def test_number_with_i18n_precision_and_empty_i18n_store
+ I18n.backend.store_translations 'empty', {}
+
+ assert_equal("123456789.123", number_to_rounded(123456789.123456789, :locale => 'empty'))
+ assert_equal("1.000", number_to_rounded(1.0000, :locale => 'empty'))
+ 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_with_i18n_delimiter_and_empty_i18n_store
+ I18n.backend.store_translations 'empty', {}
+
+ assert_equal("1,000,000.234", number_to_delimited(1000000.234, :locale => 'empty'))
+ end
+
def test_number_to_i18n_percentage
# to see if strip_insignificant_zeros is true
assert_equal("1%", number_to_percentage(1, :locale => 'ts'))
@@ -86,12 +106,27 @@ def test_number_to_i18n_percentage
assert_equal("12434%", number_to_percentage(12434, :locale => 'ts'))
end
+ def test_number_to_i18n_percentage_and_empty_i18n_store
+ I18n.backend.store_translations 'empty', {}
+
+ assert_equal("1.000%", number_to_percentage(1, :locale => 'empty'))
+ assert_equal("1.243%", number_to_percentage(1.2434, :locale => 'empty'))
+ assert_equal("12434.000%", number_to_percentage(12434, :locale => 'empty'))
+ 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_i18n_human_size_with_empty_i18n_store
+ I18n.backend.store_translations 'empty', {}
+
+ assert_equal("2 KB", number_to_human_size(2048, :locale => 'empty'))
+ assert_equal("42 Bytes", number_to_human_size(42, :locale => 'empty'))
+ end
+
def test_number_to_human_with_default_translation_scope
#Using t for thousand
assert_equal "2 t", number_to_human(2000, :locale => 'ts')
@@ -106,6 +141,13 @@ def test_number_to_human_with_default_translation_scope
assert_equal "2 Tens", number_to_human(20, :locale => 'ts')
end
+ def test_number_to_human_with_empty_i18n_store
+ I18n.backend.store_translations 'empty', {}
+
+ assert_equal "2 Thousand", number_to_human(2000, :locale => 'empty')
+ assert_equal "1.23 Billion", number_to_human(1234567890, :locale => 'empty')
+ 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)
7 activesupport/test/number_helper_test.rb
View
@@ -4,7 +4,7 @@
module ActiveSupport
module NumberHelper
class NumberHelperTest < ActiveSupport::TestCase
-
+
class TestClassWithInstanceNumberHelpers
include ActiveSupport::NumberHelper
end
@@ -16,7 +16,7 @@ class TestClassWithClassNumberHelpers
def setup
@instance_with_helpers = TestClassWithInstanceNumberHelpers.new
end
-
+
def kilobytes(number)
number * 1024
end
@@ -362,14 +362,13 @@ def test_number_helpers_should_return_non_numeric_param_unchanged
assert_equal "x", number_helper.number_to_human('x')
end
end
-
+
def test_extending_or_including_number_helper_correctly_hides_private_methods
[@instance_with_helpers, TestClassWithClassNumberHelpers, ActiveSupport::NumberHelper].each do |number_helper|
assert !number_helper.respond_to?(:valid_float?)
assert number_helper.respond_to?(:valid_float?, true)
end
end
-
end
end
end
2  guides/source/4_0_release_notes.textile
View
@@ -747,6 +747,8 @@ h3. Active Resource
h3. Active Support
+* Add default values to all <tt>ActiveSupport::NumberHelper</tt> methods, to avoid errors with empty locales or missing values.
+
* <tt>Time#change</tt> now works with time values with offsets other than UTC or the local time zone.
* Add <tt>Time#prev_quarter</tt> and <tt>Time#next_quarter</tt> short-hands for <tt>months_ago(3)</tt> and <tt>months_since(3)</tt>.
Something went wrong with that request. Please try again.