Introduce Zend\I18n\Filter\NumberParse based on Zend\I18n\Filter\NumberFormat #4510

localheinz · 2013-05-20T15:50:23Z

This PR is related to #4493 and replaces #4495 and introduces Zend\I18n\Filter\NumberParse which filters a string parseable by NumberFormatter to a number.

Ping @cgmartin.

localheinz · 2013-05-20T15:51:25Z

Credits for the code and tests should go to @cgmartin because everything is just based on Zend\I18n\Filter\NumberFormat.

cgmartin · 2013-05-20T16:04:37Z

Actually, @denixport gets the credit as original author. I'm just a curious bystander. :)

Also, see my comments in #4495 . Thanks!

denixport · 2013-05-20T16:40:12Z

I think that initially dual behavior of the filter was bad Idea of mine. But no way to change it because of BC.

So separate filter seems like good idea, just few notes:

keep ICU naming scheme: format/parse, so \NumberFormat does formatting of numeric value to string and NumberParse parses locale formatted string into int/double
there should be note in docs that parsing behavior of NumberFormat is discouraged or even deprecated
addition of DateFormat and DateParse filters (relatively easy to implement) will help to make clear that formatter and parser are separate filters

@denixport

As suggested by @denixport.

localheinz · 2013-05-20T17:08:23Z

@denixport Thanks for your suggestion, I will take care of it!

Got a question though: Do you think it will make more sense to change the order of inheritance? The NumberFormat filter could extend the NumberParse filter - if a number could be parsed from the string, it can be easily formatted to whatever is desired.

localheinz · 2013-05-20T17:16:41Z

@denixport For consistency reasons, either DateTimeFormatter should be called DateTimeFormat- or NumberFormat should be called NumberFormatter, then, no?

denixport · 2013-05-20T18:02:45Z

@localheinz

on inheritance, I agree, makes sense
on naming consistency, by the time this filter was created we already had Zend\I18n\View\Helper\NumberFormat & DateFormat, so I just kept it consistent with view helpers, and I think it's better to keep that way

samsonasik · 2013-05-20T20:33:30Z

tests/ZendTest/I18n/Filter/NumberParseTest.php

+ * @link      http://github.com/zendframework/zf2 for the canonical source repository
+ * @copyright Copyright (c) 2005-2013 Zend Technologies USA Inc. (http://www.zend.com)
+ * @license   http://framework.zend.com/license/new-bsd New BSD License
+ * @package   Zend_I18n


remove @Package

@samsonasik

As suggested by @samsonasik.

localheinz · 2013-05-21T11:59:55Z

library/Zend/I18n/Filter/NumberParse.php

+    public function filter($value)
+    {
+        if (!is_int($value)
+            || !is_float($value)) {


Should probably be &&.

weierophinney · 2013-05-23T16:29:51Z

library/Zend/I18n/Filter/NumberFormat.php

-        $formatter = $this->getFormatter();
-        $type      = $this->getType();
+        if (!is_int($value)
+            && !is_float($value)) {


closing paren and opening brace on next line, please:

if (!is_int($value) && !is_float($value) ) {

Introduce Zend\I18n\Filter\NumberParse based on Zend\I18n\Filter\NumberFormat

Close #4510 Fixes #4493

weierophinney · 2013-05-23T16:33:05Z

Merged to develop for release with 2.3.0.

- Move closing paren/opening brace to next line - constructor (multi-line definition) - if statement (multi-line conditional)

Related to zendframework/zf2/#4493 and zendframework/zendframework#4510

…eature/number-filter Introduce Zend\I18n\Filter\NumberParse based on Zend\I18n\Filter\NumberFormat

Close zendframework/zendframework#4510 Fixes zendframework/zendframework#4493

- Move closing paren/opening brace to next line - constructor (multi-line definition) - if statement (multi-line conditional)

…eature/number-filter Introduce Zend\I18n\Filter\NumberParse based on Zend\I18n\Filter\NumberFormat

Close zendframework/zendframework#4510 Fixes zendframework/zendframework#4493

Extend NumberFormat filter

3e9bf9d

localheinz mentioned this pull request May 20, 2013

Return parsed result when a parseable string was passed (fixes #4493) #4495

Closed

Rename Number filter to NumberParse

fbc9923

As suggested by @denixport.

Register NumberParse filter in FilterPluginManager

d6be7e0

samsonasik reviewed May 20, 2013
View reviewed changes

Remove package annotation

3de4b56

As suggested by @samsonasik.

localheinz reviewed May 21, 2013
View reviewed changes

localheinz and others added 4 commits May 21, 2013 14:04

Fix condition

bcd3607

Change order of inheritance

07bbb3b

Fix CS

30e0fbb

Fix name of test

228d1a5

weierophinney reviewed May 23, 2013
View reviewed changes

ghost assigned weierophinney May 23, 2013

weierophinney added a commit that referenced this pull request May 23, 2013

Merge pull request #4510 from localheinz/feature/number-filter

a57bb11

Introduce Zend\I18n\Filter\NumberParse based on Zend\I18n\Filter\NumberFormat

weierophinney added a commit that referenced this pull request May 23, 2013

Merge branch 'feature/4510' into develop

31a5534

Close #4510 Fixes #4493

weierophinney closed this May 23, 2013

weierophinney added a commit that referenced this pull request May 23, 2013

[#4510] CS fixes

39e621b

- Move closing paren/opening brace to next line - constructor (multi-line definition) - if statement (multi-line conditional)

localheinz mentioned this pull request May 28, 2013

[DateTimeFormatter] Format DateTime values correctly Bis #4547

Closed

localheinz added a commit to localheinz/zf2-documentation that referenced this pull request May 28, 2013

Add documentation for NumberParse filter

dedd7ef

Related to zendframework/zf2/#4493 and zendframework/zendframework#4510

localheinz mentioned this pull request May 28, 2013

Add documentation for NumberParse filter zendframework/zf2-documentation#884

Merged

This was referenced Jun 12, 2013

Date format not respected in LessThan and GreaterThan validators #4440

Closed

Include documentation for NumberParse filter zendframework/zf2-documentation#914

Closed

weierophinney added a commit to zendframework/zend-i18n that referenced this pull request May 15, 2015

Merge pull request zendframework/zendframework#4510 from localheinz/f…

ad3ef4c

…eature/number-filter Introduce Zend\I18n\Filter\NumberParse based on Zend\I18n\Filter\NumberFormat

weierophinney added a commit to zendframework/zend-i18n that referenced this pull request May 15, 2015

Merge branch 'feature/4510' into develop

261580a

Close zendframework/zendframework#4510 Fixes zendframework/zendframework#4493

weierophinney added a commit to zendframework/zend-i18n that referenced this pull request May 15, 2015

[zendframework/zendframework#4510] CS fixes

690cca8

- Move closing paren/opening brace to next line - constructor (multi-line definition) - if statement (multi-line conditional)

weierophinney added a commit to zendframework/zend-filter that referenced this pull request May 15, 2015

Merge pull request zendframework/zendframework#4510 from localheinz/f…

8e49021

…eature/number-filter Introduce Zend\I18n\Filter\NumberParse based on Zend\I18n\Filter\NumberFormat

weierophinney added a commit to zendframework/zend-filter that referenced this pull request May 15, 2015

Merge branch 'feature/4510' into develop

bd6d40b

Close zendframework/zendframework#4510 Fixes zendframework/zendframework#4493

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce Zend\I18n\Filter\NumberParse based on Zend\I18n\Filter\NumberFormat #4510

Introduce Zend\I18n\Filter\NumberParse based on Zend\I18n\Filter\NumberFormat #4510

localheinz commented May 20, 2013

localheinz commented May 20, 2013

cgmartin commented May 20, 2013

denixport commented May 20, 2013

localheinz commented May 20, 2013

localheinz commented May 20, 2013

denixport commented May 20, 2013

samsonasik May 20, 2013

localheinz May 20, 2013

localheinz May 21, 2013

weierophinney May 23, 2013

weierophinney commented May 23, 2013

Introduce Zend\I18n\Filter\NumberParse based on Zend\I18n\Filter\NumberFormat #4510

Introduce Zend\I18n\Filter\NumberParse based on Zend\I18n\Filter\NumberFormat #4510

Conversation

localheinz commented May 20, 2013

localheinz commented May 20, 2013

cgmartin commented May 20, 2013

denixport commented May 20, 2013

localheinz commented May 20, 2013

localheinz commented May 20, 2013

denixport commented May 20, 2013

samsonasik May 20, 2013

Choose a reason for hiding this comment

localheinz May 20, 2013

Choose a reason for hiding this comment

localheinz May 21, 2013

Choose a reason for hiding this comment

weierophinney May 23, 2013

Choose a reason for hiding this comment

weierophinney commented May 23, 2013