Skip to content

Commit

Permalink
fix previous commit
Browse files Browse the repository at this point in the history
  • Loading branch information
@semmons99
semmons99 committed Apr 7, 2011
1 parent 7e4897f commit b2cab76
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 42 deletions.
20 changes: 16 additions & 4 deletions lib/money/money.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -712,7 +712,7 @@ def symbol
currency.symbol || "¤"
end

# If I18n is loaded, looks up key +:number.currency.format.separator+.
# If I18n is loaded, looks up key +:number.currency.format.delimiter+.
# Otherwise and as fallback it uses +Currency#thousands_separator+.
# If +nil+ is returned, default to ",".
#
Expand All @@ -722,7 +722,13 @@ def symbol
# Money.new(100, "USD").thousands_separator #=> ","
if Object.const_defined?("I18n")
def thousands_separator
I18n.t(:"number.currency.format.separator", :default => currency.thousands_separator || ",")
I18n.t(
:"number.currency.format.delimiter",
:default => I18n.t(
:"number.format.delimiter",
:default => (currency.thousands_separator || ",")
)
)
end
else
def thousands_separator
Expand All @@ -731,7 +737,7 @@ def thousands_separator
end
alias :delimiter :thousands_separator

# If I18n is loaded, looks up key +:number.currency.format.delimiter+.
# If I18n is loaded, looks up key +:number.currency.format.separator+.
# Otherwise and as fallback it uses +Currency#decimal_mark+.
# If +nil+ is returned, default to ",".
#
Expand All @@ -741,7 +747,13 @@ def thousands_separator
# Money.new(100, "USD").decimal_mark #=> "."
if Object.const_defined?("I18n")
def decimal_mark
I18n.t(:"number.currency.format.delimiter", :default => currency.decimal_mark || ".")
I18n.t(
:"number.currency.format.separator",
:default => I18n.t(
:"number.format.separator",
:default => (currency.decimal_mark || ".")
)
)
end
else
def decimal_mark
Expand Down
78 changes: 40 additions & 38 deletions spec/money_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -602,47 +602,49 @@ def to_money
Money.empty(currency).symbol.should == "¤"
end

{
:thousands_separator => { :default => ",", :other => "." },
:decimal_mark => { :default => ".", :other => "," }
}.each do |method, options|
describe "##{method}" do
context "without I18n" do
it "works as documented" do
Money.empty("USD").send(method).should == options[:default]
Money.empty("EUR").send(method).should == options[:other]
Money.empty("BRL").send(method).should == options[:other]
end
context "without i18n" do
subject{ Money.empty("USD") }

its(:thousands_separator){should == ","}
its(:decimal_mark){should == "."}
end

context "with i18n" do
after :each do
reset_i18n
I18n.locale = :en
end

context "with number.format.*" do
before :each do
reset_i18n
I18n.locale = :de
I18n.backend.store_translations(
:de, :number => {:format => {:delimiter => ".",
:separator => ","}}
)
end

if Object.const_defined?("I18n")
context "with I18n" do
before :all do
reset_i18n
store_number_currency_formats(:de)
end

it "looks up current locale as documented" do
I18n.locale = :en
Money.empty("USD").send(method).should == options[:default]
I18n.locale = :de
Money.empty("USD").send(method).should == options[:other]
end

it "fallbacks to default behaviour for missing translations" do
I18n.locale = :de
Money.empty("USD").send(method).should == options[:other]
I18n.locale = :fr
Money.empty("USD").send(method).should == options[:default]
end

after :all do
reset_i18n
end
end
else
puts "can't test ##{method} with I18n because it isn't loaded"
subject{ Money.empty("USD") }

its(:thousands_separator){should == "."}
its(:decimal_mark){should == ","}
end

context "with number.currency.format.*" do
before :each do
reset_i18n
I18n.locale = :de
I18n.backend.store_translations(
:de, :number => {:currency => {:format => {:delimiter => ".",
:separator => ","}}}
)
end

subject{ Money.empty("USD") }

its(:thousands_separator){should == "."}
its(:decimal_mark){should == ","}
end
end

Expand Down

4 comments on commit b2cab76

@hakanensari
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has the potential to break (and in my case, broke) existing code. The latest release deserved a major version bump.

http://semver.org/ <<<<

@semmons99
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we tested this, it still worked with the old lookup using number.format.delimiter, etc. Are you saying this isn't the case. If so, can you provide us a bug report. It's not really helpful to tell someone "this has the potential to break..." and not provide an example of how. And honestly, when someone leaves a comment with no details and acts like we don't know how to version stuff, it's likely we just won't respond. It's not helpful.

@hakanensari
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, was a bit curt at the end of a brain-frying day.

To make a long story short, we have a Rails app that uploads inventory files to various countries in multiple currencies. Prior to 3.6.2, a thousand Euros formatted as €1.000,00. With 3.6.2, it formats as €1,000.00 (because the application locale is English?).

As incredulous as it may sound, this change would have called for a major version bump.

@semmons99
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The patch was done to read from "number.currency.format." for i18n instead of "number.format." So it shouldn't have changed anything for you unless the two are set differently. Can you open an issue for this, it will be easier to track and we can get @weppos involved who works on this project and the rails-i18n project and might have more insite on this.

Please sign in to comment.