On Wed, 7 Mar 2001, B. W. Fitzpatrick wrote:
>
> > I noticed some problems with the way the command line
> > client was checking the user's input before passing
> > commands to the actual function. For example, a
> > call to `svn delete` should notice that at least
> > 1 argument is required and print the help text
> > for the delete subcommand.
>
> Well, we could always just have it print out the command specific help...
That makes sense to me. If the needs of the subcommand argument
checking changes, it would be easier to special case the
argument parsing inside the actual function than in main.c.
> > The current code passed the args to the delete subcommand anyway and
> > that printed the main usage text instead of the delete help
> > text. The following patch fixes the problem by adding a min and max
> > number of targets field to the command struct. Main then checks to
> > see if the number of arguments the user passed satisfies the (0 to
> > N) or (1 to N) requirement.
>
> Is this really worth it? Looking through the commands that we need to
> support, it seems like only a few have this requirement:
...
> I would vote to leave the checking in the command, although I don't
> feel all that strongly about it. Does anyone else have any feelings on
> this?
>
> -Fitz
Ahh, but currently the argument checking is done before the
command and in the command.
For example, take a look at propget:
usage: propget PROPNAME [TARGETS]
The following loop in main.c is meant to parse out
the PROPNAME and pass it to the command in the
svn_cl__opt_state_t argument.
if (subcommand->num_args > 0) {
const char *this_arg;
int i;
/* loop for num_args and add each arg to the args array */
for (i = 0; i < subcommand->num_args; i++) {
if (os->ind >= os->argc) {
const char *plural = "s";
fprintf (stderr, "ERROR: The %s command requires %i argument%s\n",
subcommand->name, subcommand->num_args,
(subcommand->num_args == 1) ? "" : plural);
fprintf (stderr, "Help for %s:\n%s", subcommand->name, subcommand->help);
/* svn_cl__help (NULL, targets, pool); */
/* TODO Do we have to print out the WHOLE help thing every
time for EVERY error? -Fitz */
apr_pool_destroy (pool);
return EXIT_FAILURE;
}
this_arg = os->argv[os->ind++];
(*((svn_string_t **) apr_array_push (opt_state.args)))
= svn_string_create (this_arg, pool);
}
}
This half and half approach is what bothers me.
I figured that we could just add elements to
the command table so that TARGETS parsing
could check for the number of args just
like the subcommand->num_args check.
Would it be better to simply pass the
apr_getopt_t to the command and let
it deal with separating the initial
arguments from the target arguments?
Also, what about options that might
be passed in by the user? The current
code does not seem to deal with that.
For example, with CVS you can do:
cvs diff -10 file.c
To pass the -10 argument through to
the diff command. It seems like
command line flags like this
are going to be needed, so we
should really deal with them
now instead of later. A simple
rule might be to gather up
options (they start with -)
and then start parsing things
like the PROPNAME from propget
followed by TARGET args.
Mo DeJong
Red Hat Inc
Received on Sat Oct 21 14:36:25 2006