8000 [StringUtil] Fixed singularification of 'selfies' by pmodin · Pull Request #14452 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[StringUtil] Fixed singularification of 'selfies' #14452

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Apr 27, 2015
Merged

[StringUtil] Fixed singularification of 'selfies' #14452

merged 1 commit into from
Apr 27, 2015

Conversation

pmodin
Copy link
Contributor
@pmodin pmodin commented Apr 22, 2015
Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR n/a

Related to #14191.

At tonights PHP Stockholm meetup we discussed #14191, and we noticed that "selfie" was incorrectly handled as well. One selfie, many selfies.

@javiereguiluz
Copy link
Member

@pmodin thanks for fixing this problem.

As you know, languages designed for humans are a fractal of bad design and they are plagued with exceptions. That's why some of us want to remove the "magic pluralizator" implemented by Symfony. Hopefully getting more and more issue reports and pull requests for this feature will also make the other developers change their opinion about this.

@pmodin
Copy link
Contributor Author
pmodin commented Apr 23, 2015

@javiereguiluz yes, I've seen some discussions about this, most recently in #14189. I haven't been in the loop enough recently to argue on how to progress, but you, as well as the others, do have valid points. I unfortunately have nothing further to add to that discussion right now.

@mpdude
Copy link
Contributor
mpdude commented Apr 23, 2015

@javiereguiluz I agree with you, but don't know what more I could do a.t.m. 😒

@stof
Copy link
Member
stof commented Apr 23, 2015

@javiereguiluz until we find a way to provide explicitly the singular names for the adder and remover methods (see #5013 for the discussion about that) , we cannot drop the existing singularization (you will see even more complains if you ask people to define addElements and removeElements to add and remove a single element, because this makes the code totally confusing, breaking DX)

vrnagy
vrnagy reviewed Apr 23, 2015
View reviewed changes
src/Symfony/Component/PropertyAccess/StringUtil.php
@@ -60,6 +60,9 @@ class StringUtil
// indices (index), appendices (appendix), prices (price)
array('seci', 4, false, true, array('ex', 'ix', 'ice')),

// selfies (selfie)
array('seifles', 7, true, true, 'selfie'),
Copy link

Choose a reason for hiding this comment

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

seifles should be selfies

Copy link
Member

Choose a reason for hiding this comment

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

I admit that it looks very weird, but in this class those words are spelled backwards. So it's "correct" to use seifles here.

@fabpot
Copy link
Member
fabpot commented Apr 27, 2015

Thank you @pmodin.

@fabpot fabpot merged commit 7c2b875 into symfony:2.3 Apr 27, 2015
fabpot added a commit that referenced this pull request Apr 27, 2015
@fabpot
minor #14452 [StringUtil] Fixed singularification of 'selfies' (pmodin)
94cf4ba 
This PR was merged into the 2.3 branch.

Discussion
----------

[StringUtil] Fixed singularification of 'selfies'

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

Related to #14191.

At tonights PHP Stockholm meetup we discussed #14191, and we noticed that "selfie" was incorrectly handled as well. One selfie, many selfies.

Commits
-------

7c2b875 [StringUtil] Fixed singularification of 'selfies'
@pmodin pmodin deleted the selfie_support branch April 28, 2015 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@pmodin @javiereguiluz @mpdude @stof @fabpot @vrnagy
0