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

[python] Fixed getRegion dateshort format without leading zero #8282

Merged
merged 1 commit into from Dec 5, 2015

Conversation

Projects
None yet
7 participants
@LS80
Copy link
Contributor

LS80 commented Oct 24, 2015

Currently xbmc.getRegion('dateshort') returns a bad strftime format when locale.shortdateformat has the month and or date without a leading zero e.g. YYYY-M-D would become %Y-M-D.

Linux and OS X strftime has %-m and %-d.
Windows has to fallback to YYYY-MM-DD i.e. with leading zeros.
EDIT: The Windows equivalent is %#m and %#d.

EDIT: Platforms tested:

  • Linux
  • Windows
  • Android
  • Mac OS X
  • iOS
@mkortstiege

This comment has been minimized.

Copy link
Member

mkortstiege commented Oct 25, 2015

@tamland, @phil65 for review please.

@tamland

This comment has been minimized.

Copy link
Member

tamland commented Oct 25, 2015

According to https://msdn.microsoft.com/en-us/library/fe06s4ak.aspx windows supports removing leading zeros with #, but both - and # are non-standard extension so I'm not sure if this will really work on all platforms. Ping @Montellese

@phil65

This comment has been minimized.

Copy link
Member

phil65 commented Oct 25, 2015

I could test linux if required (no windows machine available for next 2 weeks). So if you want me to test, just ping me again.

@LS80 LS80 force-pushed the LS80:python_shortdate_format branch from 090662a to d1ebec6 Oct 25, 2015

@LS80

This comment has been minimized.

Copy link
Contributor Author

LS80 commented Oct 25, 2015

Nice find @tamland, it does indeed work on Windows. I've updated the PR.

@MartijnKaijser

This comment has been minimized.

Copy link
Member

MartijnKaijser commented Oct 31, 2015

@tamland ok to merge?

@LS80

This comment has been minimized.

Copy link
Contributor Author

LS80 commented Nov 29, 2015

@MartijnKaijser Is there a specific reason why this hasn't been merged?

@tamland

This comment has been minimized.

Copy link
Member

tamland commented Nov 29, 2015

Sorry for not replying, it completely slipped my mind. I asked koying about it a while back and it most likely will not work on android. We only know it works on linux and windows. Either it must be tested on all other platforms, or documentation found that it will, or the leading zero format must be left on those platforms. It's far worse to use and invalid format here and break addons using it than a leading zero.

This is a fix and when done it can still be merged for jarvis.

@LS80

This comment has been minimized.

Copy link
Contributor Author

LS80 commented Nov 29, 2015

@tamland OK I can also test this on Android and Mac OS X to make sure it's valid on those platforms. The only platform I can't test is iOS.

P.S. To be clear the problem is not that it shows a leading zero when it shouldn't, it actually currently outputs a completely incorrect strftime, albeit one which is technically valid and won't throw an exception. When locale.shortdateformat is YYYY-M-D the output is %Y-M-D, so strftime would produce the string 2015-M-D.

EDIT: The - format does work on Android.
EDIT: and Mac.

@tamland

This comment has been minimized.

Copy link
Member

tamland commented Dec 1, 2015

@Memphiz @koying Ok with you?

@koying

This comment has been minimized.

Copy link
Contributor

koying commented Dec 1, 2015

Not quite my area. If you want me to test something because you have no access to a droid device, you'll have to tell me what to test an how ;)

@Memphiz

This comment has been minimized.

Copy link
Member

Memphiz commented Dec 1, 2015

Same question - how can i test this?

@LS80

This comment has been minimized.

Copy link
Contributor Author

LS80 commented Dec 1, 2015

Something like this should do it.

from datetime import date
import xbmc, xbmcgui
fmt = xbmc.getRegion('dateshort')
xbmcgui.Dialog().notification(fmt, date.today().strftime(fmt))

Change the Short date format setting to 2015-12-1 then run the add-on to check that the notification shows the date in the correct format.

I can upload a test add-on this evening if it helps.

@Memphiz

This comment has been minimized.

Copy link
Member

Memphiz commented Dec 1, 2015

that would help indeed ;)

@LS80

This comment has been minimized.

Copy link
Contributor Author

LS80 commented Dec 2, 2015

@Memphiz

This comment has been minimized.

Copy link
Member

Memphiz commented Dec 2, 2015

@LS80 - works on ios - thx for spoon feeding me :)

@tamland tamland added this to the Jarvis 16.0-beta3 milestone Dec 5, 2015

@tamland

This comment has been minimized.

Copy link
Member

tamland commented Dec 5, 2015

jenkins build this please

MartijnKaijser added a commit that referenced this pull request Dec 5, 2015

Merge pull request #8282 from LS80/python_shortdate_format
[python] Fixed getRegion dateshort format without leading zero

@MartijnKaijser MartijnKaijser merged commit e1216d6 into xbmc:master Dec 5, 2015

1 check passed

default Merged build finished.
Details

@LS80 LS80 deleted the LS80:python_shortdate_format branch Feb 28, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.