phalcon / cphalcon

High performance, full-stack PHP framework delivered as a C extension.
https://phalcon.io
BSD 3-Clause "New" or "Revised" License
10.78k stars 1.96k forks source link

Camelize delimiter in 2.0.11 #11767

Closed marcojetson closed 8 years ago

marcojetson commented 8 years ago

Before 2.0.11 camelize accepted underscore (_) and dash (-) as delimiters. In 2.0.11 only underscore is used by default and a new optional argument was added in order to add custom delimiters.

IMO this change should not be part of a minor version.

If you are keeping this change have in mind that doc block is misleading since it says it converts "dash/underscored text..."

sergeyklay commented 8 years ago

I think this issue mostly related to the Zephir project which uses the practice "pull all things in master". Phalcon always uses latest Zephir from master branch. It would be better if you opened this issue in the Zephir repository. Or your question should sounds like: "Do not use latest Zephir in Phalcon's minor versions"

mdular commented 8 years ago

Hey,

while I agree that this issue should also be directed (and resolved) at Zephir, phalcon 2.0.11 breaks several naming conventions in multiple of our projects. Under semver this shouldn't happen, and we now need to ensure that everything including dev environments is pinned to 2.0.10 instead of 2.0.x

sergeyklay commented 8 years ago

Under semver

Phalcon never followed SemVer

mdular commented 8 years ago

Ok, that underlines our realisation to ensure 2.0.10 is pinned.

As for the issue itself, I do believe it's unintentionally breaking backwards compatibility since the related change describes a different scope and the docbloc still mentions both delimiters.

Issue on zehpir: https://github.com/phalcon/zephir/issues/1252

sergeyklay commented 8 years ago

@mdular Well, I'll try to sort out with this

andresgutierrez commented 8 years ago

Fixed in 2.0.x using Zephir 0.9.2

cbhsqz commented 8 years ago

Hi @andresgutierrez @sergeyklay,

I don't think the original behavior is restored.

\Phalcon\Text::camelize('test-test') returns "TestTest" in phalcon 2.0.10. \Phalcon\Text::camelize('test-test') returns "Test-test" in phalcon 2.0.12.

Can you check?

Tested on CentOS release 6.7 (Final) php-phalcon2.x86_64 2.0.10-1.el6.remi.5.6 @remi-php56 php-phalcon2.x86_64 2.0.12-1.el6.remi.5.6 @remi-php56

<?php echo \Phalcon\Text::camelize('test-test');

sergeyklay commented 8 years ago

Will be fixed in v2.0.13

aleemb commented 8 years ago

I saw some other routing related issues. Had to revert back to 2.0.10 because anything with hyphens broke. I used to called convert() before where I did my own camel casing and returned the controller name. That is also now broken because I think because the newer version of Phalcon expects it to return a non-camelized name. So now I have have to change this:

return lcfirst(\Phalcon\Text::camelize($controller));

to this

return str_replace('-', '_', $controller);

but even though I deploy via docker ubuntu images, the above only works on production. On dev box I have to do:

return lcfirst(\Phalcon\Text::camelize(str_replace('-', '_', $controller)));

My devbox host is OS X but everything else is the same. I have some other stuff I do based on EXCEPTION_HANDLER_NOT_FOUND and EXCEPTION_ACTION_NOT_FOUND like passing on the request to a fallback handler and such, but not sure what the root cause was.

Will await 2.0.13 and try again.

sergeyklay commented 8 years ago

Fixed here https://github.com/phalcon/cphalcon/pull/11804

sergeyklay commented 8 years ago

Fixed in v2.0.13 and master