|
Rockbox mail archiveSubject: Re: jdgordon: r12227 - in trunk/apps: . menus recorderRe: jdgordon: r12227 - in trunk/apps: . menus recorder
From: Daniel Stenberg <daniel_at_rockbox.org>
Date: Thu, 8 Feb 2007 08:37:59 +0100 (CET) On Thu, 8 Feb 2007, mailer_at_svn.rockbox.org 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/Received on 2007-02-08 Page template was last modified "Tue Sep 7 00:00:02 2021" The Rockbox Crew -- Privacy Policy |