Revert CHF format changes introduced in v6.14

[kennyadsl]

As described in #966, we noticed that the default format for the CHF currency changed with the last version released. This PR restores the previous format and, I think, needs to be part a patch release to ensure backward compatibility.

If accepted, I can send another PR to update the CHANGELOG including this in the next patch level (or making the change in this PR directly if #968 will be merged before).

[antstorm]

@kennyadsl sorry for the breaking change. I wonder if setting for format to just %n%u (without space) would have the same effect and not fallback to the legacy behaviour? Unless I'm missing something here.

Basically that change was not supposed to be breaking and should have retained the same formatting, but expressed using a new attribute. However looks like CHF has slipped.

[kennyadsl]

@antstorm no need to say sorry and thanks for the answer. 🙏

I wonder if setting for format to just %n%u (without space) would have the same effect and not fallback to the legacy behaviour?

The issue with CHF is the opposite, the new version is actually removing the space between CHF and the amount.

Old version:

Money.new(1234, currency: 'CHF') # => CHF 1,234.00

New version:

Money.new(1234, currency: 'CHF') # => CHF1,234.00

If you look at the changes to the currencies introduced in #943, you'll notice that CHF is the only currency that has the format specified and symbol_first set to true (here's an example).

I think this was done by mistake and/or it has been done to have a currency to test in the specs, see here. That's why I created the new CHF_WITH_FORMAT currency in the specs: we can keep the tests for the format+ symbol_first context without compromise the behavior of a real currency, since we don't have that specific scenario for any of the currencies defined in the iso files.

It's a new domain for me and I know I may be missing something important here so, if needed, feel free to point it out and I'll adjust the PR accordingly.

Thanks again!

[antstorm]

Not sure I'm following here, this is what I'm getting:

>> Money.locale_backend = nil
=> nil
>> Money::VERSION
=> "6.14.0"
>> Money.new(1234, 'CHF').format
=> "CHF 12.34"

This is what your "old version" example is showing and seems to be the correct behaviour — %u %n is applied as expected. What am I missing?

[antstorm]

Oh, ok the older version does not have a space:

>> Money.locale_backend = nil
=> nil
>> Money::VERSION
=> "6.13.8"
>> Money.new(1234, 'CHF').format
=> "CHF12.34"

But you can get the same behaviour with the new version:

>> Money::VERSION
=> "6.14.0"
>> Money.new(1234, 'CHF').format
=> "CHF 12.34"
>> Money.new(1234, 'CHF').format(format: '%u%n')
=> "CHF12.34"

So basically you need to update the format in the currency definition from %u %n (with space) to %u%n (no space)

[kennyadsl]

Thanks for the feedback. I think you are right, it's the other way around and we have some configuration that for some reason swaps the space between versions, I still can't get which one though.

Anyway, I think that removing the format entirely in the currency definition has the same effect since %u%n should be the default for currencies with symbol_first: true, isn't it?

[antstorm]

@kennyadsl yes, but the intention is to get rid of symbol_first in the long run and other settings that can just be replaced with format, which gives you much more flexibility in configuring formatting rules.

This is why the change was applied in the first place. Happy to accept a PR fixing the incorrect format for CHF

[kennyadsl]

@antstorm Thanks for the feedback, I made the change in this PR directly.

I still don't get why CHF is the only currency with symbol_first: true and format set, but it fixes our issue and if it makes sense for you I'm sure it's just me missing something here. Thanks!

[kennyadsl]

Hey @antstorm, any feedback on my latest changes? 🙏

[antstorm]

Looks good, thank you for contributing and addressing my comments 👍

[kennyadsl]

Thank you for guiding me! I think this PR can also be merged now. 🙏

[kennyadsl]

I forgot the link in my previous message 🤦 , I meant this PR.

[kennyadsl]

@antstorm Hey there, what about releasing a new version for this?

[antstorm]

@kennyadsl pushed 6.14.1, sorry for the delay