[WIP - help] Allow to remove decimals in CurrencyFormat #2318

Closed
wants to merge 2 commits into
from

3 participants

@bakura10

This PR adds an option to CurrencyFormat view helper to remove the two decimals number (I often need to simply prints the integer number).

HOWEVER it looks like NumberFormatter has a bug (I tried to read the unit tests of PHP and none of them covered the case, and I have not been able to find any help anywhere on the Internet).

The problem is that depending on the locale, it does not remove the decimals digits. This option sets the NumberFormatter::FRACTION_DIGITS attribute to 0, which should remove the decimals.

For instance, when setting the locale to "en_US", this $numberFormatter->formatCurrency('45', 'USD') prints "$45". However, when keeping the same locale (en-US) but chaning the currency to 'EUR', it prints "€45.00". At the same time, if I change the locale to "fr-FR", and prints a number with currency "EUR", it prints 45 €, and thus removing the decimals.

I've tried the money_format function of PHP and it does not suffer from this problem. So it really looks like Intl implementation is buggy, but at the same time, it works with some locales, and for other locales, it just does nothing. So we may merge (or not ?).

If anyone have some thoughts about it... (side note : rounding the number before formatting does nothing).

@weierophinney weierophinney reopened this Sep 14, 2012
@weierophinney
Zend Framework member

@bakura10 Have you had any feedback on this yet? You might try posting to zf-contributors, or asking in IRC.

@bakura10

Unfortunately no,I didn't get any feedback yet. Dasprid found this strange too, StackOverflow didn't help too, php channel on IRC too when I asked. This is bad because I really need that.

@bakura10

Still no news about this one. I opened a bug report on PHP.net but nothing...

I think I'll have to have my own helper just for that...

Just a test : does anyone have a newest version of ICU data (I have 4.6, it appears that there is 4.8). Could someone test ?

@bakura10

I just received an answer about this. It seems this is a bug from ICU itself (https://bugs.php.net/bug.php?id=63140)

So I think we can merge this. At least it works for some locales, and it may even be already corrected for latest versions of ICU (anyway, the logic behind the PR is good).

@weierophinney weierophinney added a commit that referenced this pull request Oct 4, 2012
@weierophinney weierophinney Merge branch 'hotfix/2318' into develop
Forward port #2318
2550eb4
@weierophinney weierophinney added a commit that closed this pull request Oct 4, 2012
@weierophinney weierophinney Merge branch 'hotfix/2318'
Close #2318
b1a862f
@weierophinney weierophinney was assigned Oct 4, 2012
@ghost Unknown pushed a commit that referenced this pull request Jul 14, 2013
@weierophinney weierophinney Merge branch 'hotfix/2318'
Close #2318
c10c50b
@ghost Unknown pushed a commit that referenced this pull request Jul 14, 2013
@weierophinney weierophinney Merge branch 'hotfix/2318' into develop
Forward port #2318
a08d05a
@intellix

Do we know which version of PHP the ICU fix is provided in? I know it's nothing to do with ZF2 but I keep ending up here at this issue but don't have anymore information.

@bakura10

ICU is independant from PHP. You can install the latest ICU version even with an old PHP 5.3 version. You can do a phpinfo() to check your version of ICU. I think the latest is 4.6.x or 4.7.x.

@demichl68 demichl68 added a commit to demichl68/zf2 that referenced this pull request Jul 30, 2014
@demichl68 demichl68 Provided failing tests for CurrencyFormat helper
referencing zendframework#2318
We're on icu version 51.2 and still have the problem of wrong formatting for some locale and currency code combination

Please run this test and verify the results
17ab1ef
@weierophinney weierophinney added a commit to zendframework/zend-i18n that referenced this pull request May 15, 2015
@weierophinney weierophinney Merge branch 'hotfix/2318' c2a11e1
@weierophinney weierophinney added a commit to zendframework/zend-i18n that referenced this pull request May 15, 2015
@weierophinney weierophinney Merge branch 'hotfix/2318' into develop 2333225
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment