Implement RRule::Rule#humanize
#49
Conversation
Sure, happy to add this, just left one comment. Ideally this would be i18n-able, but I can imagine that'd be more complicated than just swapping out the words, and that non-English languages likely have different sentence structures that change the way you need to humanize the rule, so fine to leave that for a potential future improvement. |
Addresses feedback here square#49 (comment)
This was originally ported from JS, but since it's working now it's more Ruby-ish
Addresses feedback here square#49 (comment)
craigmcnamara
force-pushed
the
humanize-method
branch
from
8c1a319 to
2593903
Compare
|
Ok I think it's ready for a review and merge. The highlights are
|
| @@ -12,7 +12,7 @@ def initialize(index, ordinal = nil) | |||
| def self.parse(weekday) | |||
| match = /([+-]?\d+)?([A-Z]{2})/.match(weekday) | |||
| index = RRule::WEEKDAYS.index(match[2]) | |||
| ordinal = match[1] ? match[1].to_i : nil | |||
| ordinal = match[1]&.to_i | |||
There was a problem hiding this comment.
I didn't mean to change this, but RuboCop was complaining about it. Since safe nav has been around since Ruby 2.3 it should be fairly safe to change. I didn't notice any official Ruby versions listed as supported anywhere.
I now have a morbid curiosity about what it would take to make this string slamming be more in line with i18n. I'll be looking in to that a bit more as a followup pull request. |
|
LGTM, thanks for the PR! |
|
Thanks for the great gem and nice PR 👏🏼 |
Yep, makes sense - maybe once #55 lands, just to get all caught up? |
Released as 0.6.0 🎉 |
I got jealous of https://github.com/jakubroztocil/rrule being able to print a human understandable version of RRule and decided to take a run at making it work.
Are you at all interested in this or should I keep it in my own fork? This is a bit incomplete, but it's actually all I need for my app. If you'd take the pull request I'd go through the motions to complete the port.