[Freeciv-Dev] Re: [RFC PATCH] init_techs

[Reinier Post <rp@xxxxxxxxxx>]

On Sat, Sep 22, 2001 at 04:48:01PM -0400, Justin Moore wrote:
> 
> > >    I posted a perl-like split a little while back, but got very little
> > > response other than, "Boy, that's kind of neat."  Is there interest in
> > > someone (or me) rewriting some of the parsing code?  I don't think I have
> > > anything better to do this weekend. :)
> >
> > I would really love to see this.  Some small patches I've been meaning to
> > do hang on the lack of proper parsing.
> 
>    My eyes hurt.
> 
>    I'm in the process of resurrecting my split patch, but I'm learning
> how, uhm, "robust" the command line-parsing is.  All the following
> commands are equivalent on the server:
> 
> set scorelog 1
> /set scorelog 1
>       /set scorelog 1
> !@#$%^&*()/set scorelog 1
> /!@#$%^&*()set scorelog 1
> 
>    I can see 1, 2, and maybe 3 as being valid.  But is there really any
> reason to make the code as complex as it is just so it can handle cases 4
> and 5?

No, and I don't think that's what happened.  It just tries to skip
to the next word, then to the end of the word, etc., with small loops,
that have been copied wherever they were needed.  It's not complex, it's
just lack of organization.  Checking on validity has only been added
as far as it was necessary to disambiguate commands.

> Yes, I know, be conservative in what you send and liberal in what
> you accept, but I think this may be a bit overboard.

Definitely.  A recent example: it turned out that the 'set option=value'
syntax worked, and was in use, after it was broken by accident.

> The command-line
> parsing could/should probably be implemented this way:
> 
>   /* ... */
>   strip_leading_whitespace();
>   handle_server_char(); /* the '/', can/should keep the if statement */
>   split(pattern, input);  /* Get command in one buf, all args in other */
>   lookup_command(command); /* Get a callback function */
>   command_func(args);
>   /* ... */

I'm not sure the split operation is really needed, as long as the code
to read arguments and the space between them is shared.  In the future
commands may have to extend to the next line.

>    If the command they entered was invalid, tell them so.

This already happens, but not in a central way.  It's better to
tokenise the line based on a command line grammar specification, a
scanf-like in the command struct for example, eg. "%p" for
'a player name goes here'.  That way you can have a single
place in the code that issues messages like: "error: %dth argument to
%s command should be a %s".

Or use lex.

> The command
> struct could easily be modified to facilitate a change by associating a
> function pointer with it.  Thus:
> 
> typedef void (*command_func)(char *args);
> 
> struct command {
>   char *name;              /* name - will be matched by unique prefix   */
>   enum cmdlevel_id level;  /* access level required to use the command  */
>   char *synopsis;          /* one or few-line summary of usage */
>   char *short_help;        /* one line (about 70 chars) description */
>   char *extra_help;        /* extra help information; will be line-wrapped */
>   command_func *fptr;      /* Pointer to a callback function */
> };

Yes. This has already been done by dwp for options; it has a
callback hook to add command-specific checks / side effects.

[...]
>    Within each *_cmd function it could do all the parsing of per-function
> arguments.  The 'set' function could be much, much simpler, too, and allow
> for per-option callbacks, too (just an idea).  The init_techs patch (among
> others) would become relatively trivial.  The syntax could be
> 
> set attribute nation.Greek.techs Railroads,Explosives,etc,etc

Well, that is another step: arguments that address parts of the C data
structures.  I don't think you want to specify a syntax for them and
specify the parse code on a case by case basis.

It is possible to specify a generic language for addressing every single
bit of an arbitrary Freeciv data structure, and derive a parser for it
automatically using a C utility linked against the Freeciv code.  That
way if someone adds a new player field 'mood' all it takes to add
it to the command language syntax is a recompile of that utility.

In this general syntax you can even say things like

  set game.player4.city25.name Dresden

It can also have abbreviations:

  set cities."Dr sden".name Dresden

(which works iff there is exactly one city names "Dr sden").

>    And in the attribute sub-func call:
> 
> #define MAX_PARSE_DETAIL 6 /* Can only be x.x.x.x.x.x descriptions */

This would not be necessary.

> #define MAX_INIT_TECHS  16 /* Don't want anyone to be too strong */
> 
> char *opts[MAX_PARSE_DETAIL];
> int nfound;
> char *techs[MAX_INIT_TECHS];
> 
> nfound = split(".", args[2], opts, MAX_PARSE_DETAIL);
> /* Realize we're doing a nation */
> /* Get Greek player data */
> /* Realize it's tech-related */
> nfound = split(",", args[3], techs, MAX_INIT_TECHS);
> /* Add all techs */
> 
>    Or something like that. :)

This code would all be generated automatically.
> 
>    Now my brain hurts.

Mine is wobbling, I think.

> -jdm

-- 
Reinier