Hey! I get so lost in the option parsing code also. The new funnel version im slowly working on has no config options at the moment, so I am flexible :)
I think GOptionGroup's are good, as you get the per-group help for free, and like you say having the user_data param gives you extra flexibility. My 2cents On Thu, Jul 30, 2009 at 2:00 PM, Kay Röpke<[email protected]> wrote: > Hi! > > (Nick, I cc'ed you on this because it likely directly affects funnel. > Probably in a trivial way, but nonetheless.) > > Looking at bug#46048 <http://bugs.mysql.com/46048> I realized that our > option handling code is a bit suboptimal. > While in general it works just fine, and fixing the bug can be done with > minimal effort, it's a bit cluttered. > BTW, this bug is simply that we can't figure out if an INT option has been > set on the command line or via an assignment (most likely assigning the > default value) in code when parsing the defaults file. > For strings and string arrays it works, doubles (which we don't use > currently) would be even worse to handle, since you can't rely on a double > fitting into a pointer. > > To expand on the current implementation a bit: > Currently we base everything on GOptionEntry, which does not know about > default values. > What's more, large chunks of main_cmdline deals with options, making the > code a bit harder to follow than necessary, especially figuring out at what > points config variables can change their values. > The plugins are a bit cleaner in this regard, since they will generally > initialize their variables in their init function, set the GOptionEntries in > get_options and do something based on the actual values in apply_config. > > Nevertheless, it would be nice to abstract out the entire options vs > keyfiles vs default values process, but this will likely mean breaking > binary compatibility with older plugins (which means you wouldn't be able to > load them into a newer chassis). That's fine, really, since most of the time > no one does that anyway (at least I don't know of anyone who does). > > Here's my proposal in generic terms (haven't fully spec'ed it out): > Wrap all chassis config variables in a struct, makes it easy to see which > ones are influenced by the outside world. > Rather than using GOptionEntry directly, wrap it in a struct as well, > alongside a union holding the different possible default values. > Construction of an argument "object" would mean a function call with pretty > much what GOptionEntry contains, either followed by a function call to set > the default if there is any, or via some pointer trickery in the > construction function directly (i.e. pass in void* and deref/cast to the > correct type based on the G_OPTION_ARG enum). > > To get the actual array of GOptionEntry structs, you'd call a function once > you registered all the options. Optionally you could request a GOptionGroup > directly, haven't made up my mind on that one yet. > Using option groups directly have the added benefit of allowing to pass in a > user_data, which would contain the default values. > Ideally I'd like to hide all the complexity that allowing the defaults file > brings with a post parse hook, also provided by the option groups. This > would be a function in the chassis that deals with figuring out the defaults > file implications. > > The neat thing would be that almost all of the complexity of dealing with > options would be hidden, making the available options, their storage > location and their default values painfully obvious in the code. And it > hopefully cuts down on the size of main_cmdline - I always nearly get lost > in there, it's too long (which also address part of one of the review > comments we received). > > Thoughts, opinions and critique to me please :) > > cheers, > -k > -- > Kay Roepke > Software Engineer, MySQL Enterprise Tools > > Sun Microsystems GmbH Sonnenallee 1, DE-85551 Kirchheim-Heimstetten > Geschaeftsfuehrer: Thomas Schroeder, Wolfang Engels, Wolf Frenkel > Vorsitz d. Aufs.rat.: Martin Haering HRB MUC 161028 > > -- Nick Loeve _______________________________________________ Mailing list: https://launchpad.net/~mysql-proxy-discuss Post to : [email protected] Unsubscribe : https://launchpad.net/~mysql-proxy-discuss More help : https://help.launchpad.net/ListHelp
