|
Rockbox mail archiveSubject: BookmarksBookmarks
From: Björn Stenberg <bjorn_at_haxx.se>
Date: Wed, 29 Jan 2003 10:08:09 +0100 Benjamin wrote: > Trust me (since I developed the code), it's probably best that it > not make it into the next public release. It's a patch that is less > then perfect (I'm working on that) and can be done better. I dove into the patch yesterday and here's a list of things I want addressed before merging: - Auto Bookmark is one option. It should occupy one bit, not eight. If you feel the triplet yes/ask/no is a must-have instead of a simple boolan yes/no, that is still just two bits. - The Auto Load settings should be removed. Use a single Auto Bookmark setting. - Don't use the resume settings as temporary variables when you load bookmarks - Put bookmarks in a separate sub menu, below recording settings - Default auto bookmark setting should be "off" - Name the attribute constant in the same form as the already existing ones, i.e TREE_ATTR_BMARK - Restore the deleted "restore=true; break;" before "case TREE_ATTR_BMF:" - Change bookmark format to be as simple as possible. (why are you storing queue resume index?) - Use standard comma-separated syntax in the file. If you need a filename, put it last on the line. Then it can contain anything. - Don't assume the first bookmark in the file is the auto bookmark. Mark it specifically. - Parse the bookmark file with strtok_r(), not manually byte-by-byte - Simplify the code more. bookmark_create() should be max two snprintfs and two writes. Not 14. - Create a bookmark struct instead of passing 14 parameters to many functions. - Don't copy read_line() from settings. Export the one in settings instead. - Hardcode the bookmark extension and separator to ".bmark" and ",". I appreciate your current flexibility, but it clutters the code. - Move all strings to the english.lang file - Fix the code to work on players too (max 2 lines of text, different keys) Additionally, I want to separate between file and playlist bookmarks. Also we need to solve the location problem. I suggest a single setting choosing between creating the file in /.rockbox or the same dir as the .m3u/.mp3. The .bmark file contains the path+name of the file, but only uses it if there is not an .mp3/.m3u with the same name in the same dir. Then we get both models in a single solution. -- BjörnReceived on 2003-01-29 Page template was last modified "Tue Sep 7 00:00:02 2021" The Rockbox Crew -- Privacy Policy |