-
Notifications
You must be signed in to change notification settings - Fork 105
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
Using only months and days drops weeks and produces surprising results #77
Comments
This is working as intended (https://github.com/radar/dotiw#only), but maybe you can come up with an implementation of the |
2 years after this change I bumped my version of the Gem from 3.01 to the latest, and there's been a bug across my application as a result. The application used My point is that this issue raised by @jmperez127 is not actually a new feature request. The introduction of the weeks concept has resulted in a change in behavior. And, since it's by far the fastest way to address it, I'm likely to just add Anyway, I greatly appreciate anyone that's helped build and maintain this Gem. I don't mean to come off like this is a huge problem or give anyone a hard time. Just that for posterity this issue is not exactly a new feature request, so much as a change in behavior from the way the Gem had been behaving. I'll take a look at the |
While this works as expected I personally do think it's surprising. Does anyone have a strong use-case of |
@dblock while it might be complex, a nifty way to do it would be to drop results SMALLER than the smallest value in the
Here, the smallest value in the array is days. Therefore anything for hours, minutes, and seconds is dropped. However, weeks is not smaller than the smallest value in the array, so rather than being dropped, it gets "downward converted" to the next smallest specified value. The more I think about this as I type it, the more I suspect it would be absurdly complex and not worth the effort. But since I've already typed it up, I might as well post it and perhaps it will help you brainstorm! |
I think what you propose is an interesting idea! I don't think it's too bad to implement though if you want to give it a shot:
I'd start by writing a bunch of specs :) |
Hi @dblock I hope to find the time at some point to give this a shot, but I'm also not that experienced of a developer, have never really contributed to a gem like this, and might take me a while to get it acceptably implemented. But hopefully I can work on it some day. |
Cool. I would start by writing tests of what you expect the output to be, and that alone is valuable. |
If we could make [1] pry(main)> hash = DOTIW::Methods.distance_of_time_in_words_hash(Time.now, 72.days.ago, except: [:minutes, :hours])
=> {:years=>0, :months=>2, :weeks=>1, :days=>4, :minutes=>59, :seconds=>59}
[2] pry(main)> hash = hash.merge(days: hash[:days] + (hash.delete(:weeks) || 0) * 7)
=> {:years=>0, :months=>2, :days=>11, :minutes=>59, :seconds=>59}
[3] pry(main)> DOTIW::Methods.send(:_display_time_in_words, hash)
=> "2 months, 11 days and 59 minutes" |
Hello everyone!
I'm trying to use only months and days, but the value I'm getting is not right. If I set the date to 9/28 I get "1 month and 2 days" instead I should get "1 month and 9 days". So I guess it's omitting the week and is not combining the week into days. Am I doing something wrong or is this an issue? Thanks!
PS: here's the code
distance_of_time_in_words(date_a, date_b, false, :only => [:months, :days])
The text was updated successfully, but these errors were encountered: