[asterisk-dev] RFC: internal CLI changes

[Luigi Rizzo]

On Tue, Nov 14, 2006 at 04:32:39PM -0600, Kevin P. Fleming wrote:
> Luigi Rizzo wrote:
> > In the patch attached, i am trying to address the issue by putting
> > all the implementation of a cli command in a single function, in a
> > way that is backward compatible so that we can slowly upgrade
> > commands from the old to the new format. The patch involves two
> > files (include/asterisk/cli.h and main/cli.c), and there is at the
> > end an example on how a new-format handler can be implemented.
> 
> I agree with the concept whole-heartedly, and would include extending it
> to support telling the CLI how many (maximum) arguments the command can
> take, and minimum as well, to eliminate much of the argc checking in the
> command handlers.

[executive summary: all good suggestions, and some changes are upcoming,
read below]

the min-max is a bit tricky, some commands have variable args.
But i will see what i can do.
Another thing i have been thinking about is adding regexp-like
entries to describe additional or optional arguments.
But again, this needs a bit of thinking.

Anyways, from the above, i believe it is a good thing to replace
the two subfunctions (CLI_USAGE and CLI_CMD_STRING) with just one,
CLI_INIT, where the handler fills any required fields in the
descriptor (e.g. command string, usage, maybe the min-max args that
you mention)

> However, I very much don't like this implementation... you've gone
> through a lot of machinations to keep the command handler prototype the
> way it is now, including non-obvious casting of char * to int, using

yeah the cast really sucks, but i am going to fix it soon.

> negative array indexes and other weirdness... Can you describe why you
> think this is better than just changing the prototype for command

it is just a transient thing.

With the tree still partly broken because of mismatches in the argc
checks, i wanted to give the old-style entry access to the extra
information (basically, e->args) so that i could fix those entries
without having to redo it when moving to the new-style.  Hence the
e = argv[-1] thing.

for the rest, surely the right way (and what i want do do, very
shortly) is to provde a new prototype for the handler with all the
required arguments.  I am just trying to make up my mind on how to
pass the nine (!) required arguments to the handler, and since this is
a decision that affects all handlers, i want to experiment a bit
before committing to something.  I am leaning towards a format

	char *handler(e, subfunction, args)

where args is a struct with all required arguments;
the return value is already good for the _complete() subfunction,
i only need to remap to char * the return values for the regular handler

	#define RESULT_SUCCESS          0
	#define RESULT_SHOWUSAGE        1
	#define RESULT_FAILURE          2

and maybe for the CLI_INIT subfunction (which, at this point,
only needs to return SUCCESS or FAILURE).

I am afraid the e = argv[-1] trick is probably something that we need
to keep for a while to help old-style handlers, but that is a relatively
straightforward, and hopefully sufficiently documented to avoid confusion.

	cheers
	luigi
-----------------------------------+-------------------------------------
  Luigi RIZZO, rizzo at iet.unipi.it  . Dip. di Ing. dell'Informazione
  http://www.iet.unipi.it/~luigi/  . Universita` di Pisa
  TEL/FAX: +39-050-2217.533/600    . via Diotisalvi 2, 56122 PISA (Italy)
  Mobile   +39-347-0373137
-----------------------------------+-------------------------------------