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

Use inferred timezone for time field in output #31

Merged
merged 4 commits into from
Aug 18, 2020

Conversation

AstroCB
Copy link
Contributor

@AstroCB AstroCB commented Aug 14, 2020

Since an inferred timezone is returned as part of the placemark data, it can be used to localize the timestamp string given in the JSON blob, as well as the string data (if requiresPlaceMarkLookup is toggled on as the result of one of the other format specifiers).

This PR just alters the time field (in both the JSON output and the formatted string output) to use the user's inferred timezone if available – it outputs in exactly the same format as the default, but will use the correct timezone instead of UTC. If this isn't available, it falls back to UTC (the existing behavior).

@fulldecent
Copy link
Owner

Thank you for sharing. Please implement this as a new field rather than losing the existing field. Maybe the new field can be time_local.

@AstroCB
Copy link
Contributor Author

AstroCB commented Aug 18, 2020

Done!

@fulldecent
Copy link
Owner

Yup, that looks great, thank you!

@fulldecent fulldecent merged commit f73a1b4 into fulldecent:master Aug 18, 2020
@fulldecent
Copy link
Owner

Released version 3.2.0.

I'm not sure how our version gets bumped in Homebrew. Somebody else did this last time, possibly automatically, and I don't see documentation of how that works.

@AstroCB
Copy link
Contributor Author

AstroCB commented Aug 22, 2020

Done.

Also filed a PR #32 to link to the instructions I used for filing this (it was automated with their script).

@fulldecent
Copy link
Owner

Awesome, thank you, merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants