Skip to content

Allow parsing timestring at different timezones. #58

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Allow parsing timestring at different timezones. #58

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f08e6c6
Allow parsing timestring at different timezones
knobo Oct 2, 2016
abac2a1
Add missing newline
knobo Oct 2, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions src/local-time.lisp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1557,8 +1557,9 @@ The value of this variable should have the methods `local-time::clock-now', and
(allow-missing-date-part allow-missing-elements)
(allow-missing-time-part allow-missing-elements)
(allow-missing-timezone-part allow-missing-elements)
(offset 0))
"Parse a timestring and return the corresponding TIMESTAMP. See split-timestring for details. Unspecified fields in the timestring are initialized to their lowest possible value, and timezone offset is 0 (UTC) unless explicitly specified in the input string."
(offset 0)
(timezone *default-timezone*))
"Parse a timestring and return the corresponding TIMESTAMP. See split-timestring for details. Unspecified fields in the timestring are initialized to their lowest possible value, and timezone offset is 0 (UTC) unless explicitly specified in the input string. If offset is nil, default timezone is used, or the timezone provided by the timezone keyword."
(let ((parts (%split-timestring (coerce timestring 'simple-string)
:start (or start 0)
:end (or end (length timestring))
Expand All @@ -1584,7 +1585,8 @@ The value of this variable should have the methods `local-time::clock-now', and
:offset (if offset-hour
(+ (* offset-hour 3600)
(* (or offset-minute 0) 60))
offset))))))
offset)
:timezone timezone)))))

(defun ordinalize (day)
"Return an ordinal string representing the position of DAY in a sequence (1st, 2nd, 3rd, 4th, etc)."
Expand Down
14 changes: 14 additions & 0 deletions test/timezone.lisp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,20 @@
(new (timestamp+ old 24 :hour eastern-tz)))
(is (= (* 24 60 60) (timestamp-difference new old)))))

(deftest test/timezone/parse-timestring-different-timezones ()
Copy link
Owner

@dlowe-net dlowe-net Oct 3, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're testing timezones but only using date timestamps? That seems odd.

(let ((*default-timezone* eastern-tz))
(is (timestamp=
(parse-timestring "2016-12-01" :offset nil) ;; Use default timezone
(parse-timestring "2016-12-01" :offset (* 60 60 -5)) ;; Ignore timezone
(parse-timestring "2016-12-01" :offset (* 60 60 -5) :timezone utc-leaps) ;; Ignore timezone
(parse-timestring "2016-12-01" :offset nil :timezone eastern-tz) ;; Use timezone
Copy link
Owner

@dlowe-net dlowe-net Oct 3, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be wrapped in a (let ((default-timezone +utc-zone+)) ...) because you're not actually testing here that the timezone used is different from the default.

Copy link
Contributor

@orivej orivej Oct 3, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this were in UTC zone, parse-timestring might change such that :offset nil would be interpreted as :offset 0 regardless of the *default-timezone*, whereas this code tests that :offset nil uses *default-timezone*.

Copy link
Owner

@dlowe-net dlowe-net Jan 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, I've let this PR seriously languish. Sorry.

I mean that by setting default-timezone to eastern-tz and carefully arranging the arguments so that they return the same timestamps, this test basically doesn't show that the arguments do anything at all. A version of parse-timestring that ignored all its arguments and only used default-timezone would pass this test.

(adjust-timestamp (parse-timestring "2016-12-01" :offset nil :timezone utc-leaps) (offset :hour +5)) ;; Use different timezone
Copy link
Owner

@dlowe-net dlowe-net Oct 3, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line seems more of a test of adjust-timestamp.

;; Rest use timezone from timestring
(parse-timestring "2016-12-01T00:00:00-05:00")
(parse-timestring "2016-12-01T00:00:00-05:00" :offset 0)
(parse-timestring "2016-12-01T00:00:00-05:00" :offset -10)
(parse-timestring "2016-12-01T00:00:00-05:00" :offset nil :timezone utc-leaps)))))

(deftest test/timezone/timestamp-minimize-part ()
(is (timestamp=
(timestamp-minimize-part
Expand Down