Skip to content
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

timezone data in post date is overwritten #4096

Closed
willnorris opened this issue Nov 2, 2015 · 12 comments
Closed

timezone data in post date is overwritten #4096

willnorris opened this issue Nov 2, 2015 · 12 comments
Labels
frozen-due-to-age stale Nobody stepped up to work on this issue.
Milestone

Comments

@willnorris
Copy link
Contributor

One of the unfortunate side effects of how the "posts as collection" feature was implemented in b89f943 is that now the original timezone of a post's date is forever lost. For previous discussion of this issue, see #1069.

Prior to the above change, I was relying on the fact that if a date was specified as a string, safe_yaml would not try and parse it as a date, but instead just leave it alone (see my original comment at #1069 (comment)). Similarly, Jekyll itself would only ever set self.date, but always leave data["date"] untouched (see Post.initialize).

With the new Document.merge_data function, the data["date"] value is explicitly being ovewritten with the localtime value returned by Utils.parse_date, meaning that there is no way for a plugin to get at the original, author-specified value. Could that safely be reverted to set self.date rather than data["date"]? I'm not sure what else is relying on the data hash.

For the time being, I'll likely work around this by monkey-patching Document.merge_data to preserve the original value somewhere. But I'll still repeat what I said previously:

if an author has taken the effort to specify the TZ in the post date, then it seems very unfortunate that we don't respect and preserve that.

@envygeeks envygeeks added this to the 3.0.1 milestone Nov 2, 2015
@envygeeks envygeeks added the Bug label Nov 2, 2015
@envygeeks
Copy link
Contributor

Hulk: +discover:document.rb++merge_data!

@envygeeks
Copy link
Contributor

Your text kinda jumps to me, is this about your date being overridden or it this about you not being able to get at "post.date" the way you used to be able to? On the comment about nothing using it, it's because it was just implemented with the release. Can you clarify your problem for me?

@willnorris
Copy link
Contributor Author

Yeah, sorry. Disregard the "Could that safely be reverted to set self.date rather than data["date"]?" part... I don't actually care that much how it's implemented.

The main thing that I care about is that I'm including a timezone in the front matter of my posts, for example:

---
date: "2015-10-23T09:28:15-07:00"
---

but Jekyll is always converting that to the local timezone. The time is still correct (it's adjusting the hours appropriately when needed), but I have no way of knowing what the original timezone was. So the problem isn't that Jekyll is displaying the "wrong" time... it's not actually "wrong", it's just in a different TZ. The problem is that Jekyll is masking some of the user-specified data with no ability (that I'm aware of) to access it.

Does that make sense?

@envygeeks
Copy link
Contributor

I see now, yeah it's a bit weird we don't preserve your time zone and shift on that, is that intended @parkr? I don't actually know if it's an intended enforcement to normalize or not.

@envygeeks envygeeks added Discussion and removed Bug labels Nov 2, 2015
@parkr
Copy link
Member

parkr commented Nov 3, 2015

Oh man, this is an old conundrum. I actually wrote about this "regression" on my blog: https://byparker.com/blog/2014/ruby-2-2-0-time-parse-localtime-regression/

Jekyll has historically (because Ruby did this automatically) converted timezones to whatever your local system timezone is. In Ruby 2.2.0, that changed: the ruby-core team decided to stop automatically applying #localtime so we had to do it in order to maintain the old functionality. I'm surprised this is new, though. Related PR: #3234

@willnorris
Copy link
Contributor Author

what's new is that data["date"] is being overwritten with the adjusted-to-localtime value. That was the one place that the author-specified value was not touched (assuming it was specified as a string in the front matter, otherwise safe_yaml still clobbered the original timezone)

@willnorris
Copy link
Contributor Author

And while I agree that the behavior in Ruby 2.2 is a regression from the perspective of ruby (particularly in a minor revision), it's also desirable behavior in many cases. As is the case here, forcing all times to localtime as soon as they're parsed means that you never know what timezone it was originally specified in.

I'm not suggesting that Jekyll should stop defaulting to localtime, particularly since that has been the behavior for so long. I'm just suggesting that somehow the original, as-specified-by-the-author timezone ought to be preserved.

@parkr parkr added the Bug label Nov 3, 2015
@willnorris
Copy link
Contributor Author

I finished upgrading my site to Jekyll 3, and the fix for this didn't wind up being too bad. I did still end up having to monkey-patch merge_data!, but I was already previously patching to_liquid, so this doesn't actually end up being that much worse. However, from a design philosophy perspective, it still makes me sad though that user data gets lost.

For those that are curious, here was my change: willnorris/willnorris.com@dcb7ef0

@envygeeks
Copy link
Contributor

That's pretty nasty to have to do: /cc #3829 - @jekyll/core do you think we should add a hooks to before and after we merge so that people can preserve and modify data?

@parkr
Copy link
Member

parkr commented Nov 3, 2015

Yeah @envygeeks before_data or before_merge might be a good idea? Or we don't call #localtime until we hit #to_liquid.

@parkr parkr modified the milestones: 3.0.1, 3.0.2 Nov 4, 2015
@alfredxing
Copy link
Member

Yeah, if the whole point of hooks is to get rid of the need for monkey patching, I'd be all for adding these hooks rather than have users need to monkey patch the methods.

@envygeeks envygeeks modified the milestones: 3.1, 3.0.2 Dec 4, 2015
@parkr parkr modified the milestones: 3.2, 3.1 Jan 15, 2016
@jekyllbot jekyllbot added the stale Nobody stepped up to work on this issue. label Jun 6, 2016
@jekyll jekyll locked and limited conversation to collaborators Jul 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
frozen-due-to-age stale Nobody stepped up to work on this issue.
Projects
None yet
Development

No branches or pull requests

5 participants