Enhancement: Select First Day of Week for Locale

[lainsce]

This eventually will change the order of days on Calendar and Date Time indicator popover by issuing a setting that gets read by both. Fixes #86 .

Looks like:

[lainsce]

The way this works is:

1. io.elementary.switchboard.locale/first-day changes, and sets a (translatable) string of the day that it is ("Sunday" is default)
2. This will propagate to whatever place reads this such as a calendar widget or some such, and reorders the columns to reflect the setting.
3. ???
4. Profit!

Now all you need, is to use this on apps or indicators that have a calendar grid, to reorder the grid based on this setting.

[lainsce]

Actually, going to need some help with save-stating this on the checkbox, as currently it jumps to Sunday.

[lainsce]

Ready to review.

[lainsce]

Could you guide me on how to make a ranged gschema? I've never done that type of thing before.

As for the rest about checking on code if it's on the range, I'd wager it should be fixed if the gschema is ranged, wouldn't it?

[jeremypw]

I am not quite sure what happens if code tries to write an out of range value to a setting so it is better to be sure this cannot happen one way or another.

[lainsce]

There we go.

[lainsce]

After addressing things, there's a small lag on first open, but shouldn't be anything bad.

[lainsce]

After porting to a enum key, it seems that lag on first open vanished somehow :smile:

[lainsce]

Removed the options that aren't matching real world use.

As long as Maldives keeps using Friday, that option will be here.

[jeremypw]

I notice that all the monthly wall calendar images I've looked at for Maldives show Sunday as the leftmost column and the weekend as Friday/Saturday despite what Wikipedia says about Friday being the first day of the week :confused: However, I agree we should keep Friday as an option for now although how to interpret that in the Calendar widget might be tricky.

[lainsce]

Now the first day store stores both the index and name of the first day.

Feeling like new reviews are making the PR complex and not atomic as needed to be. :wink:

[jeremypw]

I apologise - I forgot you had switched to storing the first day as an enum so in fact you do not need to store the day of the week as it is not used in this plug :disappointed: You can revert the latest changes. However, any app using this setting will have convert the enum to the appropriate day of week.

[jeremypw]

Just pushed a PR to this branch with a suggested simplification for you consideration. It has the advantage that the value of the enum retrieved from the settings is already the correct day number so no conversion is required by client apps.

[lainsce]

Yeah, that PR seems much more sane than what I was doing :P

Merged it here.

[jeremypw]

I think the code is now OK for storing a single global first-day. This is an improvement on the current situation where it is not possible to change the default first day at all. A further enhancement would be for the correct first day to be automatically set according to the chosen locale. I guess this would mean creating and storing a table storing (or hard-coding) the correct first day for each locale available (or is there a library function for this??). However, this could be left for a further PR.

I'll flag this for UX input.

Need to block a further stable release until this setting actually has an effect on the calendar app.

[lainsce]

Need to block a further stable release until this setting actually has an effect on the calendar app.

There is a further PR involving the change applying to Calendar here: https://github.com/elementary/calendar/pull/638 (Calendar does set things properly, but CI build fails on there.)

And another one for the Date Time indicator here: https://github.com/elementary/wingpanel-indicator-datetime/pull/240 (Will need a further PR for handling placing the events properly.)

:smile:

[jeremypw]

Yes, once this is merged I'll review those PRs.

[danirabbit]

I guess this would mean creating and storing a table storing (or hard-coding) the correct first day for each locale available (or is there a library function for this??). However, this could be left for a further PR.

First week information is already stored in LC_TIME, but I have no idea how to read it. I can't seem to find anything on Valadoc about it. Maybe @tintou or @ryonakano knows

[lainsce]

Review points addressed. :+1:

[jeremypw]

If we can read the first day of the week from Posix then we do not really need a combobox ...

There a Posix enum NLTime.FIRST_WEEKDAY in posix.vapi - maybe this is useful?

[lainsce]

The thing is that it's not the problem this PR is solving really reading from Posix and automatically setting things; it's solving the "I'm Spanish, but my system is purposefully in English because I'm studying it or my company set it so, or whatever the reason, so calendars starts on Sunday instead of my country's week start. I can customize via the combobox to make it familiar to me, and keep using my system in English.".

Also, we already do that, but now we can show to the user the setting via this, in a future PR as well.

[lainsce]

Please understand that further efforts on Locale and Account Services or ideas like that would bloat this PR and make it not follow the (unwritten?) guideline of a PR doing one thing per PR as I was told many times to do.

[mcclurgm]

I know this is out of scope, but since there's a discussion already here I don't know where better to put it. I put a bit of info about the posix locale in the original issue. There's also some stuff about this over in the calendar code.

Also, there's a potential issue with the WEEK_1STDAY variable here. According to posix, the numbering of each day of the week depends on the value of that variable. So either we could take that into account here, or assert that every day of the week has the same number no matter what posix says. I put a comment to this effect in the Calendar PR.

[jeremypw]

I am happy for this to be merged on the assumption that further work will be done to ensure that the default setting for this is correct.

[mcclurgm]

I'm happy to do that since I did the week start stuff in calendar, but it will be a while before I can guarantee. But in theory a "correct" solution that accounts for the posix stuff should be pretty similar to what's in that codebase, assuming I did it right.

[jeremypw]

Good - just needs to @danrabbit to sign off on it now.

[danirabbit]

I feel like this should start at 1, not 0 because as @mcclurgm points out in that calendar code block GLib uses 1-7 and we could reserve 0 for a future "automatic/from locale" setting

[lainsce]

I feel like this should start at 1, not 0 because as @mcclurgm points out in that calendar code block GLib uses 1-7 and we could reserve 0 for a future "automatic/from locale" setting

But AFAIK, the Evolution key starts at 1 for Monday. So which futureproofing are we going to push towards? Besides, I think work towards an Automatic Locale thing for the combobox can be done in a new PR.

[mcclurgm]

But AFAIK, the Evolution key starts at 1 for Monday. So which futureproofing are we going to push towards?

Not sure what other future proofing you're referring to? It seems to me like matching Evolution would be a good thing.

I'd personally support changing to 1 = Monday, since then we could use it directly as a GLib.Weekday in any clients that access the setting. But if we change it, that will break what's in the active Calendar PR. (The current setting is equivalent to what's in all the Ubuntu locales I've seen, which is 1 = Sunday.) But it would simplify the code needed in Calendar, since it could just see the setting and immediately short-circuit with that result. (Basically, this.week_start = (GLib.Weekday) setting; return;)

[danirabbit]

Yeah I think 1-7 starting with Monday seems like the best option since it matches Evolution and GLib.Weekday

[lainsce]

Alright, changes added in. :)