Rockbox.org home
Downloads
release
dev builds
extras
themes
Documentation
manual
wiki
device status
Support
forums
mailing lists
IRC
Development
bugs
patches
dev guide
Search



Rockbox mail archive

Subject: Re: (Another) New Bookmark Release - 2004-01-10
From: Björn Stenberg (bjorn_at_haxx.se)
Date: 2004-01-14

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örn

Page was last modified "Jan 10 2012" The Rockbox Crew
aaa