-
Notifications
You must be signed in to change notification settings - Fork 966
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
Added ToPast & ToPastParticiple - #58 #64
Conversation
I'd appreciate a review. I didn't do a thorough research on English rules around converting the tenses & I may have missed a rule or two. The irregular verb list should be complete though. I am not necessarily happy with method names either. I was thinking to have a Any feedback and thoughts is welcome. /cc @hazzik, @robdmoore and @hmemcpy |
P.S. This is one Pull Request I am going to regret. I am rather brain dead and my son was climbing on my back as I was writing this while I was also watching tv. Hopefully I won't swear at myself tomorrow! |
OK - I found an article with the rules. I have missed a few. Fixing now. [Update]: Found a more extensive rule set here. |
I've implemented all rules except one: "If the verb has two syllables, and the final syllable is not stressed, do not double the final consonant." and this one is going to kill me! |
{ | ||
[Theory] | ||
// Irregular verbs | ||
[InlineData("Arise","Arose")] |
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.
Wow
In general this looks awesome. There is probably some extra work required, but I wouldn't see any harm in putting this out there and letting people raise issues for words it doesn't work for? |
The problem is with that last rule and there doesn't seem to be any way to code it. It's just impossible to check which syllable the stress is on using code which makes this whole PR useless. I am actually thinking about abandoning it :( I could push this and then add all the failing words to dictionary as "irregular" verbs; but I am afraid there are many of them; e.g. happened, listened, opened, which makes this feature unreliable. What do you think? |
@robdmoore thanks for the review mate |
I don't know. I feel like it's still useful. Adding irregular ones sounds good though. I wonder if there is a list of them somewhere? |
Thanks @robdmoore for your review and input. I am going to leave this out for now as it's incomplete and I am sure A LOT of verbs are going to fall into that one missing rule which is going to scar the reliability of the library. I am thinking that if enough interest exists for this feature, I will create a new Humanizer.English or Humanizer.Dictionary library that will implement this feature properly and have more similar features like spell correction and changing tense both ways. |
I have closed this PR unmerged. The coded solution works perfectly for all cases except when the verb is not irregular, is more than one syllable, the stress is not on the final syllable and the last letter is not x and w. I know of a few words like "happened", "listened" and "opened" that fit this profile and I can take care of them one by one; but I am sure there are a lot more and I don't know all of them. |
This partially addresses #58. Need to add conversion from past and past participle to other tenses too.