timvink / mkdocs-git-revision-date-localized-plugin

MkDocs plugin to add a last updated date to your site pages
https://timvink.github.io/mkdocs-git-revision-date-localized-plugin/index.html
MIT License
206 stars 44 forks source link

Use babel.format_datetime instead of a str concatenation #23

Closed Guts closed 4 years ago

Guts commented 4 years ago

In https://github.com/timvink/mkdocs-git-revision-date-localized-plugin/blob/4e36cbee97dce3895f1e6e272fb5b65efa1f32b2/mkdocs_git_revision_date_localized_plugin/util.py#L52-L58

Prefer babel as for format_date: http://babel.pocoo.org/en/latest/api/dates.html#babel.dates.format_datetime

timvink commented 4 years ago

Thanks for the PR!

This will change the output, both date (shorter version) and time (adding AM/PM time in certain locales):

from babel.dates import format_datetime, format_date
from datetime import datetime

dt = datetime(2007, 4, 1, 15, 30)
format_datetime(dt, format="medium", locale='en_US')
# 'Apr 1, 2007, 3:30:00 PM'
format_datetime(dt, format="medium", locale='nl')
# '1 apr. 2007 15:30:00'
format_date(dt, format="long", locale='nl')
# '1 april 2007'

To maintain consistency while removing the string concat, you can specify a custom date/time pattern to format.

timvink commented 4 years ago

Hi @Guts,

I see you added some commits, but issue remains: output is not backwards compatible.

Would you like to address this, or close this PR?

Tim

codecov-io commented 4 years ago

Codecov Report

Merging #23 into master will not change coverage by %. The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #23   +/-   ##
=======================================
  Coverage   86.45%   86.45%           
=======================================
  Files           2        2           
  Lines          96       96           
=======================================
  Hits           83       83           
  Misses         13       13           
Flag Coverage Δ
#unittests 86.45% <100.00%> (ø)
Impacted Files Coverage Δ
mkdocs_git_revision_date_localized_plugin/util.py 78.72% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update be1e0a3...17a7f8f. Read the comment docs.

Guts commented 4 years ago

Hi @timvink .

I just merged from your master branch. I fixed it.

Would you be interested in my pre-commit config? It's a practical tool to ensure consistency between commiters to a repo. Let me know :)

Oh and spoiler alert: I use your https://github.com/timvink/mkdocs-git-authors-plugin too and I've got some changes to propose too.

timvink commented 4 years ago

Would you be interested in my pre-commit config?

Sure! You can reach me at vinktim@gmail.com. I agree blackening the code is an improvement.

Oh and spoiler alert: I use your https://github.com/timvink/mkdocs-git-authors-plugin too and I've got some changes to propose too.

Awesome, great to hear that! I've been trying to find time to release a new version of git-authors (my job is busy despite going remote). I expect to finish next Monday. So I advise to wait before proposing changes. Check the roadmap issue as well (https://github.com/timvink/mkdocs-git-authors-plugin/issues/16)

I fixed it.

Well, the output is still different. Depending on the locale, it shows the AM/PM time. In my locale nl:

>>> format_datetime(dt, format="long", locale='nl')
'1 april 2007 om 15:30:00 UTC'

Probably this is just a personal preference, but I like 1 april 2007 15:30:00 better.

Do you have a use case for this format or was it meant as an optimization to get rid of the 'ugly' string concat? I would be open to adding another output type, f.e. datetime_long. If not, I'd like to close this PR and leave this as they are.

Guts commented 4 years ago

Awesome, great to hear that! I've been trying to find time to release a new version of git-authors (my job is busy despite going remote). I expect to finish next Monday. So I advise to wait before proposing changes. Check the roadmap issue as well (timvink/mkdocs-git-authors-plugin#16)

No pressure. I'll wait :)

Probably this is just a personal preference, but I like 1 april 2007 15:30:00 better.

Okay, I didn't understand!

Do you have a use case for this format or was it meant as an optimization to get rid of the 'ugly' string concat? I would be open to adding another output type, f.e. datetime_long. If not, I'd like to close this PR and leave this as they are.

It was meant to get rid of the 'ugly' str concat and get more consistent using the same lib to format dates and later use skeleton method. If you prefer the actual result, you can close.

Below, I just put my test script for memory:

"""Test different behaviors in datetime formatting between babel and standard lib
"""

from datetime import date, datetime

from babel.dates import format_date, format_datetime

# globals
t = datetime.utcnow()
loc = "nl"

# date
print("Date with babel: %s" % format_date(date=t, format="medium", locale=loc))
print("Date with PSL: %s" % t.strftime("%d %B %Y"))

# datetime
print(
    "\nDatetime with babel only: %s"
    % format_datetime(datetime=t, format="long", locale=loc)
)
print(
    "Datetime with babel and concat: %s %s"
    % (format_date(date=t, format="long", locale=loc), t.strftime("%H:%M:%S"),)
)

# iso date
print("\niso_date custom: %s" % t.strftime("%Y-%m-%d"))
print("iso_date with babel: %s" % format_date(t, format="short", locale=loc))
print("iso_date with PSL isoformat: %s" % t.date().isoformat())

# iso datetime
print("\niso_datetime custom: %s" % t.strftime("%Y-%m-%d %H:%M:%S"))
print("iso_datetime babel: %s" % format_datetime(t, "medium", locale=loc))
print("iso_datetime PSL: %s" % t.isoformat())
timvink commented 4 years ago

If you prefer the actual result, you can close.

Thanks for understanding!