Rockbox.org home
release
dev builds
extras
themes manual
wiki
device status forums
mailing lists
IRC bugs
patches
dev guide



Rockbox mail archive

Subject: Re: jdgordon: r17690 - in trunk/apps: . gui

Re: jdgordon: r17690 - in trunk/apps: . gui

From: Jonathan Gordon <jdgordy_at_gmail.com>
Date: Fri, 6 Jun 2008 11:26:31 +1000

2008/6/6 Dave Chapman <dave_at_dchapman.com>:
> IMO it's debatable whether this makes things simpler, it's adding code
> complexity, and just saves WPS authors a little typing (which they do once).

this allows the same .wps to be used on more targets than just one
>
> Given the heavy use of bitmaps (and backdrops) in most WPSs, I'm not
> convinced that many themes are capable of looking reasonable with any choice
> of colour, or that users want the ability to easily change the fg/bg colours
> of text in a WPS.

so WPS's which dont look good with any colours will set them, and
those that are not can leave them out

>
> So for (IMO) a very minimal gain, we now need to worry about corner cases
> and more ways for WPS authors and/or users to break things.
>
> e.g. when the user changes the fg/bg colour in the settings menu, the WPS
> viewports aren't updated with those changes (until the WPS is reloaded).
> Not a big issue, but it makes this a partly working feature.

Yes, I'll conceed that point, but IMO its no big deal. if it really
annoys people we can fix this in one of two ways... either reloading
when the colours change, or set the colour to some invalid value in
the wps struct so it knows to use the setting value instead of the
invalid value.

>
> It also makes the %V syntax inconsistent with the other WPS tags - you have
> to include the | separator around optional values.
>
> If I'm reading the code correctly, you've removed some of the error checking
> in the wps parser. For example if a WPS author types a colour incorrectly
> (e.g. only types 5 hex digits instead of 6), the wps parser will now happily
> accept it (defaulting to the appropriate theme colour), rather than giving a
> parser error. "missing" and "invalid" are two different things.

Yes, thats correct.. again, 2 options... only "accept" missing values
if they are completly missing, or add some printf warnings

Jonathan
Received on 2008-06-06


Page was last modified "Jan 10 2012" The Rockbox Crew
aaa