#1938670: Convert unicode.inc to \Drupal\Component\Utility\Unicode comes with some PHP function wrappers. Unicode::truncate(), Unicode::mimeHeaderEncode(), etc. Some of them arn't living under the naming conventions:
Unicode::ucfirst()Unicode::substr()
Although it is nice to have PHP wrappers be similarly named to the PHP functions, it's even nicer to have consistent coding standards.
Possible solutions:
- Fix the case of the functions:
Unicode::ucFirst(),Unicode::subStr(), etc. - Rename the functions entirely:
Unicode::upperCaseFirst(),Unicode::subString(), etc. - Leave as is and live with the broken coding standards
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | core-funcname-codestyle-1985104.patch | 5.15 KB | jameswoods |
Comments
Comment #1
ParisLiakos commentedso, even if these methods stay, we should update our standards for that. eg for php standard function wrappers do that.
my +1 goes to option 2
Comment #2
ParisLiakos commentedComment #3
jameswoods commentedFirst time contributor here. Using option #2 "Rename the functions entirely: Unicode::upperCaseFirst(), Unicode::subString(), etc." per ParisLiakos' suggestion in comment #1.
Comment #4
jameswoods commentedI began running the test on my local machine...and it was taking forEVER...so I'm going to upload and leverage the testbots 8^)
Comment #5
jameswoods commentedComment #7
jameswoods commentedIt seems like the failure is:
Fatal error: Call to undefined method Drupal\Component\Utility\Unicode::substr() in /var/lib/drupaltestbot/sites/default/files/checkout/core/lib/Drupal/Component/Utility/Unicode.php on line 467
FATAL Drupal\aggregator\Tests\UpdateFeedItemTest: test runner returned a non-zero error code (255).
I'm not sure how to fix that...the file with the error lives in sites/default/files/..... Do I need to fix the tests or something?
Comment #8
jhodgdonNo, I think you are perhaps misinterpreting the error message. I think there are calls in Core to the old function/method names that need to be replaced with your new names.
For instance, this other error:
PHP Fatal error: Call to undefined method Drupal\Component\Utility\Unicode::strtolower() in /var/lib/drupaltestbot/sites/default/files/checkout/core/modules/system/lib/Drupal/system/MachineNameController.php on line 67
That looks like it is probably in core/modules/system/lib/Drupal/system/MachineNameController.php. Your patch didn't seem to touch other files in core outside of the Unicode classes, but presumably those classes are being used?
Comment #9
mile23See also: #2042625: Unicode::substr() isn't really \substr() like the docs say.
Comment #9.0
mile23a
Comment #13
mile23The current methods would need to be deprecated as per the deprecation policy: https://www.drupal.org/core/deprecation
Bump up to 8.4.x because of disruption.
Comment #26
quietone commented