Greg (H),
Just to clarify my intended course of action, the main goals of the
SVN client option/arg processing loop are:
1. Allow options and arguments to be interleaved.
2. But everything after "--" is an arg, of course.
Currently only goal 1 is met. I'm fine with any change to
apr_getopt_long() and friends that allows us to satisfy both 1 and 2.
I had also mildly had a goal #3 in mind:
3. Have just one caller loop, that takes care of both opt_state and
loading up the regular args table.
...but this is not crucial. If argv gets permuted such that all the
pure arguments are on the right, that's great too, I'll just load them
into the args table in a separate loop.
I don't know if you're a committer in APR yet, or are accessing it
anonymously. You might want to ask for commit access if you don't
have it already. Can you take care of getting your changes in there
(I don't know how much discussion with the APR dev list may be
necessary first, I imagine not much, as the changes look only
beneficial), and then I'll update subversion/client/main.c as
necessary?
-K
Greg Hudson <ghudson@mit.edu> writes:
> For review (again, can't make a patch until I resolve my apr checkout
> issues), here is an apr_getopt_long implementation which supports
> argument interleaving like the gnu getopt_long does. Unlike the gnu
> getopt_long, you have to set os->interleave = 1 to turn on
> interleaving; the gnu version does interleaving by default unless
> POSIXLY_CORRECT is set in the environment.
>
> There are a couple of other changes not shown here; "interleave",
> "skip_start", and "skip_end" fields need to be added to the
> apr_getopt_t structure, and "argv" has to become a "char **" instead
> of a "char *const *". When I can submit a real patch, all that will
> show up, of course.
>
> /* Reverse the elements argv[start..start+len-1]. */
> static void reverse(char **argv, int start, int len)
> {
> char *temp;
>
> for (; len >= 2; start++, len -= 2) {
> temp = argv[start];
> argv[start] = argv[start + len - 1];
> argv[start + len - 1] = temp;
> }
> }
>
> /*
> * Permute os->argv with the goal that non-option arguments will all
> * appear at the end. os->skip_start is where we started skipping
> * non-option arguments, os->skip_end is where we stopped, and os->ind
> * is where we are now.
> */
> static void permute(apr_getopt_t *os)
> {
> int len1 = os->skip_end - os->skip_start;
> int len2 = os->ind - os->skip_end;
>
> if (os->interleave) {
> /*
> * Exchange the sequences argv[os->skip_start..os->skip_end-1] and
> * argv[os->skip_end..os->ind-1]. The easiest way to do this is
> * to reverse the entire range and then reverse the two
> * sub-ranges.
> */
> reverse(os->argv, os->skip_start, len1 + len2);
> reverse(os->argv, os->skip_start, len2);
> reverse(os->argv, os->skip_start + len2, len1);
> }
>
> /* Reset skip range to the new location of the non-option sequence. */
> os->skip_start += len2;
> os->skip_end += len2;
> }
>
> APR_DECLARE(apr_status_t) apr_getopt_long(apr_getopt_t *os,
> const apr_option_t *opts,
> int *optch, const char **optarg)
> {
> const char *p;
> int i, len;
>
> /* Let the calling program reset option processing. */
> if (os->reset) {
> os->place = EMSG;
> os->ind = 1;
> os->reset = 0;
> }
>
> /*
> * We can be in one of two states: in the middle of processing a
> * run of short options, or about to process a new argument.
> * Since the second case can lead to the first one, handle that
> * one first. */
> p = os->place;
> if (*p == '\0') {
> /* If we are interleaving, skip non-option arguments. */
> if (os->interleave) {
> while (os->ind < os->argc && *os->argv[os->ind] != '-')
> os->ind++;
> os->skip_end = os->ind;
> }
> if (os->ind >= os->argc || *os->argv[os->ind] != '-') {
> os->ind = os->skip_start;
> return APR_EOF;
> }
>
> p = os->argv[os->ind++] + 1;
> if (*p == '-' && p[1] != '\0') { /* Long option */
> /* Search for the long option name in the caller's table. */
> p++;
> for (i = 0; opts[i].optch != 0; i++) {
> len = strlen(opts[i].name);
> if (strncmp(p, opts[i].name, len) == 0
> && (p[len] == '\0' || p[len] == '='))
> break;
> }
> if (opts[i].optch == 0) /* No match */
> return serr(os, "invalid option", p - 2, APR_BADCH);
> *optch = opts[i].optch;
>
> if (opts[i].has_arg) {
> if (p[len] == '=') /* Argument inline */
> *optarg = p + len + 1;
> else if (os->ind >= os->argc) /* Argument missing */
> return serr(os, "missing argument", p - 2, APR_BADARG);
> else /* Argument in next arg */
> *optarg = os->argv[os->ind++];
> } else {
> *optarg = NULL;
> if (p[len] == '=')
> return serr(os, "erroneous argument", p - 2, APR_BADARG);
> }
> permute(os);
> return APR_SUCCESS;
> } else if (*p == '-') { /* Bare "--"; we're done */
> permute(os);
> os->ind = os->skip_start;
> return APR_EOF;
> }
> else if (*p == '\0') /* Bare "-" is illegal */
> return serr(os, "invalid option", p, APR_BADCH);
> }
>
> /*
> * Now we're in a run of short options, and *p is the next one.
> * Look for it in the caller's table.
> */
> for (i = 0; opts[i].optch != 0; i++) {
> if (*p == opts[i].optch)
> break;
> }
> if (opts[i].optch == 0) /* No match */
> return cerr(os, "invalid option character", *p, APR_BADCH);
> *optch = *p++;
>
> if (opts[i].has_arg) {
> if (*p != '\0') /* Argument inline */
> *optarg = p;
> else if (os->ind >= os->argc) /* Argument missing */
> return cerr(os, "option requires an argument", *optch, APR_BADARG);
> else /* Argument in next arg */
> *optarg = os->argv[os->ind++];
> os->place = EMSG;
> } else {
> *optarg = NULL;
> os->place = p;
> }
>
> permute(os);
> return APR_SUCCESS;
> }
Received on Sat Oct 21 14:36:15 2006