We had a bug report about mpc options given after commands - I didn't
realize they could come after commands, and broke this when I added short
options. It's fixed now! I tested my patch with all combinations of [no
command, command with arguments, command without arguments] and [no options,
options before, options after, options before and after]. I'm pretty sure I
didn't miss anything this time. It's not the totally ideal way to do this -
it still rearranges argv, just in the correct way for mpc [options] command
[options] arguments - but it's consistent with what we have so far. More
importantly, it fixes the regression.
I notice that the usage statement (printed or in man page) doesn't say
[options] like most commands do - with 20/20 hindsight, maybe specifying
there could've saved some trouble. I went ahead and made a small patch for
this too.
Finally, I while I was messing with usage statements, I modified the help
printing behavior - if the command is invalid, just print an error, usage
statement, and reminder about 'mpc help' and man page. I suspect the vast
majority of invalid commands are typos, and none of us want our screens
filled whenever we mistype.
Unfortunately, I'm behind a firewall and can't push right now - I'm
attaching patches, and as soon as I have some free (as in freedom) internet,
I'll push to my repo (git://git.musicpd.org/jefromi/mpc.git).
Jeffrey
From cb5dc0797654b4b370fed85167876195a2c8f403 Mon Sep 17 00:00:00 2001
From: Jeffrey Middleton jefr...@gmail.com
Date: Mon, 14 Sep 2009 09:41:34 -0500
Subject: [PATCH 1/5] options.c: allow for options before and after the command
The original patch (b3f6766) adding short option support and some new
options broke support for options after the command. This patch fixes
that. At the same time, it also fixes treatment of argument '-' (invalid
option) and '--' (treat everything following as non-option).
---
src/options.c | 47 +++
1 files changed, 39 insertions(+), 8 deletions(-)
diff --git a/src/options.c b/src/options.c
index 57fd330..34f4f67 100644
--- a/src/options.c
+++ b/src/options.c
@@ -136,20 +136,31 @@ handle_option(int c, const char *arg)
void
parse_options(int * argc_p, char ** argv)
{
- int i, optind;
+ int i;
const arg_opt_t *opt = NULL;
char * tmp;
+ int cmdind = 0;
+ int optind = 0;
for (i = 1; i *argc_p; i++) {
const char *arg = argv[i];
size_t len = strlen(arg);
- if (len = 2 arg[0] == '-') {
+ if (arg[0] == '-') {
if (arg[1] == '-') {
/* arg is a long option */
char *value;
size_t name_len = len - 2;
+/* if arg is --, there can be no more options */
+if (len == 2) {
+ optind = i + 1;
+ if (cmdind == 0) {
+ cmdind = optind;
+ }
+ break;
+}
+
/* make sure we got an argument for the previous option */
if( opt opt-argument)
option_error(ERROR_MISSING_ARGUMENT, opt-longopt, opt-argument);
@@ -177,6 +188,8 @@ parse_options(int * argc_p, char ** argv)
} else {
/* arg is a short option (or several) */
size_t j;
+if (len == 1)
+ option_error(ERROR_UNKNOWN_OPTION, arg, NULL);
for (j=1; jlen; j++) {
/* make sure we got an argument for the previous option */
@@ -196,15 +209,25 @@ parse_options(int * argc_p, char ** argv)
}
}
} else {
- /* No '-'; arg is an option argument or command. */
+ /* No '-' */
if (opt opt-argument) {
+/* arg is an option argument */
handle_option(opt-shortopt, arg);
opt = NULL;
} else {
-break;
+/* arg may the command; note it and read for more options */
+if (cmdind == 0)
+ cmdind = i;
+/* otherwise, it is the first command argument and we are done */
+else {
+ optind = i;
+ break;
+}
}
}
}
+ if (optind == 0)
+ optind = i;
if (opt opt-argument)
option_error(ERROR_MISSING_ARGUMENT, opt-longopt, opt-argument);
@@ -232,12 +255,20 @@ parse_options(int * argc_p, char ** argv)
/* Fix argv for command processing, which wants
argv[1] to be the command, and so on. */
- optind = i;
+ if (cmdind != 0)
+ argv[1] = argv[cmdind];
if (optind 1) {
- for (i = optind; i *argc_p; i++)
- argv[i-optind+1] = argv[i];
+ if ( optind == cmdind || cmdind == 0 ) {
+ for (i = optind + 1; i *argc_p; i++)
+argv[i-optind+2] = argv[i];
+
+ *argc_p -= optind - 1;
+ } else {
+ for (i = optind; i *argc_p; i++)
+argv[i-optind+2] = argv[i];
- *argc_p -= optind - 1;
+ *argc_p -= optind - 2;
+ }
}
}
--
1.6.5.rc1.10.g20f34
From 5bcbb0ee2caef0706f275e6f56bc0d097202505b Mon Sep 17 00:00:00 2001
From: Jeffrey Middleton jefr...@gmail.com
Date: Mon, 14 Sep 2009 10:03:59 -0500
Subject: [PATCH 2/5] doc: indicate options in usage statements
Done both in man page and printed usage statement; this synchronizes the
two.
---
doc/mpc.1 |9 ++---
src/main.c |2 +-
2 files changed, 7 insertions(+), 4 deletions(-)
diff
