|
Rockbox mail archiveSubject: RE: Latest Bookmarking PatchRE: Latest Bookmarking Patch
From: Benjamin <mailinglists_at_samuraipanda.com>
Date: Sat, 1 Feb 2003 07:50:33 -0800 1. I gotta find a better dos2unix tool :-). Anyway, I'll look into getting rid of the CRLFs. Are they all in bookmark.h and bookmark.c or are they in some of the other modified files? 2. Autobookmark is basically an automatically generated bookmark. As a result, it uses a lot of the same functions as a manually created and loaded bookmark. The main differences is how automated it is and where it is stored (at the beginning of the file). By passing a true-false value, I can use a lot of common code. That and this has been an evolutionary process so there is probably some places I could have done it a little more efficiently. 3. I'm not attached to how I get the buttons. I just found a sample elsewhere in the code and used that. I believe I use button_get() occasionally. 4. I'd use DEBUGF() if I could, but I haven't been able to compile the simulator on my Windows boxes and I don't have a serial out on my archos. I'll probably end up removing bookmark_debug_print() by the final release. I only use it when I'm debugging a problem. Can you see any place else I might be doing a buffer overrun? 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/Received on 2003-02-01 Page template was last modified "Tue Sep 7 00:00:02 2021" The Rockbox Crew -- Privacy Policy |