fix: iso week usage #1956
Open
fix: iso week usage #1956
+36
−19
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
Fava appears to use ISO 8601 weeks (e.g. `2025-W01`), but unfortunately does so incorrectly due to a mismatch between ISO 8601 and Gregorian calendar week definitions. ISO 8601 defines precisely how to handle weeks, namely, that the first week of a year is the week that contains 4 January. This definition results in some occasional mismatches with the Gregorian calendar such as: - The first week of 2025 starts on 30 December 2024 - 3 January 2021 belongs to Week 53 of 2020 Fortunately, the fix is straightforward as `strftime` provide[^posix]: - `%G` for the week-based year - `%V` for the week number of the year (with no 'week 0' as `%W` can return). This change will ensure that there is no ambiguity on weeks that straddle the new year, though at the cost of introducing occasional off-by-one errors compared to previously generated data. [^posix]: See [POSIX.1-2024](https://pubs.opengroup.org/onlinepubs/9799919799/functions/strftime.html) Signed-off-by: JP-Ellis <[email protected]>
yagebu
requested changes
Feb 8, 2025
There was a problem hiding this comment.
Thanks for the PR - this came up in #1805 and I agree with switching to ISO week numbers. The current behaviour is as documented in the help page on the filters and that will have to be updated as well, could you take a look at that? Other than that, the PR looks good
|
Amazing! Sure happy to look at the docs too. I'll probably do this tomorrow or Monday. |
|
Docs have been updated 👍 |
yagebu
reviewed
Feb 9, 2025
Add docs explaining how ISO 8601 week dates are calculated, and give examples of when they differ slightly from what one might naively expect looking at a Gregorian calendar. Signed-off-by: JP-Ellis <[email protected]>
JP-Ellis
force-pushed
the
fix/iso-week
branch
from
1e234b4 to
5d0fb3d
Compare
|
Any further updates needed before this can be merged? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fava appears to use ISO 8601 weeks (e.g.
2025-W01), but unfortunately does so incorrectly due to a mismatch between ISO 8601 and Gregorian calendar week definitions.ISO 8601 defines precisely how to handle weeks, namely, that the first week of a year is the week that contains 4 January. This definition results in some occasional mismatches with the Gregorian calendar such as:
Fortunately, the fix is straightforward as
strftimeprovide1:%Gfor the week-based year%Vfor the week number of the year (with no 'week 0' as%Wcan return).This change will ensure that there is no ambiguity on weeks that straddle the new year, though at the cost of introducing occasional off-by-one errors compared to previously generated data.
Fixes #1805
Footnotes
See POSIX.1-2024 ↩