- Status Closed
- Percent Complete
- Task Type Patches
- Category User Interface
- Assigned To No-one
- Operating System All players
- Severity Low
- Priority Very Low
- Reported Version Release 3.0
- Due in Version Undecided
-
Due Date
Undecided
- Votes
- Private
FS#9603 - list and menu code cleanup
This patch cleans up code in (each) list.c and menu.c.
-stop passing viewports unnecessarily within list code (if we pass a list, we already pass the parent of it with it, we don’t need to pass the parent seperately too)
-cleanup do_menu by using the viewport generated by synclist_init, remove the menu_vp which was marked as “will hopefully phased out” and removing the init_default_menu_viewports
-due to the above, viewport_set_defaults will now be able to handle the bars. That also enables us to get a real fullscreen initialized viewport(finally?).
Generally save multiple initializing of lists in the menu code.
Issuing redraws of menus *should* be unneeded at all now, since, with this patch, the actual used viewport gets modified.
This patch also yields a tiny binsize & ram usage decrease on my e200.
2008-12-31 05:59
Reason for closing: Accepted
Additional comments about closing: Warning: Undefined array key "typography" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 371 Warning: Undefined array key "camelcase" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 407
the good part of this patch has been
commited
Loading...
Available keyboard shortcuts
- Alt + ⇧ Shift + l Login Dialog / Logout
- Alt + ⇧ Shift + a Add new task
- Alt + ⇧ Shift + m My searches
- Alt + ⇧ Shift + t focus taskid search
Tasklist
- o open selected task
- j move cursor down
- k move cursor up
Task Details
- n Next task
- p Previous task
- Alt + ⇧ Shift + e ↵ Enter Edit this task
- Alt + ⇧ Shift + w watch task
- Alt + ⇧ Shift + y Close Task
Task Editing
- Alt + ⇧ Shift + s save task
Now with the patch
Dont’t forget about our touchscreen lists.
synced, works well
One minor point; I believe we conventionally we uppercase #define macros, so
#define parent (list→parent[display→screen_type])
should be
#define PARENT (list→parent[display→screen_type])
etc.
Also if youi left the parameter to gui_synclist_do_touchscreen() named gui_list, rather than renaming it, the patch would be smaller (and clearer IMHO).
only changed parent and list_display_title, dont know whether its right or not…
Yea, this parent define was just to save search & replace work during the creation (which is always also a bit error-prone), so I planned to remove that altogether when it’s working and when/if it’s going to get in. I’m aware that this isn’t conventional.
But since JdGordon told me it’s going to sit for a while I left it for now.
So, just tell me if it’s getting committed soon I’ll immediately clean then up!
JayKay, I think your tidy up may have confused PARENT and parent, but that’s just from a quick look at the diff - maybe kugel can clarify.
kugel - I’d like to see this committed too, but even if not, it’s always good to keep a patch as clean as possible. It makes it more readable (as a diff) and might reduce the chance of resync being needed…
Roger that, wait a few minutes :)
here you go! Sorry for the small delay :(
I think unifying the parameter names is actually a good idea. It’d also fit the idea of this patch, but if you prefer clean patches, I’m perfectly fine with it :)
BTW: With this patch, my customlist patch at
FS#8799runs just fine (which indicates that things are done properly with this patch here, at least for me :) ).Ooops, touchscreen again :)
Some more cleanup
tested on h300 & e200 sim. Real target testing is welcome.
saves about 100bytes binsize and 300bytes ram on h300.
tested on e200, works fine.
probably the last update
- enforce 80-char width coding guideline here and there
ok, im going to commit this patch some time before the weekend (unless i forget or something) which handles the statusbar how it should be handled. the “hide_bars” parameter needs to be removed completely from do_menu() which will always show the bars if enabled.
if you sync up the list cleanup to this patch I’ll commit both one after another