Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
Fixed singular of committee#18909
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
peterrehm commentedMay 29, 2016
Fabbot should be ignored in this case... |
fabpot commentedMay 29, 2016
Can you indeed also provide a PR for 2.3? |
peterrehm commentedMay 29, 2016
Just provided#18911 |
| // Convert teeth to tooth, feet to foot | ||
| if (false !== ($pos =strpos($plural,'ee')) &&strlen($plural) >3 &&'feedback' !==$plural) { | ||
| if (false !== ($pos =strpos($plural,'ee')) &&strlen($plural) >3 &&!in_array(strtolower($plural),$irregularNouns)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Please, preserve the strict comparison atin_array().
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Updated.
| } | ||
| } | ||
| $irregularNouns =array( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This doesn't seem to be in use?
peterrehm commentedJun 6, 2016
@teohhanhui I was thinking about the whitelist approach as well but I see the problem that with the whitelist approach it will be hard to match all the relevent cases and the risk is high to introduce BC breaks whereas the blacklist approach should not break existing applications. |
TakTaki commentedJun 6, 2016
@peterrehm I don't totally agree with you as I currently have a |
TakTaki commentedJun 6, 2016
I was wondering: what if I have an attribute named |
peterrehm commentedJun 6, 2016
@TakTaki It would have needed an explicit list. After some research I only found the 3 added nouns affected, I did not find more. So I switched to the whitelist approach which should be easier. The question is if this is a BC break or not? /cc@nicolas-grekas |
| * Irregular nouns which will not be converted 'ee' to 'oo' | ||
| * | ||
| * @var array | ||
| */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
this looks unused
stof commentedJun 6, 2016
Can you synchonize the PR targetting the older branch too ? |
peterrehm commentedJun 6, 2016
@stof It was, missed some changes it is already updated. |
peterrehm commentedJun 6, 2016
@stof The other PR is already updated as well. What do you think about the changed? I could not think of any better solution. |
| * @var array | ||
| * | ||
| * @see http://english-zone.com/spelling/plurals.html | ||
| * @see http://www.scribd.com/doc/3271143/List-of-100-Irregular-Plural-Nouns-in-English |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Perhaps we can link to:http://grammar.about.com/od/words/a/A-List-Of-Irregular-Plural-Nouns-In-English.htm
This PR was submitted for the 2.3 branch but it was merged into the 2.7 branch instead (closes#18911).Discussion----------Fixed singular of committee| Q | A| ------------- | ---| Branch? | 2.3| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#18872,#18748| License | MIT| Doc PR | -In addition to PR#18909.Commits-------5661d52 Fixed singular of committee
fabpot commentedJun 8, 2016
Thank you@peterrehm. |
This PR was squashed before being merged into the 3.1 branch (closes#18909).Discussion----------Fixed singular of committee| Q | A| ------------- | ---| Branch? | 3.1?| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#18872,#18748| License | MIT| Doc PR | -As in SF 3.1 the StringUtil has been moved to a new Component, I am proposing this for 3.1.I can easily provide the same in an additional PR against the 2.3? branch.Commits-------b5e1f24 Fixed singular of committee
Uh oh!
There was an error while loading.Please reload this page.
As in SF 3.1 the StringUtil has been moved to a new Component, I am proposing this for 3.1.
I can easily provide the same in an additional PR against the 2.3? branch.