Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use a string wrapper to support intl, mbstring and iconv extension #131

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sandrokeil
Copy link

Since the PHP intl extension is available, there is no need to use mbstring anymore. Websites uses UTF-8 and PHP intl extension is more comfortable than mbstring. I've replaced all mb_* functions with the grapheme_* functions.

@beberlei What do you think about a new major version which uses the PHP intl extension?

@beberlei
Copy link
Owner

@sandrokeil I am unsure, it must be a new major version that is for sure, but i don't know if intl is as widespread as mbstring.

@@ -76,7 +76,6 @@
* @method static void nullOrChoicesNotEmpty($values, $choices, $message = null, $propertyPath = null)
* @method static void nullOrMethodExists($value, $object, $message = null, $propertyPath = null)
* @method static void nullOrIsObject($value, $message = null, $propertyPath = null)
* @method static void nullOrDate($value, $format, $message = null, $propertyPath = null)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this removal looks suspicious to me

@sandrokeil
Copy link
Author

@stof I'm not sure why this was happened. I've restored it. Thanks.

@beberlei I thought that the Intl extension replaces mbstring and is mainly used, if projects uses i18n. In my case I didn't need mbstring because I rely on the Intl extension. I don't know which one is more widespread.

@sandrokeil
Copy link
Author

@beberlei We can use a simple String Wrapper like Zend and we have a WinWin solution and can bring a new minor release.

Maybe even simpler, check for the extensions on the appropriate functions and call the correct method. What do you think? The order is intl, mbstring and iconv.

I will update the PR if you accept this.

@sandrokeil sandrokeil changed the title use PHP intl extension instead of mbstring use a string wrapper to support intl, mbstring and iconv extension Apr 21, 2016
@sandrokeil
Copy link
Author

ping @beberlei +1 for string wrapper and full backward compatibility? We will support intl, mbstring, iconv and native. No method signature will be changed, but the encoding is only used by mbstring and iconv.

@bweston92
Copy link

👍

@@ -132,7 +132,6 @@
* @method static void allChoicesNotEmpty($values, $choices, $message = null, $propertyPath = null)
* @method static void allMethodExists($value, $object, $message = null, $propertyPath = null)
* @method static void allIsObject($value, $message = null, $propertyPath = null)
* @method static void allDate($value, $format, $message = null, $propertyPath = null)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another removal?

Are you using bin/generate_method_docs.php to update the docblocks?

@bweston92
Copy link

any update?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants