Rockbox mail archiveSubject: Re: Latest Bookmarking Patch
Re: Latest Bookmarking Patch
From: Eric Linenberg <elinenbe_at_umich.edu>
Date: Sat, 1 Feb 2003 10:59:14 -0500
freeware windows notepad replacement with unix text file support and
It is VERY lightweight
B> 1. I gotta find a better dos2unix tool :-). Anyway, I'll look into
B> getting rid of the CRLFs. Are they all in bookmark.h and bookmark.c or
B> are they in some of the other modified files?
B> 2. Autobookmark is basically an automatically generated bookmark. As a
B> result, it uses a lot of the same functions as a manually created and
B> loaded bookmark. The main differences is how automated it is and where
B> it is stored (at the beginning of the file). By passing a true-false
B> value, I can use a lot of common code. That and this has been an
B> evolutionary process so there is probably some places I could have done
B> it a little more efficiently.
B> 3. I'm not attached to how I get the buttons. I just found a sample
B> elsewhere in the code and used that. I believe I use button_get()
B> 4. I'd use DEBUGF() if I could, but I haven't been able to compile the
B> simulator on my Windows boxes and I don't have a serial out on my
B> archos. I'll probably end up removing bookmark_debug_print() by the
B> final release. I only use it when I'm debugging a problem.
B> Can you see any place else I might be doing a buffer overrun?
B> -----Original Message-----
B> From: owner-rockbox_at_cool.haxx.se
B> [mailto:owner-rockbox_at_cool.haxx.se] On
B> Behalf Of Daniel Stenberg
B> Sent: Saturday, February 01, 2003 7:22 AM
B> To: Rockbox
B> Subject: RE: Latest Bookmarking Patch
B> On Sat, 1 Feb 2003, Benjamin wrote:
B> Hi Benjamin!
B> Thanks for all your work on this feature. I do however have some remarks
B> on your recent patch (dated feb 1 2003):
B> * It still contains CRLF newlines all over.
B> * On several places in the code you have changed a section like:
B> with your new code that looks like this:
B> your stuff;
B> Evidently, you alter the flow and I'm not convinced that you should do
B> that. Is it on purpose?
B> * I suggest you use button_get(true) instead of button_get_w_tmo() if
B> not gonna do anything special after the timeout has elapsed.
B> * The bookmark_debug_print() function should probably be replaced with
B> DEBUGF() in case you really want it to remain.
B> Otherwise I think it looks mighty fine!
Received on 2003-02-01