Remove seconds, regardless if it has AM/PM or not. #968
Conversation
This will trim the seconds, even if the string ends in AM or PM.
| @@ -57,7 +57,7 @@ window.plugin.scoreCycleTimes.update = function() { | |||
|
|
|||
| var formatRow = function(label,time) { | |||
| var timeStr = unixTimeToString(time,true); | |||
| timeStr = timeStr.replace(/:00$/,''); //FIXME: doesn't remove seconds from AM/PM formatted dates | |||
| timeStr = timeStr.replace(/:00$|:00(?= [AP]M$)/,''); | |||
There was a problem hiding this comment.
I'm not sure the (?=) assertion is necessary; why not /:00(?: [ap]m)?$/i instead?
There was a problem hiding this comment.
You're right, although I'd tweak it to be :00(?:\s?[ap]m)?$/i or :00(?: ?[ap]m)?$/i to cover for instances where certain clients aren't giving it how we expect with the space included.
|
Actually no, I remember now why I had the lookahead. It was to prevent stripping the AM/PM as part of the match. |
This reverts commit 67bb9dc.
This will trim seconds and minutes if they both end up being :00. If local time isn't a whole hour step from UMT, the minutes won't be :00 and instead will be :30 or whatever the step is while still trimming the seconds. If seconds are somehow not :00, they'll remain. The lookahead is to keep am/pm from being stripped.
|
I'll let you do the merge and squash if you decide to take the pull versus doing the squash on my end. |
|
I'll end up squashing, but I see you've added a repeat group to the |
|
Will unixTimeToString always return seconds? If not, a single :00 will
|
|
Maybe just me, but I'm not sure why we have am/pm inn the first place - 24hr clock all the way to avoid ambiguity |
|
@imsaguy Under the hood, That being said, we don't have any guarantees about exactly how that output is formatted. Maybe we should detect whether the user prefers 12-hour or 24-hour and then just use a specific canned format rather than a locale-specific one. There are many more outputs returned than this function copes with — see the MDN page linked above for some examples. @PrinterElf Many American agents in particular prefer the 12-hour clock to 24-hour. |
|
I think the filtering of am/pm should be done on as part of the API rather than post processing such as we see here. Otherwise, it'd be just was straightforward to make this a Date object here and format it the way you please, rather than trying to parse random strings. And if someone else tweaks unixTimeToString() later on, you wouldn't have gained much. |
This will trim the seconds, even if the string ends in AM or PM.