-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Week methods only work with local translator, not default locale #1967
Comments
Hello, This is a legacy. At this time Monday as the standard start of week was the default value for So changing this behavior would not be a bug fix but really a breaking changes. Many users use This change could break apps that legitimately rely on this. An other solution must be found or it should wait a version 3. |
Considered solution: In Carbon 3, the default locale could be en_001 “International English”. And default start of week and every calendar/format features would be aligned on this locale as a standard. Then our "en" locale could be identical to the "en_001" and so we would keep "en" as default locale (as in Laravel config) but it would now mean "en_001" and no longer "en_US" as in Carbon 2, and so default start of week would match the default locale. There is basically no wording difference between en_001 and en_US, so it should be a smooth change. |
While working on this for 3.0 I actually realized I could use an opt-in mode to enable the behavior of 3.0 in the next 2.35 version. With: Carbon::setWeekStartsAt('auto');
Carbon::setWeekEndsAt('auto');
Open PR for this fix: #2090 |
…nd-auto-mode Fix #1967 Add auto mode for start/end of week
This can be tested requiring |
…sed-on-locale Fix #1967 base week on locale
Hello,
I encountered an issue with the following code:
Trying to figure out all of the different (apparently competing?) localization settings makes my head spin. The issue seems to be that "firstWeekDay" will only ever be determined by a local translator, not the default translator:
Carbon\Traits\Localization\Date:928
I'm not sure what the point of
static::$weekStartsAt
is...backward compatibility? It conflicts with what the default translator is supposed to return.As far as I can tell, the only way to solve this is to use the deprecated methods
setWeekStartsAt()
ANDsetWeekEndsAt()
(which for some bizarre reason is not calculated automatically), or explicitly include thelocale()
call every single time I use these methods (even though it already is the default). Am I missing something obvious here?I would propose removing the default values for
static::$weekStartsAt/EndsAt
, and changing the getters to only use them if they are explicitly set:This is a simple change and I would be happy to submit a PR if acceptable, but it would technically be breaking, even though it's a bugfix in my opinion.
Carbon version: 2.28.0
PHP version: 7.4.0
I expected to get: (with default 'en' or 'en_US' locale)
But I actually get:
Thanks!
The text was updated successfully, but these errors were encountered: