Skip to content
This repository has been archived by the owner on Mar 26, 2024. It is now read-only.

Date formatting depends on current locale #28

Merged
merged 1 commit into from Oct 23, 2013
Merged

Date formatting depends on current locale #28

merged 1 commit into from Oct 23, 2013

Conversation

swsnr
Copy link
Contributor

@swsnr swsnr commented Oct 20, 2013

In metadata.py dates are formatted with .strftime() on date objects. This method is locale-aware and formats dates according to the current environment locale, that is, the value of $LANG or $LC_*.

Hence, formatting of dates in posts depends on the environment in which Tinkerer is run, and ignores the language setting in conf.py.

To build an English language blog on a system with, say, a German locale, you currently need to set LANG to en_US.utf8 explicitly (or a similar English-language locale) to make the post dates appear in English. Otherwise they will be German, i.e. Januar 1, 2013 instead of January 1, 2013.

I can submit a pull request to fix this issue, but it would introduce a dependency against the Babel i18n library. Is that acceptable?

@vladris
Copy link
Owner

vladris commented Oct 16, 2013

I think that would be awesome. See issue #29. This keeps getting fixed and reappears due to dependency on strftime. Would be great to not have to worry about this anymore.

Does Babel work on both Linux and Windows and with both Python 2.7 and Python 3.x? That is what Tinkerer is trying to support. If so, please do go ahead, would be greatly appreciated!

@swsnr
Copy link
Contributor Author

swsnr commented Oct 16, 2013

@vladris I honestly do not know. The babel documentation explains the installation on Windows, so I presume it works on Windows quite well, but I do not have access to a Windows system with Python installed, so I cannot test.

I'll implement this, and submit a PR, but I cannot guarantee Windows support. I'll provide all the help I can, but in the end someone else has to test Windows.

@vladris
Copy link
Owner

vladris commented Oct 16, 2013

I can test it on Windows, no worries. I just wanted to make sure it is something that can be automatically installed via pip or similar package manager and it seems it is from the link you provided.

@swsnr
Copy link
Contributor Author

swsnr commented Oct 16, 2013

@vladris Yes, Babel is pip installable.

@vladris
Copy link
Owner

vladris commented Oct 18, 2013

Awesome!

@swsnr
Copy link
Contributor Author

swsnr commented Oct 20, 2013

@vladris I have attached a patch that uses Babel to format dates.

There are some issues, still:

  1. Translations must be updated, because Babel uses a different formatting language than .strftime. Should be easy though, since the mapping from .strftime to Babel's format is mostly straight-forward.
  2. I left one instance of strftime around, in a strange function split_date in utils.py.
  3. I didn't check any test cases…

I can fix these, but I need your explanation and opinion.

  1. I can either update all translations, but I don't think that the format still needs to be translatable with babel. Babel provides generic formats for short and long dates, which represent the corresponding locale's standard way of formatting dates. They differ a bit from what Tinker currently uses, especially with regards to the short format, but I don't think it is worth the hassle of keeping these translations around for this tiny bit of control.

If the date was given with generic formatters, we could just add a Jinja filter for format_date, and do all the formatting in the templates.

  1. As said, the whole purpose of this function is strange to me. It seems to be used in Post to parse dates given on the command line when creating posts, but I don't get the whole date handling in Post. Imho the whole API is convoluted.

Why do you split the date into three parts in the Post class? Why don't you just use datetime.date? Why don't use parse the date given on the command line with a custom argparse type?

But anyway, this function uses a mostly language-agnostic date format (at least with regards to western languages), so it should be safe. It'll break, though, should a user ever get the idea of giving a Japanese date on the command line…

  1. I can try to add test cases, or run them, but since you said you'll test on Windows anyway, I didn't do so yet…

@vladris
Copy link
Owner

vladris commented Oct 20, 2013

Thanks! For point 1, I'm curious what the difference is, especially if it will be different than what currently happens. We don't want to introduce change of behavior and surprise users when they upgrade. How difficult it is to update translations? Can this be done quickly/automatically? I'm not very familiar with the localization APIs :)

For 2, it is a funky way of extracting date from command line argument. I will open an issue to update that. Shouldn't be related to the change you are trying to make.

For 3, it would be great to add some test cases, especially since this is a pretty big change. Would also be great to make sure current tests pass - for example your pull seems to have broken Travis build.

Thanks for looking into this! I really appreciate it!

Makes date formatting independent of system locale
@swsnr
Copy link
Contributor Author

swsnr commented Oct 20, 2013

For point 1, I'm curious what the difference is, especially if it will be different than what currently happens. We don't want to introduce change of behavior and surprise users when they upgrade.

If we just update translations, nothing will change. If we remove the translations and just use the generic Babel modifiers, the dates will look a little different in the generated HTML. That's it.

I don't think that any user will be surprised by this. Quite likely many users will even not even realize that the dates have changed. I mean, who looks at the dates anyway? They a just a small, and largely unimportant piece of metadata.

How difficult it is to update translations? Can this be done quickly/automatically? I'm not very familiar with the localization APIs :)

How am I supposed to know answers to these questions? Aren't you supposed to know how you handle translations in your own project? I just saw that Tinkerer has translations, but I have no clue how you generate or update them.

By the way, if Babel is a dependency of Tinkerer now anyway, you can build on Babel to handle all localization. That should work more reliably across Python 2 and 3, and make the UIStr class mostly superfluous.

For 2, it is a funky way of extracting date from command line argument. I will open an issue to update that. Shouldn't be related to the change you are trying to make.

I'll leave it alone then.

For 3, it would be great to add some test cases, especially since this is a pretty big change. Would also be great to make sure current tests pass - for example your pull seems to have broken Travis build.

Oh, well, up to this moment I didn't even realize that you have Travis builds :) Besides, the Travis project is a little hard to get to, because the status image in the README doesn't link to it… at least for me clicking on this image is the standard way to get to Travis, and yours is the first project I see where this does not work.

I have found a bunch of issues in my first approach, which broke the tests. I have fixed and force-pushed, and the tests pass now.

These fixes bring me to a new point 4): The default language. What language does Tinkerer use, if language is not set in conf.py? English? The system locale?

Currently, I presume the latter and fall back to the “default” (i.e. system) locale, if app.config.language is None.

@vladris
Copy link
Owner

vladris commented Oct 20, 2013

:) Yeah, I don't know the ins and outs of localizing Tinkerer as all translations and most of the infrastructure was contributed by the community.

I'll look into linking the Travis build to the project.

vladris added a commit that referenced this pull request Oct 23, 2013
Date formatting depends on current locale
@vladris vladris merged commit d516fb8 into vladris:master Oct 23, 2013
@vladris
Copy link
Owner

vladris commented Oct 23, 2013

Thank you so much!

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

Successfully merging this pull request may close these issues.

None yet

2 participants