Fix parsing Journals using a little-endian date format #694
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Ever since version 2.0, when parsing a journal file, jrnl would not use the custom date format string anymore. Instead, it relied on the dateutil library to get the parsing right. This change was made to allow people to change their date format without having to manually change their file. However, this broke some existing date formats like %d.%m.%Y, as it would falsely interpret the month as day and vice versa. This commit adds some tests to catch this error and fixes it by trying to parse the dates using the custom format first, only falling back to dateutil when needed.
pspeter
changed the title
|
I'm out of the country right now, but should be back in a few days. v2.1 is
already in beta, and soon to be released, so it's too late for that one.
I haven't had a chance to look at this yet, but if all looks good, I don't
see why we can't bump it up into v2.1.1
|
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes parts of #630 and one source of failure during upgrading to 2.0 (#664).
Ever since version 2.0, when parsing a journal file, jrnl would not use the custom date format string anymore. Instead, it relied on the dateutil library to get the parsing right. This change was made to allow people to change their date format without having to manually change their file. However, this broke some existing date formats like %d.%m.%Y, as it would falsely interpret the month as day and vice versa (when %d <= 12). This commit adds some tests using the little endian date format and fixes the behavior by trying to parse the dates using the custom format first, only falling back to dateutil when needed.
Note that this does not yet allow users to make new entries using the little endian format yet. It will still use the dateutil parser for this. As I usually create new entries with no timestamp or just "today" or "yesterday" as date, this is an okay trade-off for me, but still not ideal. At least upgrading and querying works again properly this way.