Added Number extension #27

Open
wants to merge 2 commits into
from

Conversation

Projects
None yet
6 participants
@merk

merk commented Sep 10, 2011

Added a Intl NumberFormatter filter that falls back to money_format if it is not available.

lib/Twig/Extensions/Extension/Number.php
+ $formatter = new NumberFormatter(Locale::getDefault(), NumberFormatter::CURRENCY);
+ }
+
+ return $formatter->formatCurrency($value, $currency);

This comment has been minimized.

Show comment Hide comment
@stof

stof Sep 10, 2011

Contributor

What about making the locale configurable with a third parameter (and using the default when it is not given) ?

@stof

stof Sep 10, 2011

Contributor

What about making the locale configurable with a third parameter (and using the default when it is not given) ?

@merk

This comment has been minimized.

Show comment Hide comment
@merk

merk Sep 10, 2011

I've added support to override the locale, but it seems like a bit of a hack to call setlocale() for the non Intl function. Not sure if there is a better way?

merk commented Sep 10, 2011

I've added support to override the locale, but it seems like a bit of a hack to call setlocale() for the non Intl function. Not sure if there is a better way?

+ }
+
+ if (null === $formatter) {
+ $formatter = new NumberFormatter($locale, NumberFormatter::CURRENCY);

This comment has been minimized.

Show comment Hide comment
@stof

stof Sep 10, 2011

Contributor

you should not use a static formatter as it will fail when using the method several times with different locales.

@stof

stof Sep 10, 2011

Contributor

you should not use a static formatter as it will fail when using the method several times with different locales.

This comment has been minimized.

Show comment Hide comment
@sstok

sstok Jan 5, 2013

Actually you can use a static but compare the locale of the object with the given one (that is how I do it).
Most users will not change the locale during runtime so keeping this 'cache' with a locale check is better imo.

(null === $formatter || $locale !== $formatter->getLocale())
@sstok

sstok Jan 5, 2013

Actually you can use a static but compare the locale of the object with the given one (that is how I do it).
Most users will not change the locale during runtime so keeping this 'cache' with a locale check is better imo.

(null === $formatter || $locale !== $formatter->getLocale())
@stof

This comment has been minimized.

Show comment Hide comment
@stof

stof Sep 10, 2011

Contributor

You should also add some tests

Contributor

stof commented Sep 10, 2011

You should also add some tests

@merk

This comment has been minimized.

Show comment Hide comment
@merk

merk Sep 10, 2011

I will add some tests and a decimal formatter tomorrow.

On Sat, Sep 10, 2011 at 19:40, Christophe Coevoet <
reply@reply.github.com>wrote:

You should also add some tests

Reply to this email directly or view it on GitHub:
fabpot#27 (comment)

merk commented Sep 10, 2011

I will add some tests and a decimal formatter tomorrow.

On Sat, Sep 10, 2011 at 19:40, Christophe Coevoet <
reply@reply.github.com>wrote:

You should also add some tests

Reply to this email directly or view it on GitHub:
fabpot#27 (comment)

@Seldaek

This comment has been minimized.

Show comment Hide comment
@Seldaek

Seldaek Sep 21, 2011

Contributor

Could you also take in the changes from #14 (cc @falmp) and consolidate it all in one extension?

Contributor

Seldaek commented Sep 21, 2011

Could you also take in the changes from #14 (cc @falmp) and consolidate it all in one extension?

@stof

This comment has been minimized.

Show comment Hide comment
@stof

stof Oct 16, 2011

Contributor

@merk ping ?

also, the implementation using intl could probably be added in the Intl extension I submitted in #30 instead

Contributor

stof commented Oct 16, 2011

@merk ping ?

also, the implementation using intl could probably be added in the Intl extension I submitted in #30 instead

@merk

This comment has been minimized.

Show comment Hide comment
@merk

merk Oct 16, 2011

@stof: In hopsital, wont get a chance to do anything. SonataIntlBundle provides everything I need so I probably wont be dedicating time to this especially since I dont need 5.2 support.

merk commented Oct 16, 2011

@stof: In hopsital, wont get a chance to do anything. SonataIntlBundle provides everything I need so I probably wont be dedicating time to this especially since I dont need 5.2 support.

@jnonon

This comment has been minimized.

Show comment Hide comment
@jnonon

jnonon Nov 22, 2011

Any update in this feature?

jnonon commented Nov 22, 2011

Any update in this feature?

@merk

This comment has been minimized.

Show comment Hide comment
@merk

merk Nov 22, 2011

If you dont need 5.2, https://github.com/sonata-project/SonataIntlBundle has NumberFormatter support.

merk commented Nov 22, 2011

If you dont need 5.2, https://github.com/sonata-project/SonataIntlBundle has NumberFormatter support.

+}
+
+if (class_exists('NumberFormatter')) {
+ function twig_format_currency($value, $currency, $locale = null)

This comment has been minimized.

Show comment Hide comment
@stof

stof Jan 4, 2013

Contributor

This part should be removed in favor of #55 IMO

@stof

stof Jan 4, 2013

Contributor

This part should be removed in favor of #55 IMO

@jrobeson

This comment has been minimized.

Show comment Hide comment
@jrobeson

jrobeson Aug 9, 2014

a localizedcurrency and localizednumber filter has been merged. what should happen here now?

jrobeson commented Aug 9, 2014

a localizedcurrency and localizednumber filter has been merged. what should happen here now?

@stof

This comment has been minimized.

Show comment Hide comment
@stof

stof Aug 12, 2014

Contributor

@jrobeson this PR should be closed IMO, as localizedcurrency is the same than what is done here when Intl is available.

Contributor

stof commented Aug 12, 2014

@jrobeson this PR should be closed IMO, as localizedcurrency is the same than what is done here when Intl is available.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment