Add support for ZonedDateTime in EventHandler
#136
Conversation
Signed-off-by: Arnau Mora <[email protected]>
Signed-off-by: Arnau Mora <[email protected]>
Signed-off-by: Arnau Mora <[email protected]>
rfc2822
left a comment
rfc2822
left a comment
There was a problem hiding this comment.
See #135 (comment)
The "parse and catch exception" approach doesn't seem to be a proper solution, at least not in the handle().
I think we should define a separate parseStartDate() method that
- has well-defined inputs and outputs,
- supports the required input formats,
- is tested separately.
Within that method, we should use other methods to determine the input format. If that's really not possible, we can still use the exception method.
lib/src/main/kotlin/at/bitfire/vcard4android/contactrow/EventHandler.kt
Outdated
Show resolved
Hide resolved
lib/src/test/kotlin/at/bitfire/vcard4android/contactrow/EventHandlerTest.kt
Show resolved
Hide resolved
Signed-off-by: Arnau Mora <[email protected]>
|
Should be ready. I've added tests for all formats from AOSP Contacts which should cover the original issue (bitfireAT/davx5-ose#1797 - |
lib/src/main/kotlin/at/bitfire/vcard4android/contactrow/EventHandler.kt
Outdated
Show resolved
Hide resolved
lib/src/main/kotlin/at/bitfire/vcard4android/contactrow/EventHandler.kt
Outdated
Show resolved
Hide resolved
lib/src/main/kotlin/at/bitfire/vcard4android/contactrow/EventHandler.kt
Outdated
Show resolved
Hide resolved
lib/src/main/kotlin/at/bitfire/vcard4android/contactrow/EventHandler.kt
Outdated
Show resolved
Hide resolved
lib/src/main/kotlin/at/bitfire/vcard4android/contactrow/EventHandler.kt
Outdated
Show resolved
Hide resolved
rfc2822
left a comment
rfc2822
left a comment
There was a problem hiding this comment.
Looks good, a few small things
| return OffsetDateTime.parse(dateString, formatter) | ||
| } catch (_: DateTimeParseException) { | ||
| // ignore: given date is not valid | ||
| continue |
There was a problem hiding this comment.
Can continue be omitted? I think it shouldn't change anything.
| * @param dateString The date string to parse. | ||
| * @return The parsed [PartialDate] or `null` if parsing fails. | ||
| */ | ||
| internal fun parsePartialDate(dateString: String): PartialDate? { |
There was a problem hiding this comment.
Should be @VisibleForTesting, too
| * @return The parsed [PartialDate] or `null` if parsing fails. | ||
| */ | ||
| internal fun parsePartialDate(dateString: String): PartialDate? { | ||
| var dateString = dateString // to allow modification |
There was a problem hiding this comment.
I'd prefer only constants. So maybe something like val withoutZ = ..., then val preparedDateStr = ... and then parse it.
| dateString = dateString.removeSuffix("Z") + "+00:00" | ||
| } | ||
|
|
||
| val regex = "\\.\\d{3}".toRegex() |
There was a problem hiding this comment.
Shall we drop {1,3} or only with 3 decimal places?
|
|
||
| val regex = "\\.\\d{3}".toRegex() | ||
| if (dateString.contains(regex)) { | ||
| // partial dates do not accept nanoseconds, so strip them if present |
2 participants
See #135 and bitfireAT/davx5-ose#1797.