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



Rockbox mail archive

Subject: Re: (Another) New Bookmark Release - 2004-01-10

Re: (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örn
Received on 2004-01-14

Page template was last modified "Tue Sep 7 00:00:02 2021" The Rockbox Crew -- Privacy Policy