Rockbox

  • Status Unconfirmed
  • Percent Complete
    0%
  • Task Type Patches
  • Category User Interface
  • Assigned To No-one
  • Operating System All players
  • Severity Low
  • Priority Very Low
  • Reported Version Daily build (which?)
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by tieum - 2011-05-07
Last edited by sideral - 2011-05-10

FS#12101 - Option for date format

Attached is a patch that:

* adds an option to chose the date format in the system menu (choice between YMD, DMY and MDY)
* adds a %cF theme tag so that theme can know in which order to display the date

There are a few new strings that I only added in English.

This is my very first rockbox patch so please let me know if anything should be improved.

I did the patch against svn 29827 and tested it in a simulated Fuze v2 as well as a real one.

fml2 commented on 2011-05-07 19:11

I have several thoughts on this.

1. I'm not sure we need such configurability in the menu. The date is set once and is then just there. Where we do need configurability is in the WPS or in the status bar, but then we already have tags for specific parts of the date, so the theme authors can display it how they want. The same applies to the time format, so I'd exclude that option too.

If we'd still include this feature, I have some (pure subjective) comments on the code style.

2. I'd introduce constants (or enums) for the date formats, e.g. DATEFORMAT_DMY, …_MDY, …_YMD

3. I would rename the lang symbols from LANG_DATE_YMD to LANG_DATEFORMAT_YMD etc.

4. I would rename the wps token to SKIN_TOKEN_RTC_DATEFORMAT

5. I would place the menu option for the date format *before* the option for the time format

6. In the switch (global_settings.dateformat) I'd probably introduce local variable for day, month, and year, and then use them in sprintf – to avoid code duplication. Same for the 'talk_date'.

Otherwise the code looks OK to me (I have not tried it though).

But: see 1 above.

tieum commented on 2011-05-07 19:20

For time there is %cf that allows themes to know if they should display the date in 12 or 24 format (although all themes does not take advantage of this). The thinking being this option is to make exactly the same available for the date. Now the date display in themes is fixed depending on what the theme authors choose. For sure I can hack in my preferred theme and change the order but it would be nice if I can use any theme and choose to display the time and date the way I am used to it and not the way the theme authors choose too.

You can read all settings from the WPS via %St, no need for a new tag.

tieum commented on 2011-05-08 18:20

I did nit know this. Thanks. But you still need to have the option available for the user to choose which this patch does then it is easier to have a dedicated option.

Set the date format is very great idea for non-english speaking countries / users.

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing