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.
+#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?
+struct choice_setting {
+ void (*option_callback)(int);
+ int count;
+ unsigned char **desc;
+};
And this is suddenly no longer following the 4-space indent tradition.
--
Daniel Stenberg -- http://www.rockbox.org/ -- http://daniel.haxx.se/
