-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[PropertyAccess]: Allow custom singularify class (WIP) #15766
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
Conversation
8d95675
to
4a5603c
Compare
| 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
4a5603c
to
d67e7b8
Compare
My guess is that the idea behind it is to program in English, the same "language" as PHP is. Not to mention that if 1 language is supported additionally, people want all of them. Language rules are complex and hard to maintain. Not to mention that they might change over time. |
@javiereguiluz That would mean people would install additional packages just because they prefer to program in another "language", I know this is beyond symfony, but to me that sounds like writing a spellings correction library where you add things like I don't think that making this configurable is the right way to go. I'm fine with people programming in their native "language" (so please don't get me wrong), I just don't see a lot of good coming from this. |
The truth is that some developers already use their own languages for some elements (e.g. entities). In those cases, Symfony makes hard-to-understand mistakes (see #13721 for example). |
There is already a possibility to do this, which is built in into the PropertyAccess component. This feature is disabled however: #5011 The reason is that we need to fix this in a more generalized way (See #5013). We should instead finish the discussion in #5013 and enable the feature again (with the decided syntax). Having a globally configured setting with a class doing this kind of thing doesn't seem right (a 3rd party library can require the English Singularization rules, while app code relies on some custom rules). |
@iltar Knowing English is no-way a requirement to be a good PHP programmer. In fact, I've had students that were real good PHP programmers and had a rather basic English knowledge. My point is that allowing people to use properties in other languages could benefit people that is no fluent in English. In fact, I do prefer using English properties in my projects, but I cannot force my students to do the same because I teach programming, not language. By the way, what would be wrong with people publishing bundles offering other singularization rules? They wouldn't be officially supported anyway, but the community could. The key here is flexibility: if you want to use an alternate set of rules, at least you can. Anyway @wouterj has a point with the globally configured setting. It would break other bundles/libraries expecting the default behaviour and that is really bad. That could be worked out allowing multiple singularization classes and merging the output of all of them, but I feels somewhat hacky. I really don't 8000 mind how is implemented as long the override feature is included in the core, even if you must explicitly annotate each property. I agree that PR #5013 should be solved before 2.8 is launched. It will be very difficult to implement big changes after the release for some time. If we move the discussion to #5013 I'll be glad to close this issue. Thank you for your comments! |
@lrlopez What I think is wrong with requiring packages that provide that, would be people acknowledging packages in production which are only used for development. It's not wrong to develop in another language, I just don't agree with having code to fix code because of a design choice. |
@iltar I see your reasoning there, but I think that is unavoidable because Every now and then an issue is open because Anyway, if there was an override setting anywhere in the model definition (like #5013 proposes) there would be no need for any of this. |
Isn't there something that can be re-used from the translation component here? |
Hmmm... I don't think it would be aplicable to this case. Anyway, requiring the translation component for property access just doesn't seem right to me. |
Big +1 for this. We have customers with special terms that simply can not be translated into english. If we create entities or forms, what should we do? We can not disable the singularify without override the whole PropertyAccess. And it is imposible to translate these terms into english without confusing. So, now we have misspelled methods to meet PropertyAccess singularify. And that's wrong for me. Actually PropertyAccess should adapt to our needs. |
Closing in favor of #5013 |
@fabpot Fabien, PR #5013 seems to be stalled since October 2014 which is almost a whole year. Can anyone from the team have a look into it? I'd really like to have a mechanism embedded into |
I'm going to teach Symfony to some Spanish students. We are using models created in other subjects into our test projects, and so we have to keep the original entities names and properties, which are in Spanish.
When implementing Doctrine X-to-many relationships, Symfony calls
StringUtils::singularify()
for property access. As the method only works for English nouns, awkward methods names (for a native speaker) have to be created for the app to work.I know this has been discussed multiple times, but I'm proposing a different approach to the problem. What about allowing to override the default behaviour by using custom singularify classes? It would be backward compatible while allowing to plug-in different languages, should it be needed. The PR implements a new configuration parameter inside
property_access
calledproperty_singularify
that now can point to an alternative class that implementsStringSingularifyInterface
. The interface only defines asingularify()
method. I still have to run some benchmarks using Blackfire.io to see if there is any noticeable performance degradation, although I don't expect much.This is my first Symfony contribution so this PR could be (well, it will be) wrong at many levels, so all comments are welcome. Maybe just the idea is bogus, but I want to give it a try.
I'd rather like to see it accepted for the 2.8/3.0 release, I hope is not too late for it.
Thanks guys!