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: Latest Bookmarking Patch
From: Benjamin (mailinglists_at_samuraipanda.com)
Date: 2003-02-01


I misread your mail. I'll explain why I changed the code from
global_settings.resume in tree.c.

There are two places where I made this change, both occur when loading a
file to be played (MP3 or M3U). In both cases, I want to intercept the
call to ensure that a auto-load is not necessary. If one is, then load
to the bookmark and play at that point. If autoload is not enabled, or
the user doesn't want to auto-load, then play as normal. In either
case, I eliminated the global_settings.resume check because I grab the
global_settings.resume_file information, which wouldn't be set if resume
is not enabled. Does that make sense?

Ben

-----Original Message-----
From: owner-rockbox_at_cool.haxx.se [mailto:owner-rockbox_at_cool.haxx.se] On
Behalf Of Daniel Stenberg
Sent: Saturday, February 01, 2003 7:22 AM
To: Rockbox
Subject: RE: Latest Bookmarking Patch

On Sat, 1 Feb 2003, Benjamin wrote:

Hi Benjamin!

Thanks for all your work on this feature. I do however have some remarks
on your recent patch (dated feb 1 2003):

* It still contains CRLF newlines all over.

* On several places in the code you have changed a section like:

  if(global_settings.resume)
    bla-bla;

  with your new code that looks like this:

  if(bookmark_autoload)
    your stuff;
  else
    bla-bla;

  Evidently, you alter the flow and I'm not convinced that you should do
  that. Is it on purpose?

* I suggest you use button_get(true) instead of button_get_w_tmo() if
you're
  not gonna do anything special after the timeout has elapsed.

* The bookmark_debug_print() function should probably be replaced with
  DEBUGF() in case you really want it to remain.

Otherwise I think it looks mighty fine!

-- 
 Daniel Stenberg -- http://rockbox.haxx.se/ -- http://daniel.haxx.se/



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