IITC-CE / ingress-intel-total-conversion

intel.ingress.com total conversion user script with some new features. Should allow easier extension of the intel map.
https://iitc.app
ISC License
285 stars 110 forks source link

Standardize Date/Time format usage #458

Open johnd0e opened 3 years ago

johnd0e commented 3 years ago

Atm we have variety of home-brewed formatting functions, like unixTimeToString, unixTimeToDateTimeString, unixTimeToHHmm, formatInterval (and perhaps a numerous local in-place formatting routines in plugins).

It'd be great to use some standardized approach instead, e.g. Intl.DateTimeFormat.

There are still some controversies:

Affected parts:

Note: started in https://github.com/IITC-CE/ingress-intel-total-conversion/commit/e92dd96334bc490a759876ed6b2aed739f8f365a#commitcomment-42973205

le-jeu commented 3 years ago

I don't find any call of unixTimeToString except in score-cycle-times with full=true (it was used to display the capture time long ago). The remaining calls assume a format with seconds at the end(that is broken on 12h cycle)

https://github.com/IITC-CE/ingress-intel-total-conversion/blob/5d593f0221882345aee92f8d54b7eabd6bd20a1b/core/code/utils_misc.js#L186-L197

Is unixTimeToHHmm supposed to format time as HH:mm or is it supposed to respect the locale ? Is getting '9:04 pm' to avoid ? Are there places we don't want to be locale dependent ?

Also, region score uses its own format https://github.com/IITC-CE/ingress-intel-total-conversion/blob/5d593f0221882345aee92f8d54b7eabd6bd20a1b/core/code/region_scoreboard.js#L432-L452

In chat, we use a date time string with milliseconds, the millisecond part is put in a <small> tag. https://github.com/IITC-CE/ingress-intel-total-conversion/blob/5d593f0221882345aee92f8d54b7eabd6bd20a1b/core/code/chat.js#L487-L491 We can't assume with locale that the milliseconds are at index 19. Also, the fractional depends on the locale too (, or ., is there anything else ?) making millisecond low-light more complex to do, also the millisecond could not be at the end of the resulting string (pm/am)

also chat input time has its own formating, https://github.com/IITC-CE/ingress-intel-total-conversion/blob/5d593f0221882345aee92f8d54b7eabd6bd20a1b/core/code/chat.js#L761-L774

johnd0e commented 3 years ago

Is unixTimeToHHmm supposed to format time as HH:mm or is it supposed to respect the locale ?

I suppose that we should leave unixTimeToHHmm as is, as deprecated, and switch to use Intl.DateTimeFormat directly. Maybe unixTimeToHHmm itself should be reimplemented with Intl.DateTimeFormat, but without locale considerations.

We can't assume with locale that the milliseconds are at index 19

This part may be implemented without locale considerations (it is tooltip).

le-jeu commented 3 years ago

We can't assume with locale that the milliseconds are at index 19

This part may be implemented without locale considerations (it is tooltip).

on the contrary, using locale here is relevant since we show a full datetime data for the user

McBen commented 3 years ago

In my humble opinion:

that would lead to these ToDo's:

johnd0e commented 3 years ago

on the contrary, using locale here is relevant since we show a full datetime data for the user

We show here full and detailed (milliseconds) datetime data, and even in North America in such cases preferred 24h format:

24- and 12-hour time systems. In countries where the 12-hour clock is dominant, some professions prefer to use the 24-hour clock. For example, in the practice of medicine, the 24-hour clock is generally used in documentation of care as it prevents any ambiguity as to when events occurred in a patient's medical history.

le-jeu commented 3 years ago

DateTimeFormat options allow to ask for hour24 whatever the locale.

johnd0e commented 3 years ago

DateTimeFormat options allow to ask for hour24 whatever the locale.

Exactly. But I meant other: we may want to ignore locale settings completely for some cases where we would like to see precise values.

johnd0e commented 3 years ago

If using Intl.DateTimeFormat I propose to set some conventions.

johnd0e commented 3 years ago

formatInterval is used only in getHackDetailsText. But there are other similar routines in region_scoreboard.js, player-activity-tracker, and perhaps other places.

We should either make formatInterval more generic (to be applicable in more cases), or just stop exposing it.

Related: