On 08/02/07, Daniel Stenberg <[EMAIL PROTECTED]> wrote:
On Thu, 8 Feb 2007, [EMAIL PROTECTED] wrote:
To me, this move is a wrong move. This introduces a complicated system that
fewer people will understand/change and that will be bad for us in the long
run.
Some nits in the actual code follows:
> /* -----------------------------------------------------------------
> * vim: et sw=4 ts=8 sts=4 tw=78
> */
I realize you didn't add this chunk, but I personally think we should not have
stuff like this in source files.
> + if (m->flags&MENU_HAS_DESC)
> + menu_callback= m->callback_and_desc->menu_callback;
> + else menu_callback = m->menu_callback;
This latest line is normally written on two separate lines in other Rockbox
code, and I would appriciate if we stayed with that source code style. You use
this same style in multiple places.
> + DEBUGF("%x\n",setting->flags);
> + if (setting->flags&F_INT_SETTING)
> + {DEBUGF("boo");
These DEBUGF() calls seem like leftovers that should be removed.
forgot about them... I've already seen them and removed them.
> +#ifdef CONFIG_TUNER
> +MENUITEM_FUNCTION(load_radio_screen, ID2P(LANG_FM_RADIO),
> + (menu_function)radio_screen, dynamicitem_callback);
> +#endif
> +
> +#include "settings_menu.h"
... here you #include a header in the "middle" of the C file. Why?
Isnt there a comment with them saying its temporary?
> +struct choice_setting {
> + void (*option_callback)(int);
> + int count;
> + unsigned char **desc;
> +};
And this is suddenly no longer following the 4-space indent tradition.
Not sure what happened there... ill fix it
--
Daniel Stenberg -- http://www.rockbox.org/ -- http://daniel.haxx.se/
