Alternative ways to do it:

If you are thinking of developing a complex system, then at some point you might need to structure your language more, rather than just adding lots of commands with different patterns (sooner or later, you're likely to accidentally have 2 patterns that both match, especially if you load patterns from modules). How you do this is up to you. It might not be a bad idea to use the standard Unix form command [-option [argument...] ...] [command-argument...] and maybe even use one of the Getopt modules to parse the command line?

(Thinking some more) You have two choices:

syntax: centralised/delegated
semantics: centralised/delegated

Where centralised means "handled in the parser" and "delegated" means "handled in the module". I suggest centralised syntax, as much as possible, and delegated semantics. This combines predictability with flexibility. For example, given the input

foo -b baz boot.txt bop.txt

your central parser decides if it is a valid command, then passes it to the module (which has previously registered a "foo" command with the parser). The module decides what to do. All modules inherit from a Module class which provides an overridable method for deciding what to do. (E.g. "call the subroutine foo with the arguments parsed into a perl data structure as follows".)

What I am suggesting has some advantages. Your program could maintain state: e.g. "cd /foo" followed by "ls" prints something different from "ls" on its own. You would implement this by having the module which provides "cd" and "ls" use method calls on an object rather than just subroutine calls.

However, it all depends on how regular the commands are - which depends on what exactly you are trying to do. If everything is very regular, perhaps you can have centralised semantics too and just dispatch to particular subroutines, as now. OTOH, if you want a complex language, you might want to think of a parsing module like Parse::RecDescent

dave hj~

Thanks for the thoughts.
I guess it's not always easy to give tips without knowing what the thing is being used for. I purposely just wanted tips on the code and not its surroundings, but I'll tell you a little bit more anyway.
It's not about external command-line arguments, but commands typed into the 'shell', so I guess Getopt wouldnt help much (unless I can use that internally as well?) and isnt really the sort of syntax I need.
You'll have to enlighten me on 'semantics' I'm afraid, I think I know what you mean with 'syntax' .. ;)


It's actually a Telnet Client for playing muds with.. With commands similar to zMUD, ie '#VAR myvar {2}' '#TRIGGER {sometext} {#colour red}' to mention a couple, this is the stuff and commands that are getting parsed. I'm already using Parse::RecDescent to parse complex scripts, but am trying to avoid using it for simple commands, which seems to be working so far.


C.

I don't know if you can use Getopts modules on a string, but I bet there's an option somewhere. Take a look.

Semantics means "meaning", more or less. The semantics of a statement are going to be what it actually does. The syntax is just how the statement is structured. So you have to think about how the commands you are using are related. Perhaps some commands have a context (a "state"). Some commands might be "like" others (so you can use inheritance to specify their semantics: derive behaviour from a base class, modify it in the child class). Et cetera.

At the moment, you are basically assuming "all commands are semantically different" - they each get their own subroutine or module method call. You're also assuming "all commands are syntactically the same", but only in that they can be parsed by regexes. You could increase the level of syntactic sameness, and this might save you time typing out different regexes and format messages.

Here's a rough example:

package main;

# ...
my $command;
eval {
    $command = CommandFactory->create ($input_string); # returns the r
+ight subclass of Command
}; #trap non-existent commands
if ($@) {printandprompt("command not recognized");}

if ($command->parse_syntax) { # check for syntax errors
    eval {
        $command->execute;
    }; # trap semantic errors, e.g. "user doesn't exist"
    if ($@) {
        print $@; # maybe die if need be
    }
} else {
    printandprompt $command->syntax_string;
}

package CommandFactory;

sub create {
    my $class = shift;
    my $str = shift;
    
    my ($cmd, @args) = split $str;
    my $cmd_class = "Command::" . ucfirst ($cmd);
    return $cmd_class->new(@args);
}

package Command;

use vars qw/$SYNTAX/;
# ...
sub new {
    my $class = shift;
    my @args = @_;
    
    my $self = {args => \@args};
    bless $self, $class;
}

sub _syntax {
    my $self = shift;

    no strict 'refs';
    my $class = ref ($self) || $self;
    return ${"$class::SYNTAX"};
}

sub parse_syntax {
    my $self = shift;

    # check $self->{args} against $self->_syntax
    # then parse the arguments into $self fields
    # return false on error
    # alternatively "die" and catch the exception
}

package Command::Msg;

use Command;
use base 'Command';
use vars qw/$SYNTAX/;

$SYNTAX = {
    options => {
        player => {
            required     => 1,
            arg        => 'string',
        }
        loudness => {
            required     => 0,
            arg        => 'int'
        }
    }
    arguments => {
        min => 1,
    }
};

sub execute {
    my $self = shift;
    
    # send $self->{msg} to $self->{player} with $self->{loudness}
}
[download]

dave hj~

so I guess Getopt wouldnt help much (unless I can use that internally as well?)

Untested code ahead (must take daughter to school):

use Text::ParseWords;
use GetOpt::Std;

@ARGV = shellwords($textin);
my $cmd = shift @ARGV;
my %opts;
getopts('oif:', \%opts);
[download]

I wouldn't use arrays but hashes, where the key names make the code self documenting. Than you could do something like:

$command{'modules loadall'}->{'function'}={ ( "Module" => "MAIN",
                                              "Function" => "loadmodul
+es"
                                            )};
[download]

CU
Robartes-

Since I'm not calling the method from the module directly, but from an object of created from the module, I still need to find out if it's a local function or not.
Thanks for the idea though.


C.

As for critique on your current code, here are some incoherent thoughts. Most of all, I don't like your use of symbolic references. I can assume this isn't your actual code, that your syntax is actually described in an external datafile. Still, there's no need to keep the data in this way. I would use anonymous subroutines instead of the functions and method calls. Like:

$Commands{'log'}{'function'} = sub { createlog(shift) };
$Commands{'modules saveall'}{'function'} = sub { savemodules() };
$Commands{'modules loadall'}{'function'} = sub { loadmodules(shift) };
# extra example for module call:
$Commands{'foo'}{'function'} = sub { Bar->foo(@_[0, 1]) };
[download]

$code = 'print "Hello, $name!\n";';
$sub = eval "sub { my \$name = shift; $code }";
print "Ready to test:\n";
$sub->('world');
[download]

You do have to be careful, if this code comes from an external source, to check that it doesn't contain malicipous code.

Also, use qr// for your patterns. Again, you get a compile time checks of all the patterns, plus it'll be faster at runtime.

I'm quite unhappy on how you actually do the parsing, but I'm not sure how I'd tackle that. I think I'd use (precompiled) regexes, not substr(). What if somebody typed "logic", would you try to do the "log" action? What if somebody typed "modules  saveall", would it fail to match? Still, if you don't want to use regexes, you definitely need to extract the words from the command line first, and then check if these words match.

And I think I would group the "modules saveall" and "modules loadall" under one common processing umbrella, "modules". For that, you'd have to have two subpatterns, one for each option.

As a summary... your data structure seems to have a bit of redundany, doesn't it? All you really need is the syntax/pattern on one hand, and the function on the other. The number of parameters is determined by the pattern.

Perhaps use Parse::RecDescent would still be the simpler solution... :)

Hmm, you're assuming wrong. That *is* my actual code :)


I chose this way of doing it (as far as I remember) because it was easier to read a list of method names/patterns from an external (plugin) module and add them to my Hash at the time. I'm not quite sure how that would work using anonymous subroutines, and if its not more dangerous creating them with eval..
(Maybe I should post how I do that at the moment..)


If someone types 'logic' then it does nothing at all.. thats why I compare the length of the command to $data, *and* then compare the pattern. As the pattern has 'log<space>' in it, it won't match.
Using compiled regexs sounds like an idea, if they're faster, I've not used qr// yet, so I'll have to look that up.
Also grouping the types of commands was something I'd thought of, but didn't get around to trying out yet :)


(P::RD is a tad too slow for mud triggers.. :)
C.

Well, your typing way too much. Lots of redundant punctuation, cut+paste blocks of nearly identical code. Your sample code can be reduced to this without trading clarity for brevity. Though maybe you are a quick typist and see verbocity eq clarity.

sub init_commands
{
    # Fill %Commands hash for parsecommands 
    my @keys = qw/pattern function syntax/;
    
    # %Commands{commandname}{@keys} = 
        ( 'pattern', 
        ['modulename', [['name', number of arguments]],
         'syntax' 
    );
    @Commands{log}{@keys}= 
          ('^log (.*)$', 
          ['', ['createlog', 1]], 
          'log <filename> <new/append>'
    );
    @Commands{modules saveall}{@keys} = 
        ( '^modules saveall$', 
        ['', ['savemodules', 0]], 
        'modules saveall' 
    );
    @Commands{modules loadall}{@keys} = 
        ( '^modules loadall ([\w\.]+)$', 
        ['', ['loadmodules', 1]], 
        'modules loadall <filename>' 
    );
    ... and so on..
}

while( my ($command, $params) = each %Commands) {
    #debug("parselocalcommand: Command: $command\n");
    
    if( $data =~ /^$command/ ) {
    
        if(my @res = $data =~ $params->{pattern} ) {
        
            if($#+ == $params->{function}[1][1]) {
            
                my $answer = $params->{function}[0]
                            ? modulemethod(
                                $params->{'function'}[0],
                                $params->{'function'}[1][0],
                                @res )
                            : $params->{function}[1][0]->(@res);

                printandprompt($answer) if $answer ;
            }
        }
        else {
            printandprompt("Syntax: #" . $params->{syntax} . "\n");
        }
    }
}
[download]

package my::Class;
use Dispatch
(
    doStuff => \&yadda,
    doMore  => sub{ ... }
);

# ...your methods here
[download]

my %commands = (
    name => {
        args => 2,
        usage => 'name arg1 arg2',
        sub => 'sub_1', # might wanna allow for code refs here :)
    },
);
my @cmd = split /\s+/, $command;
if ($commands{$cmd[0]}) {
    my $sub = $commands{$cmd[0]}{sub};
    if ($#cmd == $commands{$cmd[0]}{args}) {
        $self->$sub(@cmd);
    }
    else { print "usage: ..." }
}
else { print "unknown command $cmd[0]"}
[download]

DON'T USE THE STANDALONE MODULE!

If it has been worked into a new perl then maybe fine but I suffered deeply from using it with perl 5.005 last year. It will break your code just when your deadline is up. You have been warned. We will return you to your regularly scheduled coffee break now..