> Since now things are implemented in perl, may be it'd make more sense to match the perl > API with perl keys? Since now you have: > >> + cmd_data => 'some info', > > but you retrieve it with: > > ->info > > that's confusing.

well, confusing or not, it's following the Apache convention

struct command_struct {
...
    /** Extra data, for functions which implement multiple commands... */
    void *cmd_data;

and

struct cmd_parms_struct {
    /** Argument to command from cmd_table */
    void *info;

and is the same as it was in mp1. I'm inclined to keep it info(), since for (most if not) all other record accessors (which cmd_data and info both are) the names are the same as the record slot.

>
>> the patch was (for the most part) generated by making the above change in WrapXS,
>> compiling, then putting the results from the generated .c into Apache__CmdParms.h -
>> in other words, the patch was autogenerated too, so don't blame me :)
>
>
> Yes, but you need to rewrite it to reduce the clutter, which is ok when things are
> autogenerated, since no one has to maintain them. There is a typemap for doing all 
the
> conversions and croaking. Look at other thin wrappers in .h files under xs/ or the
> guide for writing those wrappers. It should be pretty trivial for this function.

I don't see much that can be cleaned up, but I'll take a look at it.

>
>> anyway, I had to shuffle the modperl_module_cmd_data_t struct around so that
>> everybody could see everything, but it all worked out in the end.
>
>
> Won't it better to put it into modperl_types.h then? All public types reside there.

that's fine, I'll move it there instead.

--Geoff


--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]



Reply via email to