Re: [Musicpd-dev-team] mpc command-line options
On 2009/07/07 16:56, Jeffrey Middleton jefr...@gmail.com wrote: options: support short options; add other options In order to support short options, the option parser has been replaced. The parser itself comes largely from ncmpc (under the GPL); it was largely written by Kalle Wallin. I don't think that code was particularly good. That's why I removed it from ncmpc in favor of GLib's option parser. mpc should not use GLib, but why not use getopt() / getopt_long()? New options have been added, as well as the short form of existing ones. The options are now: -v/--verbose, -q/--quiet, -h/--host, -p/--port, -f/--format Is -h a good shortcut for --host? Most programs map -h to --help. mpc does not have a --help option, but that's another problem.. fprintf(stderr,verbose!\n); You forgot to remove the debug code.. Max -- Enter the BlackBerry Developer Challenge This is your chance to win up to $100,000 in prizes! For a limited time, vendors submitting new applications to BlackBerry App World(TM) will have the opportunity to enter the BlackBerry Developer Challenge. See full prize details at: http://p.sf.net/sfu/blackberry ___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] mpc command-line options
I have a version that uses getopt_long but I thought I was told that was a bad idea because it comes from GNU getopt.h and we couldn't count on that being part of embedded systems? I can dig and find the email if it's relevant. Sorry about the debug code! I pulled that out of it on my end and must have forgotten to push it up to the repo. Jeffrey On Tue, Jul 7, 2009 at 11:06 AM, Max Kellermann m...@duempel.org wrote: On 2009/07/07 16:56, Jeffrey Middleton jefr...@gmail.com wrote: options: support short options; add other options In order to support short options, the option parser has been replaced. The parser itself comes largely from ncmpc (under the GPL); it was largely written by Kalle Wallin. I don't think that code was particularly good. That's why I removed it from ncmpc in favor of GLib's option parser. mpc should not use GLib, but why not use getopt() / getopt_long()? New options have been added, as well as the short form of existing ones. The options are now: -v/--verbose, -q/--quiet, -h/--host, -p/--port, -f/--format Is -h a good shortcut for --host? Most programs map -h to --help. mpc does not have a --help option, but that's another problem.. fprintf(stderr,verbose!\n); You forgot to remove the debug code.. Max -- Enter the BlackBerry Developer Challenge This is your chance to win up to $100,000 in prizes! For a limited time, vendors submitting new applications to BlackBerry App World(TM) will have the opportunity to enter the BlackBerry Developer Challenge. See full prize details at: http://p.sf.net/sfu/blackberry___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] mpc command-line options
On 2009/07/07 18:07, Jeffrey Middleton jefr...@gmail.com wrote: I have a version that uses getopt_long but I thought I was told that was a bad idea because it comes from GNU getopt.h and we couldn't count on that being part of embedded systems? I can dig and find the email if it's relevant. getopt_long() is a GNU extension, that's right. getopt() is standardized in POSIX, and it's available on mingw32, but doesn't understand long options. By the way, what's wrong with mpc's current option parser? Yours is twice as large, more complex, error handling is missing, inconsistent code style, modifies argv strings, and all it gives us is short options. Max -- Enter the BlackBerry Developer Challenge This is your chance to win up to $100,000 in prizes! For a limited time, vendors submitting new applications to BlackBerry App World(TM) will have the opportunity to enter the BlackBerry Developer Challenge. See full prize details at: http://p.sf.net/sfu/blackberry ___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] mpc command-line options
On 2009/07/07 18:45, Jeffrey Middleton jefr...@gmail.com wrote: Sorry, I emailed a bit on the mailing list a while back and thought I'd done the best thing given what we talked about. Since I was going to have to rewrite most of the code anyway to support long options, I thought reusing code would be nice, to avoid having more versions of the same wheel, and ncmpc's seemed good. A couple things about your list of points: error handling is missing -- I'm not sure what error handling you're referring to - it handles missing arguments, unknown options, and has a provision for bad arguments. Note that this is an improvement; the parser previously ignored unknown options, letting them be treated as unknown commands instead. I didn't think I'd removed any error handling in the process, but if I did I can certainly put it back in. strdup() can fail. Don't strdup(), just pass the original pointer to lookup_long_option(). modifies argv strings -- The original parser did this as well. The function remove_index in options.c is run for every extracted option. Right. The old parser sucked as well, but less than Kalle's ncmpc parser (IMHO). inconsistent code style -- I can easily take care of this. Don't let it be a deciding factor. twice as large, more complex -- The net diffstat was +279/-208. options.c is indeed much longer, but a lot of error checking and parsing/default options have been moved into it from elsewhere in the code. I can prune out some of the unnecessary complexity in the parser (e.g. callback function) and get it even closer. Larger code because of more checks is a good thing in general, your patch description didn't point that out (that's why I'm asking). So, tell me what you think is best. I can rewrite it from whatever approach you want, and if short options really aren't a feature worth adding 50 lines of code, that's fine, I'll just add them as long options to the previous setup. I didn't think I'd be the only one who'd want short options, though. Personally, I don't care much about short options, but I think it's a good idea. -- Enter the BlackBerry Developer Challenge This is your chance to win up to $100,000 in prizes! For a limited time, vendors submitting new applications to BlackBerry App World(TM) will have the opportunity to enter the BlackBerry Developer Challenge. See full prize details at: http://p.sf.net/sfu/blackberry ___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
