Skip to content

Locale Timestamp #170

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

Merged

Conversation

dennisdeng2002
Copy link
Contributor

On Android, if you set the app language to Arabic the timestamp will be formatted using the device locale, i.e. ٠٥T١٠:٤٨:٢٩.٥Z. I've attached screenshots of the behavior before / after the changes

Before / After (Arabic)

Before / After (English)

val sdf = SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss'.'Szzz")
// Note, we should specify locale = Locale.ROOT, otherwise the timestamp returned will use
// the default locale, which may not be what we want.
val sdf = SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss'.'Szzz", Locale.ROOT)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Android docs recommend using Locale.US, but since this might be used in JVM projects I opted to go with Locale.ROOT

The default locale is not appropriate for machine-readable output. The best choice there is usually Locale.US – this locale is guaranteed to be available on all devices, and the fact that it has no surprising special cases and is frequently used (especially for computer-computer communication) means that it tends to be the most efficient choice too.

@dennisdeng2002 dennisdeng2002 marked this pull request as ready for review June 5, 2023 18:13
Copy link
Contributor

@didiergarcia didiergarcia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch!

@Test
fun `dateTimeNowString() produces a string in the correct format`() {
val dateTimeNowString = dateTimeNowString()
val regex = Regex("^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:\\d{2}\\.\\d{3}Z$")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this regex fails unit test. try this one "^\\d{4}-[0-1][0-3]-[0-3]\\d{1}T[0-2]\\d{1}:[0-5]\\d{1}:[0-5]\\d{1}Z$"

@wenxi-zeng wenxi-zeng merged commit d124a36 into segmentio:main Jun 5, 2023
@dennisdeng2002 dennisdeng2002 deleted the dennisdeng/locale-timestamp branch June 5, 2023 19:08
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.

3 participants