Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #9818 (closed defect: invalid)

Opened 10 months ago

Last modified 7 months ago

[PATCH] Time and DateTime #advance ignores invalid options

Reported by: gbuesing Assigned to: core
Priority: normal Milestone: 2.x
Component: ActiveSupport Version: edge
Severity: normal Keywords:
Cc:

Description

Currently, Time and DateTime #advance passes options other than :year, :month, and :day to #change, which allows for nonsensical stuff like: 

>> Time.utc(2000,10,1,10,30,45).advance(:days =>3, :hour => 2)
=> Wed Oct 04 02:00:00 UTC 2000

...where the hour is merely changed, not advanced.

This patch removes an options.merge in #advance, so that only :years, :months, and :days are passed to #change -- other options will be silently ignored.

Tests included.

Attachments

time_and_datetime_advance_ignore_invalid_options.diff (4.4 kB) - added by gbuesing on 10/08/07 15:32:22.

Change History

10/08/07 15:32:22 changed by gbuesing

  • attachment time_and_datetime_advance_ignore_invalid_options.diff added.

(follow-up: ↓ 2 ) 10/08/07 15:46:08 changed by chuyeow

+1.

How about changing #advance to actually use any :hours, :minutes and :seconds options to advance the time by the given hour/minute/second as well?

(in reply to: ↑ 1 ; follow-up: ↓ 3 ) 10/08/07 16:30:06 changed by gbuesing

Replying to chuyeow:

How about changing #advance to actually use any :hours, :minutes and :seconds options to advance the time by the given hour/minute/second as well?

Good idea, makes sense -- would be easy enough to implement -- something like: 

def advance(options)
  d = to_date.advance(options)
  time_advanced_by_date = change(:year => d.year, :month => d.month, :day => d.day)
  seconds_to_advance = (options.delete(:seconds) || 0) + (options.delete(:minutes) || 0) * 60 + (options.delete(:hours) || 0) * 3600
  seconds_to_advance == 0 ? time_advanced_by_date : time_advanced_by_date.since(seconds_to_advance)
end

I guess the question then is, is there any reason #advance shouldn't accept :hours, :minutes and :seconds?

(in reply to: ↑ 2 ; follow-up: ↓ 4 ) 10/08/07 16:38:15 changed by chuyeow

Replying to gbuesing:

Replying to chuyeow:

How about changing #advance to actually use any :hours, :minutes and :seconds options to advance the time by the given hour/minute/second as well?

I guess the question then is, is there any reason #advance shouldn't accept :hours, :minutes and :seconds?

Hmm, I can't think of any. Go for it I say ;)

(in reply to: ↑ 3 ) 10/09/07 02:52:10 changed by gbuesing

Replying to chuyeow:

I guess the question then is, is there any reason #advance shouldn't accept :hours, :minutes and :seconds?

Hmm, I can't think of any. Go for it I say ;)

I can't, either. Ticket created for this idea: #9825.

12/09/07 15:19:41 changed by chuyeow

  • status changed from new to closed.
  • resolution set to invalid.

Closing as invalid now since the other, better, patch (see #9825) has been applied.