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

Camelize delimiter in 2.0.11 #11767

Closed
marcojetson opened this issue May 9, 2016 · 11 comments
Closed

Camelize delimiter in 2.0.11 #11767

marcojetson opened this issue May 9, 2016 · 11 comments
Assignees
Labels
bug A bug report status: medium Medium
Milestone

Comments

@marcojetson
Copy link

marcojetson commented May 9, 2016

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
Copy link
Contributor

sergeyklay commented May 9, 2016

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
Copy link

mdular commented May 9, 2016

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
Copy link
Contributor

Under semver

Phalcon never followed SemVer

@mdular
Copy link

mdular commented May 9, 2016

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: zephir-lang/zephir#1252

@sergeyklay
Copy link
Contributor

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

@andresgutierrez
Copy link
Contributor

Fixed in 2.0.x using Zephir 0.9.2

@cbhsqz
Copy link

cbhsqz commented May 17, 2016

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
Copy link
Contributor

Will be fixed in v2.0.13

@aleemb
Copy link

aleemb commented May 18, 2016

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
Copy link
Contributor

Fixed here #11804

@sergeyklay
Copy link
Contributor

Fixed in v2.0.13 and master

@niden niden added bug A bug report status: medium Medium and removed Bug - Medium labels Dec 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A bug report status: medium Medium
Projects
None yet
Development

No branches or pull requests

7 participants