[00:01:01] maybe it's defensible to have an abstraction for string formats but expanding it to TS_UNIX was a mistake [00:01:20] if I wrote that library, there wouldn't even be a TS_UNIX [00:02:08] but it's legacy, I think it goes back to Lee Daniel Crocker [00:02:33] just pulled out of MediaWiki into a library without ever rethinking what was going on there [00:03:36] this is a bug resulting from TS_UNIX/TS_MW confusion: https://phabricator.wikimedia.org/T384241 [02:15:47] right, it wouldn't be unreasonable to require specifying what the given format is when parsing/inputting/converting. [02:16:00] or to only support ones that aren't ambigious. [02:22:08] Unix timestamps will reach 11 digits in the year 2286, 12 digits in the year 5138, 13 digits in the year 33,658, and 14 digits in the year 318,857 [02:25:58] that should make it relatively easy to pick the more reasonable interpretation, and unless I just mixed up my inputs it seems that's what we do already (14 digits must be TS_MW, not Unix). [02:26:29] but if something really did have a 14-digit timestamp, it should indeed not have to be "parsed" in the first place but just given as-is. [02:27:32] Having something like `new Timestamp(int/float $unix)` with `Timestamp::parse(string, $format = 'detect')` would be a small improvement I suppose. [02:28:36] The idiom could then become Timestamp::parse(string)->format(TS_MW), for example. Or we can ditch the constants and add a dozen getters, including `getUnix():int` [02:28:57] or getTime() if we like similarity to `time()`. [02:29:46] but that feels like it'll be mistaken for time of day, even if easily realized, it's needlesly confusing for new comers. [02:32:46] or if we want to go full MW-tradition, we'd have a dozen Timestamp::newFromX() constructors instead of parse() [02:34:58] though `Timestamp::newFromUnix($ts)->format(TS_MW)` is quite a bit more verbose than `wfTimestamp($ts, TS_MW)` [02:36:07] if we assume that 99% of non-unix/mw is output-only, and inputs almost exclusively just unix or TS_MW, we could say that ctor/parse/format verbose is a reasonable fallback for edge cases. [02:38:03] and do something else for the common case like `Timestamp::format(int $ts, TS_MW)` where the static format() only accepts unix. If you need other input, you go chain from ::parse()->format() instead, maybe. [02:42:27] It appears Domas committed wfTimestamp [02:42:54] https://static-codereview.wikimedia.org/MediaWiki/4635.html [02:43:03] [02:43:03] Add internal timestamp wrapper wfTimestamp($outputtype,$timestamp); [02:43:03] Obsoletes: wfTimestamp2Unix, wfUnix2Timestamp, wfTimestampNow; [02:44:41] Even better. Timestamp::unixToMw, ::mwToUnix, ::now. And for everything else, ::convert($str, $formatFrom, $formatTo). [02:45:13] Thoes older functions looked so reasonable! [03:44:20] yes you can reject dates after 2286, that's what we did to fix that bug, but it's code and it's a nuisance [03:44:53] The user is saying make a block expire in the year 2286, their input is perfectly clear, we're just getting it confused along the way [03:45:19] the more reasonable interpretation is not really the year 1000 [19:24:50] I was thinking more that it's reasonable to reject dates after the year 300,000 when Unix reaches 14 digits, which we did already. [19:26:22] Inputs less than that should be unambiguous regardless whether they're 14-digit TS_MW or <=13 digit Unix [19:26:34] I'm missing something. [19:31:05] And closer to us it seems reasonable also to reject >9,999 for TS_MW; but i guess where it goes wrong is when > 9,999 in a way that is almost valid TS_MW is then forced to be misinterpreted as Unix [19:36:05] Hmm that didn't happen though. What happened is that a Unix ts for >9,999 was requested to be formatted as TS_MW which threw? That seems like that would/should happen even in a better version of the library.