Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Calendar week numbers #1877

Closed

Conversation

prohornikitin
Copy link
Contributor

@prohornikitin prohornikitin commented Dec 7, 2022

I'm sorry, but my previous PR #1868 introduced more bugs than fixed. Luckily @LukashonakV told me that the issue itself was wrong and there have to be custom format for a week number in config.
So, I reverted almost all changes and fixed week number calculation bugs (both mine and previous).
And now it should work fine for any locale. If there is no custom format, it will apply default format which is %U or %V depending on the first day of the week.
It fix #1874 and other bugs that have not been reported yet.
Sorry again for the previous PR. I hope this is better :)

almost revert my previous change, but fix a few issues with week calculation
which is %U or %V depending on the first day of the week
@Alexays
Copy link
Owner

Alexays commented Dec 8, 2022

@LukashonakV is it ok?

@LukashonakV
Copy link
Contributor

My apologies for late answer. For some reason I didn't get any notifications via email. For me it looks OK.
The only thing I'm thinking of about calculation of the week number. It's implicit relates to the ISSUE#762 of the date library. Out current approach is to provide week number is based on the last week day of the week . Actually looking at example I am inclined to think of changing the approach and provide week number is based on the first day of week. For the first calendar row it should be calculated on the first day of the month. What do you think @Alexays , @prohornikitin ?

LukashonakV added a commit to LukashonakV/Waybar that referenced this pull request Dec 10, 2022
 1. Lets do code simplier
 2. Added regexp in order to align week days in case when user provide
 some additional characters in week format strings + to calculate
 default week formats
 3. Week numbers now are base on the first day of the week. For the
 first day of month week date is calculated on this date
 4. Week format has default values: ":%U", ":%V"
@LukashonakV
Copy link
Contributor

LukashonakV commented Dec 10, 2022

Let's check #1882 as a summarized set of changes with defaults formats and some improvements which taking into account @prohornikitin 's changes
Once it will be merge into the main branch I'll complete Wiki module clock with the new information

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

format-calendar-weeks won't accept format strings
3 participants