The format of timestamps for delayed XMPP messages differs slightly between ejabberd versions. While old versions don't include microseconds, newer ones do. This patch ensures 0ad shows the correct timestamp no matter if it contains microseconds or not.
Details
- Reviewers
elexis - Commits
- rP20776: Support both DateTime formats of XEP-0082 in the lobby, refs #3156, rP16507…
- Trac Tickets
- #4920
Apply the patch and connect to a lobby run by an old ejabberd (like the official lobby) and one run by a recent ejabberd. Notice that both show correct timestamps for messages in the backlog.
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
XEP numbers and links should be documented in the code (see ticket, see documentation of the function).
As mentioned in the ticket, it might be a gloox bug, not our bug.
Looks like the duplication can't be removed with a ternary.
But we have a GLOOXVERSION macro which would make that a bit cleaner.
Or maybe some pattern matching is supported ? (IIRC that format ends up in ICU)
Well, they are. It's just that XEP-0082 allows the date time to contain microseconds as well.
As mentioned in the ticket, it might be a gloox bug, not our bug.
It's our, see ticket for more details.
Looks like the duplication can't be removed with a ternary.
But we have a GLOOXVERSION macro which would make that a bit cleaner.
Doesn't matter which gloox version it is. It's just related to the ejabberd version.
Or maybe some pattern matching is supported ? (IIRC that format ends up in ICU)
Well, I don't know. I just chose the easiest path.
I see. Can you try to take a look to unify that, preferably by just updating the pattern? You will also find some related differential revisions using blame.
Internally L10n::ParseDateTime uses [[ https://ssl.icu-project.org/apiref/icu4c/classSimpleDateFormat.html | SimpleDateTime from ICU ]]. As far as I understand SimpleDateFormat doesn't offer the ability to match multiple different datetime formats with a single expression. The internet seems to agree with that (e.g. see https://stackoverflow.com/questions/36337511/parse-an-iso-8601-date-time-with-optional-fractional-second-using-icu and https://stackoverflow.com/questions/26895428/how-do-i-parse-an-iso-8601-date-with-optional-milliseconds-to-a-struct-tm-in-c). So I still think the current implementation is the most straight-forward one, but I'm open to any alternative.
Too bad. I can confirm reading the links and https://ssl.icu-project.org/apiref/icu4c/classicu_1_1SimpleDateFormat.html
Well, then as ICU doesn't support that, it could use a macro (for instance)
#define doSomething(format)\ .... doSomething(format1); doSomething(format2); #undef doSomething
or a range-based loop, i.e.
for (someStringType format : {"format1", "format2"} )
I guess the loop would look cleaner.
Thanks for the patch Dunedan! Makes it a bit less effort to upgrade :-)
Assuming you tested with your more recent ejabberd version, I only tested against ours.
source/lobby/XmppClient.cpp | ||
---|---|---|
940 ↗ | (On Diff #5102) | Don't need a copy, a reference is sufficient, so & +3 spaces |
941 ↗ | (On Diff #5102) | tabs instead of spaces |