Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Patches
  • Category Plugins
  • Assigned To No-one
  • Operating System Gigabeat F/X
  • Severity Low
  • Priority Very Low
  • Reported Version Version 3.2
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by teru - 2009-04-17
Last edited by fml2 - 2009-06-24

FS#10138 - some minor fixes

this patch is some minor fixes to following plugins/files.

*text_editor
remember closing file descriptor.
*spacerocks
pause only if playing game when HOLD is enabled,
to fix bug that enabling and disabling HOLD in demo allows player to “resume” game.
*star
hide statusbars in menu so as not the bars to be drawn on title.
*rocklife
check result of open properly.
open in Rockbox seems to return negative value not only -1 when error.
iirc, open returns -2 when too many files are opened.
*apps/plugins/lib/helper.c
no need to check if rb is NULL since it should be initialized in loader.

*dict
*stopwatch
*keybox
*pegbox
replace ‘ROCKBOX_DIR “/rocks/(apps|games)/…“’ with ‘PLUGIN_(APPS|GAMES)_DIR “/…“‘.

*apps/keymaps/keymap-gigabeat.c
make source code look better.

*apps/plugins/spacerocks.c
*apps/plugins/star.c
*apps/plugins/sudoku/sudoku.h
*apps/plugin.c
*apps/plugin.h
replace tab with 4 spaces.

Closed by  fml2
2009-06-24 21:50
Reason for closing:  Accepted
Additional comments about closing:  

I've committed all patches (see comments) except "star_no_statusbar.patch" as it doesn't do anything on the targets I tried. If it's still a matter then please reopen the task. That said, thank you very much for your work.

teru commented on 2009-05-17 13:11

synched and updated patch.
now, this patch is some minor fixes to following plugins/files.

*apps/lang/english.lang
in “Set Time/Date” screen, Button A dose not revert setting but POWER dose.

*text_editor
make sure closing file descriptor.
remove code that changes statusbar setting.
*spacerocks
pause only if playing game when HOLD is enabled,
to fix bug that enabling and disabling HOLD in demo allows player to “resume” game.
*star
hide statusbars in menu so as not the bars to be drawn on title.
*rocklife
check result of open properly.
open in Rockbox seems to return negative value not only -1 when error.
iirc, open returns -2 when too many files are opened.
*pictureflow
it seems to returning wrong variable.

*apps/plugins/lib/helper.c
no need to check if rb is NULL since it should be initialized in loader.

*dict, stopwatch, keybox, and pegbox
replace ‘ROCKBOX_DIR “/rocks/(apps|games)/…“’ with ‘PLUGIN_(APPS|GAMES)_DIR “/…“‘.

*apps/keymaps/keymap-gigabeat.c
make source code look better.

I committed all the parts of this patch I was familiar with. Someone else will have to take a look at the others. I’ve uploaded a patch with the uncommitted changes.

However, in the future, could you separate different changes into different patches? You can post several patches to the same task if you like, and it makes it much easier to review and commit.

spacerocks.c, lib/helper.c, dict.c, stopwatch.c, keybox.c & pegbox.c look good to me; can’t comment on the others..

Committed those then.

teru commented on 2009-05-28 14:28

Thanks for commiting.
> However, in the future, could you separate different changes into different patches? You can post several patches to the same task if you like, and it makes it much easier to review and commit.
I’ll keep in mind, sorry.

details for the remaining part of patch.
*star
In the function do_menu in apps/menu.c, there is a comment as follows.
/* if hide_bars is true, assume parent has been fixed before passed into
* this function, e.g. with viewport_set_defaults(parent, screen, true) */
But viewports in star don’t seem to be fixed. I started star on my gigabeat x and statusbar was displayed over the title after a few seconds. (statusbar in settings was enabled, of course)
I thought easiest way to fix is to set false to hide_bars.

*first hunk of text_editor
if the file conatins neither CR nor LF (i.e. one line or empty), file descriptor will not be closed.
*rest of text_editor
maybe it’s not good to change setting without asking to do so?

teru commented on 2009-06-14 14:35

*solitaire_delete_save_file.patch
every time i quit solitaire, it tries to delete save file (even if it dosen’t exist) and cause disk access. it is sometimes annoying.

*solitaire_update_screen.patch
since spash was modified to use viewport, whole display is not cleared before “You Won :)” and cards and cursor remain.

*star_no_statusbar.patch
hide statusbar because it is displaied ovarlapping the title of menu if statusbar in settings is enabled.

*text_editor_close_fd.patch
if the file is empty or contains only one line, file descriptor will not be closed when cheking EOL type.

*dont_change_statusbar_setting
remove code that changes statusbar setting from text_editor and properties.
it’s not good to change setting without asking to do so. should use viewportmanager_set_statusbar instead. and plugin loader hides statusbar with it, no need to reset it.

fml2 commented on 2009-06-24 15:19

The “text_editor_close_fd.patch” has been committed with r21491.

fml2 commented on 2009-06-24 20:18

The “dont_change_statusbar_setting.patch” has been committed with r21493.

fml2 commented on 2009-06-24 21:15

“solitaire_delete_save_file.patch” has been committed with r21497.

fml2 commented on 2009-06-24 21:16

I don’t see any sense in “star_no_statusbar.patch” Everything seems to work without it just fine.

fml2 commented on 2009-06-24 21:19

“solitaire_update_screen.patch” has been committed with r21498.

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing