Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Preserve specified time offsets from YAML #121

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

Open
chrisperelstein wants to merge 1 commit into ruby:master
base: master
Choose a base branch
Loading
from chrisperelstein:preserve_offset

Conversation

@chrisperelstein
Copy link

@chrisperelstein chrisperelstein commented Jan 16, 2013

This change causes parse_time function to return a Ruby Time object with an offset specified in the parsed YAML. I've been playing with Jekyll which leverages Pysch for parsing YAML data in posts and it would be nice to have Time objects extracted from posts respect the offset specified in the YAML front matter. Then, I could either display it with the specified time offset, convert it to utc with the Time::utc, or convert it to local time with Time::getlocal. This seems to me to be a little bit better behavior, but there may be a good reason not to do this that I'm not aware of.

Copy link
Contributor

sheerun commented Feb 27, 2013

fuck yeah

Copy link
Member

@chrisperelstein it seems fine, but can you demo the issue for me? I don't want the code to be reverted accidentally. :(

Copy link
Author

I'll work on getting an example up late tonight with example Yaml, but basically the last line in parse_time uses Time.at. Time.at always returns a Time object whose gmt_offset method will be the current offset on the machine in which it's running, instead an offset specified in the Yaml front matter.

Assuming a bit of Yaml like "date_published: 2013年04月08日T17:34:54-07:00":

  • Currently, if I'm in New York (-4 hour offset), I'll get a Ruby Time object that whose gmt_offset method returns -14400 (-4 hours) instead of -25200 (-7 hours).
  • With my change, if I'm in New York, I'll get a Ruby Time object whose gmt_offset method returns -25200 (-7 hours).

I'll get a gist or something up tonight that demonstrates this.

Copy link
Member

Bump. Any update on a test? Thanks!

Copy link
Author

I'm sorry, I've been slack lately. Bought a house and it's been a bit of a time suck. Rough ruby script that demonstrates the issue here: https://gist.github.com/chrisperelstein/5978935. Hopefully that makes enough sense. With my change, you'll get the same result when printing the date since the Psych generated time object will get the offset originally specified in the input.

Changing scaler_scanner's parse_time function to return a time object
with offset preserved if it was specified.
Copy link
Member

@chrisperelstein sorry to take so long (2 years 😦) to get back on this. I just looked at your repro script. It looks like you're calling Psych.load against a value that Psych didn't generate. Time#to_s doesn't produce a sting that's in the YAML format spec.

If this is still an issue, can you please give me a test for it?

Copy link
Author

@tenderlove My turn to take so long. I seem to have deleted the gist I was using as an example (sorry!) so I'm going to try to recreate something. Don't remember where the snippet of yaml came from that I used but I think it was something I was manually doing in order to fix a time zone changing issue I noticed when using octopress. The functional punchline of the pull request is to simply preserve time offset when specified regardless of where the time came from. My opinion is that Pysch shouldn't discard original offset info by automatically converting to local time offset, especially since the Time object includes the convenient getlocal method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /