|
Rockbox mail archiveSubject: Re: (Another) New Bookmark Release - 2004-01-10Re: (Another) New Bookmark Release - 2004-01-10
From: Björn Stenberg <bjorn_at_haxx.se>
Date: Wed, 14 Jan 2004 01:19:24 +0100 Benjamin wrote: > This release mostly addresses the issues Björn had along with some other > changes. Excellent! I have now, finally, committed this code to CVS. I made a number of changes to the code though: - Replaced all short variables with int. Short has no advantage for internal variables, and causes type incompatibility in function calls. - Replaced your email address with your name in the copyright header. I don't know if it makes a legal difference, but at least it's now more consistent with the rest of the files. - Changed the exit button in the bookmark viewer from LEFT to OFF, since most other screens do that (except menus, but we should fix that). - Renamed /.rockbox/mrb.bmark to /.rockbox/most-recent.bmark, for clarity. - Removed all /* BM */ comments, naturally. - Reformatted and added some space to the code, including replacing all "if(a)" with "if (a)". (This is not a hard rule, just a pet peeve of mine not to make keywords look like function calls.) - Removed the following commented-out code: /* if(bookmark_count >= MAX_BOOKMARKS) break; */ I hope that's not supposed to be there. :-) - Removed the bookmark_autobookmark() call from the Disk Info screen(!) in debug_menu. I assume that was just forgotten debug code. - Reshuffled the following lines in randomise_playlist(): From this: if (write) if (playlist.num_inserted_tracks > 0 || playlist.deleted) playlist.shuffle_modified = true; { ...to this: if (playlist.num_inserted_tracks > 0 || playlist.deleted) playlist.shuffle_modified = true; if (write) { - Renamed the configuration file setting strings to conform with the format of all other settings. - Replaced "," with ";" as bookmark field separator. This allows a single separator for all fields, instead of using both "," and "\". It also breaks old bookmarks, but I figured it was better to do it now than after two weeks of discussion. :-) I probably broke something when doing all these changes, so test it thoroughly. I still have a few small issues with the bookmark feature: - I think the icon is nice, but it doesn't look like a bookmark at all. It's an excellent "document" icon, though! :-) - Playlist bookmark files are named <playlist>.m3u.bmark, which means that when browsing in "show only supported files" mode, the file is displayed as <playlist>.m3u. This is quite confusing, and isn't helped much by the icon. - I still haven't grasped the difference between List Bookmarks and Recent Bookmarks, or when to use which. Maybe there is an explanation I didn't read somewhere, but then most users don't either so I think we need to clarify it. :-) At last, a very big THANK YOU for writing this code and having the patience to adapt it to all my whims. :-) -- BjörnReceived on 2004-01-14 Page template was last modified "Tue Sep 7 00:00:02 2021" The Rockbox Crew -- Privacy Policy |