In message <m0obmYa-0002wDC@hock.bolis.sf-bay.org>, Alan Millar writes:
> Ok, now the $list and $sender make sense: not as values already
> parsed from the parameters, but as pre-existing context. For what it
> is worth, the current majordomo subroutines get these values as
> global variables and not as parameters, so I don't see much harm in
> not passing them as explicit parameters to the new routines either.
Probably true.
> > Not that I am assiging @parts as well as passing @parts. The
> > pre-command can change anything it wants.
>
> I think the return value of the pre_subroutine needs to be an
> error flag that stops execution of the primary command subroutine
> if the error flag is set. Therefore we are back to passing @parts
> by pointer. I don't think that's really a problem, though.
Nope, just set it up so that the reurn is:
($error, @parts) = pre_command{$key}...;
I try to stay away from globbed forms as much as possible especially
where perl novices are likely to be writing pre/post commands.
> > Given that $pre_command{"get"} = &pre_get, why wouldn't the following
> > change requests for "get bblisa-help 1" to "get bblisa 1"??
>
> In general your example is very much what I had in mind.
>
> > if defined ($pre_command{$key}) {
> > $simple_pre_command_scalar = $pre_command{$key};
> > ($sender, @parts) = $simple_pre_command_scalar($sender, @parts);
> > }
Woops typo there:
> > ($sender, @parts) = $simple_pre_command_scalar($sender, @parts);
should read:
> > ($sender, @parts) = &$simple_pre_command_scalar($sender, @parts);
> As I mentioned, the return value should be an error code which decides
> whether to continue or not. Here's something of what I was thinking:
>
> $fail = 0;
> $pre_command_subroutine = $pre_command{$key};
> if (defined (&$pre_command_subroutine)) {
> $pre_failed = &$pre_command_subroutine(*parts);
> }
> if ($pre_failed) {
> &squawk("$key failed."); # this line may be redundant.
> } else {
> # call sub and post_sub
> }
>
> Which is mostly the same, plus the error check.
>
> > # evaluate do_get etc.
> > [...]
> >
> > <in another file far far away, but still eval'ed>
> > $pre_command{"get"} = "main'pre_get";
>
> We might not even need to explicitly specify this. I'm thinking that
> my recently-posted patch for automatic command recognition from
> subroutine names could very simply add a check for the existence of
> a pre_ and post_ subroutine in addition to the current check for do_
> subroutines. Just define a subroutine called "pre_get" and it will
> be found automatically.
Quite true the same mechanism could be used, and it would work nicely.
> This is exactly the sort of thing that I think a "pre_" subroutine
> is good for. Also continuing the "bblisa" example:
>
> sub main'pre_subscribe {
> local(*parts) = @_;
>
> # get $subscriber from @parts and/or $reply_to
> # get $clean_list from $parts[1]
> ....
>
> if ($subscriber =~ /local.domain/ &&
> $clean_list eq "outside") {
> $parts[1] = "inside";
> }
>
> if ($subscriber !~ /local.domain/ &&
> $clean_list eq "inside") {
> $parts[1] = "outside";
> }
>
> }
Yeap, I thought of exactly that on the way home from work last night
(well this AM actually). That gets me all my wish list items from
before I think.
> > As an efficiency measure, we could also pass a "valid token" on
> > the front of @parts so that the do_get function wouldn't have to
> > revalidate the form of the command, or just not validate the
> > command line in do_get if a pre_command{"get"} existed, assuming
> > that the pre-get did the validation for us.
>
> Interesting idea, but I wouldn't trust a "pre_" subroutine to do proper
> validation. One of the main reasons for having hooks for "pre_"
> routines is to allow local variations to the standard commands.
> Therefore, the standard commands are a constant while the "pre_"
> commands will have quite a bit of variability. I would expect
> the average "pre_" command to be rather "quick and dirty". So I think
> the standard command should not trust that the pre_command
> validated anything, and should go through the full checks (just
> in case a poorly-designed pre_ command changes one wrong value
> to a different wrong value).
If as you say most pre_ routines will be quick and dirty (and I think
you are probably right), then the efficiency hack should be
implemented with a "valid token" on the front of the @parts array.
There may be some people out there who define their own pre_ routines
who could benefit from the efficiency measure. Besides, if I am mapping
the previous @parts array to a totally new valid parts array, then I
should be able to avoid the validation step in the do_ routine.
Probably a versioned valid request would be needed. E.G. lets call the
current "get <list> <file>" majordomo command line version 1, then the
pre command that parses this should put "valid_v1" onto the @parts
array. Any replacement version of get (e.g. "get <list> <passwd>
<file>" would recognize the bogus valid token, and parse the @parts
array anyway. (Not quite sure how that would work with cascaded pre_
functions).
Also as I have found before, if you don't give a developer a mechanissm to
do things like bypass redundent checks, one of 2 things happens:
1) They don't bypass the checks making things run less efficiently
2) They implement it themselves, usually breaking one or more things
in the process (which I have to fix up at some later date 8-(.), or
they keep reimplementing it on an adhoc basis which is a
nightmare (for me) to maintain.
One thing I misssed is that there may be a cascade of pre_ commands. I
can see a pre command from one module that maps mailing list names
under the control of a set of config file parameters, while another
maps bare subscribe/unsubscribe requests sent to -request to standard
subscribe/unsubscribe requests. What I am aiming for is the ability to
have module developers code to a standard interface so that modules
can be dropped in and out at will, and should work with any version of
majordomo provided the follow the spec for a pre/post/do function.
Also, I guess this is a good time to deal with the problem of
documentation. One problem with Brent's help message is extending it.
If a new do_command is added, where is the commentary for it in the
help message? How do we register the comments? How do we display them?
The only problem I see with your auto-discovery ability is that
somebody can add a majordomo command without documenting it. Needless
to say I think this is a not A GOOD THING (TM). That is why I used an
function in the config files to add a new keyword, and the function
had an explicit place for commentary. (As it currently stands there
can be no commentary, but I think that is a mistake). It is fine for
pre_/post_ commands, but very bad for the do_ commands. I also chose a
functional interface so that I can change the implementation of the
config file in any way I want to as long as I keep the interface to
new_keyword() the same.
The other question is how do we get documentation on the
administrative side of things? (E.G. a mechanism for remotely
manipulating file archives?)
Warning modular C programmer solution ahead.
I would suggest a functional mechanism to add descrete commands to
majordomo, and have it deal sensibly with the resulting requests for
pre/post/do commands. The reason I suggest a functional interface is
that we are still just prototyping this code, a functional interface
hides the implementation details so that we can reimplement the
underlying code without breaking any already existing modules.
I would suggest 3 functions:
add_do(function_name (e.g. do_subscribe), majordomo_command (e.g. add),
type (admin|user), comments);
This puts a functional interface on your %alternate_commands array, as
well as explicitly noting the comments. Also the type field could be
used when generating help ccommands for users (e.g. sub/unsub, get) or
admins (newinfo, newconfig etc). Returns 0 if no majodomo command
existed, returns the previous majordomo command (and warns??) if one
existed.
add_pre(function_name (e.g. pre_add), majordomo_command, [location] );
add_post(function_name (e.g. post_add), majordomo_command, [location] );
These would add the corresponding pre/post commands. With a functional
interface, cascading is possible. The location argument (if present)
would give the cascade implentation a hint about when to schedule the
pre command. I am not actually sure how the pre/post commands would
know/care about when they are executing, or how to handle conflicts
with that.
-- John
Follow-Ups:
References:
|
|
