On 4/6/2010 12:33, Magnus Holmgren wrote:
On 2010-04-05 22:53, [email protected] wrote:
New Revision: 25494
Log Message: Restructure some bookmarking code, preparatory to adding
version info to bookmarks. Saves some bin size as a bonus. No
functional changes yet.
Some comments, though I don't know exactly what you plan to change
(and are you aware of FS#9407?):
Hm, are the flags really needed? With the struct, it seems like
F_BMFILES flag is the only one... But if they are kept, longer names
would be nice.
Why not pass struct bookmark_values as an argument? No need to keep it
as a static then, and safe to skip (most of) the flags.
resume_first_index can be removed, it isn't used.
Finally, I prefer to not "hide" function calls in macros. Doesn't seem
motivated here, IMHO.
I posted a patch at FS#11178 to show what I had in mind. That probably
shows where I want to go with it better than text.
The flags are required to tell parse_bookmarks which tokens to assign to
the variables. On second thought, there's no good reason not to
populate them all the time. It doesn't really hurt anything to do so.
Some flags will be necessary to process optional tokens. The variables
are in a struct only because the original plan was to pass the struct as
a parameter, but a list of static variables would do just as well.
first_index is gone in the version I working on now.
I macro'd the check_bookmark function to make it clearer what was trying
to be accomplished and to get rid of the actual function.
