Add apostrophe as thousands marker

Firsts add in D8.
Added a patch for D8 & D7.

We usually do "'" instead of '\'', but I'm not sure that warrants a re-roll.

Yeah!

Ops! Too fast!

Thanks.
RTBC if it comes back green.
I didn't know that the Swiss do this, but I checked. For reference, this is a page on the website on of the biggest Swiss newspapers: http://www.nzz.ch/finanzen/indizes/_detail?ID_NOTATION=1555183&ID_INSTRU...

While looking at this, should the Arabic separators be added too?

U+066B ٫ Arabic Decimal Separator
U+066C ٬ Arabic Thousands Separator

And there is also the thin space recommended by many agencies:

U+2009   thin space (HTML:    ).

IE: 123 456 789 [space] and 123 456 789 [thin space]

Ok, I'm going to add all the options.

Supply two patches (or one new one) as the core maintainers may or may not like the extended options :)

My system it's working fine with the Thin space but I have problems writing the arabic characters. For example, if I use chr() to print these characters:

echo 'Arabic Decimal Separator:' . chr(1644);
echo '<br />';
echo 'Arabic Thousands Separator:' . chr(1643);

In my system it appear as:

Arabic Decimal Separator:l
Arabic Thousands Separator:k

"l" and "k"! WTF! O.o

Any idea of how to represent correctly the arabic characters? Maybe a problem with my system configuration?

Added 3 patches: Only apostrophe addition, apostrophe + thin space addition, apostrophe + thin space + arabic separators addition

Ops! All separators patch. Note that the arabic characters are not representing correctly in my local system.

Ok. We have some representation problems with Arabic separators, so move forward and put the two new thousand separators (apostrophe for Swiss and thin space as recommend some agencies). RTBC?

Apostrophe and thin space make sense. Let's add them.

Committed to 8.x. Moving this to 7.x; we could backport this one.

Yeah!!! This is the port to D7!

Manually tested. Straight forward. This small patch is OK for D7.

Um. Was the thin space tested?

+++ b/core/modules/field/modules/number/number.module
@@ -228,6 +228,8 @@ function number_field_formatter_settings_form($field, $instance, $view_mode, $fo
+      chr(8201) => t('Thin space'),

http://php.net/manual/en/function.chr.php clearly documents the argument as $ascii. The maximum value is 255.

This user comment explains the behavior of chr() for values > 255 and < 255:
http://php.net/manual/en/function.chr.php#91868

And this is also what happens:

var_dump(chr(8201));      // string(1) "	"
var_dump(ord(chr(8201))); // int(9)
var_dump(chr(9));         // string(1) "	"

And as http://www.asciitable.com nicely clarifies, that's a TAB, not a thin-space. :)

Ok, first of all remove the thin space option.

We are only removing a new option that it's adding a bug, after the commit we can discuss how to add it properly.

I think it would make more sense to simply replace the chr() with the appropriate Unicode character.

That said, I also wonder whether the apostrophe character is the "real" typographic apostrophe that is used as thousand separator. I almost guess that there is a dedicated Unicode character for that, too.

And that explains why the Arabic characters were failing.

Maybe html_entity_decode() rather than using the multibyte string library? (if we want to support these) Note that the encoding parameter (the 3rd) is ISO-8859-1 in versions of PHP prior to 5.4.0, and UTF-8 from PHP 5.4.0

$thinspace = html_entity_decode(' ', ENT_NOQUOTES, 'UTF-8');
$arabic_decimal_sep = html_entity_decode('٫', ENT_NOQUOTES, 'UTF-8');
$arabic_thousand_sep = html_entity_decode('٬', ENT_NOQUOTES, 'UTF-8');

Added all separators in #26 and #10, not tested.

And this is the option using html_entity_decode instead of the unicode character. Maybe we can test both #28 and #29 with PHP 5.4 and PHP < 5.4, and add code comments if it's needed.

@sun do you know of any issues directly including the characters? These files should be encoded in UTF-8, but if someone saves these in another format, then there may be issues. It seems cleaner than decoding..., although the thin space doesn't show in my editors font, making space and thin space look identical in the UI.

All files in Drupal are in UTF-8. We use the Unicode characters directly everywhere. So #29 is the way to go.

There is some risk in that a developer might use an editor or IDE that doesn't handle UTF-8 correctly, but the only way I could think of to protect ourselves against that would be a unit test for number_field_formatter_settings_form(), which mocks the function parameters and just ensures that the different options have the correct values.

At this point, that feels a bit paranoid to me, so I'd rather leave that test to the hypothetical bug report of the future that fixes the characters because someone screwed them up (if any).

I have tested the patches in #28 and #29 with Apache/2.2.22 (Win32) PHP/5.3.13

The HTML form is correctly created:

But the Thin space, Right single quotation mark, Arabic thousands separator and Arabic decimal separator are not working.

No errors in the screen or in the log but when I select one of this values and save, the action is not performed and the value is not saved. When I select apostrophe, space, comma... the action is performed, the value saved and correctly displayed in the field example :(

When I disabed javascript the save action is performed but the non-ascii characters are not displayed propertly.

Server configuration problem???

Um. Was the thin space tested?

Yes, I tested it visually. I saw a space. Actually, the chr(8201) was transformed into char(9), a tab. That explains visual and misleading space. I did not inspect the byte values. So, a little bit too confident.

This is really more complicated than I initially thought.

In between I researched about Unicode.

But the Thin space, Right single quotation mark, Arabic thousands separator and Arabic decimal separator are not working.

I think the reason is the PHP function number_format().

Changelog
5.4.0 This function now supports multiple bytes in dec_point and thousands_sep. Only the first byte of each separator was used in older versions.

All these characters are Unicode, thus multi-byte. If only the first byte is taken, then we get an illegal UTF-8 string.
Thus, Thin space, Right single quotation mark, Arabic thousands separator and Arabic decimal separator do not work with PHP 5.3. PHP 5.4 is required for them.

For PHP 5.3, we have to stick we the plain apostrophe. Actually, the initial request.

Could you apply the finished part of the patch to D7?

Thanks.

Rerolled the patch

Number module was removed, so this patch will not work.

The relevant code is now in \Drupal\Core\Field\Plugin\Field\FieldFormatter\NumericFormatterBase::settingsForm() (for the thousand separator) and in \Drupal\Core\Field\Plugin\Field\FieldFormatter\DecimalFormatter::settingsForm() (for the decimal separator).

Adding tag back.

Re-rolled.

#41 still applies. Removing the "Needs reroll" tag in preparation for Drupalcon Los Angeles sprints.

The patch in #41 is not against Drupal 8.

(More specifically, it actually is against Drupal 8, but contains the wrong code.)

See #39 for what needs to be done.

It seems it has been fixed in 8.0.x so it just needs backport. Changing version to 7.x-dev (not sure if that's correct)

Here is a patch for D7.

Still not available in 7.x, but patch #48 is not correct (right parent not properly placed). The fixed attached patch works for me.