Rockbox

This is the bug/patch tracker for Rockbox. Click here for more information.

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#10722 - some improvements for gui/list.c

Attached to Project: Rockbox
Opened by Teruaki Kawashima (teru) - Sunday, 25 October 2009, 15:56 GMT+2
Last edited by Teruaki Kawashima (teru) - Tuesday, 27 October 2009, 14:35 GMT+2
Task Type Patches
Category User Interface
Status Closed
Assigned To No-one
Player Type All players
Severity Low
Priority Normal
Reported Version Daily build (which?)
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Private No

Details

this patch is some improvements for gui/list.c
* add function list_get_nb_lines and replace duplicated codes to calculate number of lines by it.
* refine calculation of gui_list->start_item[screen] in function gui_list_put_selection_on_screen.
- tries to show as many items as possible. hopefully fix  FS#10054  without further glitches, iiuc the issue.
* remove trailing spaces and fix indents.
   improve-gui-list.patch (15.9 KiB)
 apps/gui/list.c |  191 +++++++++++++++++++++++++-------------------------------
 1 file changed, 88 insertions(+), 103 deletions(-)

This task depends upon

Closed by  Teruaki Kawashima (teru)
Tuesday, 27 October 2009, 14:35 GMT+2
Reason for closing:  Accepted
Additional comments about closing:  committed in r23367
Comment by Thomas Martitz (kugel.) - Sunday, 25 October 2009, 19:50 GMT+2
If you want your patches reviewed (which I assume you do if you upload it here), then please separate indention & space fixes from actual code changes to make reviewing easier.

I'll have a look if nobody beats me to it, but remember the above for future patches.
Comment by Teruaki Kawashima (teru) - Monday, 26 October 2009, 12:05 GMT+2
I'm sorry. this is one without changes of spaces.
   improve-gui-list.2.patch (6.2 KiB)
 apps/gui/list.c |   89 ++++++++++++++++++++++----------------------------------
 1 file changed, 36 insertions(+), 53 deletions(-)

Comment by Thomas Martitz (kugel.) - Monday, 26 October 2009, 12:29 GMT+2
Thanks! Much easier to review now.

The patch looks very good. I assume it works, so go for it :)

On a unrelated note: gui_synclist_item_is_onscreen() seems to only check the lower bound of the visible list, not the upper. Is that correct? I don't think so. But that's not related to your patch.
Comment by Teruaki Kawashima (teru) - Monday, 26 October 2009, 14:39 GMT+2
quick fix of gui_synclist_item_is_onscreen. the function doesn't used as far as i greped.
   improve-gui-list.3.patch (6.2 KiB)
 apps/gui/list.c |   89 ++++++++++++++++++++++----------------------------------
 1 file changed, 36 insertions(+), 53 deletions(-)

Loading...