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

[PropertyAccess] Add french singular for words like typologies > typologie #13721

Closed
wants to merge 2 commits into from

Conversation

CaporalDead
Copy link

Hi,

We just encountered a problem while using 'by_reference' attribute as false in a form which caused a NoSuchPropertyException because the StringUtil class in PropertyAccess doesn't handle correctly the case when for example you have "Typologies" (plural) that should be translated into "Typologie" and not "Typology".

I just add this case and a test case which covered the word "typologies" and it seems that it doesn't break other test cases.

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #13723
License MIT
Doc PR [PropertyAccess] Add french singular for words like typologies > typologie

@xabbuh
Copy link
Member

xabbuh commented Feb 17, 2015

I'm not sure if the StringUtils class was designed to work well with other languages than English. Or is "typologie" a French word that is frequently used in English and I am not aware of this fact?

@ghost
Copy link

ghost commented Feb 17, 2015

it would be typology -> typologies in english

@wouterj
Copy link
Member

wouterj commented Feb 17, 2015

I'm -1 when this is adding support for other languages than French. Supporting plain English pluralization/singularization is already hard enough, adding support for even more languages makes it impossible.

Anyway, thank you for taking the time to fill in a bug report and create a PR (including PR template)!

@javiereguiluz
Copy link
Member

@wouterj what do you think about removing this "pluralization black-magic" even for English. What's the problem of writing some methods using bad English grammar in exchange of having zero problems, zero magic and a nice performance bump?

@wouterj
Copy link
Member

wouterj commented Feb 17, 2015

There is no magic, just plain english rules.

@javiereguiluz
Copy link
Member

@wouterj anything related to languages created by humans is magic, because they are as irregular as they can get. By the way, the "magic pluralizator" used by Symfony (and used too by other frameworks) may be lacking some irregular plurals found on this list: http://en.wikipedia.org/wiki/English_plurals#Irregular_plurals

@fabpot
Copy link
Member

fabpot commented Feb 18, 2015

Closing as we only support English here. Actually, I think that using French (or any other language) in method names is a big mistake as it makes reading the code really strange and messy.

@javiereguiluz I agree with you and I was against adding such a thing in Symfony, but well, we need to support it now.

@fabpot fabpot closed this Feb 18, 2015
@webda2l
Copy link

webda2l commented Feb 18, 2015

Remove this difficult magic in 3.0 will be appreciate.

@wouterj
Copy link
Member

wouterj commented Feb 18, 2015

@webda2l and then not support pluralization? I'm afraid that'll break too much code...

@stof
Copy link
Member

stof commented Feb 18, 2015

@webda2l the issue is that using addMembers and removeMembers to add or remove a single member (if we don't singularize the property name) would mean that the PropertyAccess component forces you to define methods with broken names for adders and removers. This is the reason why we have this logic in place

@webda2l
Copy link

webda2l commented Feb 18, 2015

Yes recurrent debate #9205 #5013

When magic doesn't work well, and PRs likes #13618 and #13714 are needed, facts that broken names for adders/removers are less important in my mind...

Or maybe find alternative to add/remove naming that broke less name, plus/minus? ...
Or if someone know better alternative with other frameworks.

@CaporalDead
Copy link
Author

Thanks for replying, still I think that there should be a way to tell to symfony to use another translator.

@fabpot I understand the fact that it should handle everything or nothing or a way to tell to symfony to use a specific getter/setter maybe. Using english can be very usefull sometimes but when you work with a french customer that have specific words related to his business, translate them can be confusing for your team or for him.

We'll stick with our fork for the moment until 2.7 is released.

Thanks all.

@wouterj
Copy link
Member

wouterj commented Feb 18, 2015

I think we should add support like proposed in this PR (but then reverted before merging): #3819

lrlopez added a commit to lrlopez/symfony that referenced this pull request Jul 6, 2015
| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | symfony#13721, symfony#13723
| License       | MIT
| Doc PR        | WIP
lrlopez added a commit to lrlopez/symfony that referenced this pull request Jul 6, 2015
| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | symfony#13721, symfony#13723
| License       | MIT
| Doc PR        | WIP

- [ ] Document new feature
lrlopez added a commit to lrlopez/symfony that referenced this pull request Sep 11, 2015
| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | symfony#13721, symfony#13723
| License       | MIT
| Doc PR        | WIP

- [ ] Document new feature
lrlopez added a commit to lrlopez/symfony that referenced this pull request Sep 11, 2015
| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | symfony#13721, symfony#13723
| License       | MIT
| Doc PR        | WIP

- [ ] Document new feature
lrlopez added a commit to lrlopez/symfony that referenced this pull request Sep 11, 2015
| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | symfony#13721, symfony#13723
| License       | MIT
| Doc PR        | WIP

- [ ] Document new feature
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.

None yet

7 participants