You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@perl.apache.org by "Philippe M. Chiasson" <go...@ectoplasm.org> on 2004/09/13 23:34:54 UTC
[mp2] Suggest renaming Apache::CmdParms $parms->info to $parms->cmd_data
From this example :
our @APACHE_MODULE_COMMANDS = (
{
name => 'MyDirective1',
func => \&MyDirective,
cmd_data => 'One',
},
{
name => 'MyDirective2',
func => \&MyDirective,
cmd_data => 'Two',
},
);
sub MyDirective {
my($self, $parms, $args) = @_;
my $info = $parms->info;
^^^^
Wouldn't $parms->cmd_data make more sense ?
--
--------------------------------------------------------------------------------
Philippe M. Chiasson m/gozer\@(apache|cpan|ectoplasm)\.org/ GPG KeyID : 88C3A5A5
http://gozer.ectoplasm.org/ F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3A5A5
Re: [mp2] Suggest renaming Apache::CmdParms $parms->info to $parms->cmd_data
Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
>>>while our code interface into it is parms->info->cmd_data that's not
>>>the way
>>>a C programmer would use it. over in C land the directive handler
>>>interface
>>>looks exactly like the current perl interface.
>
>
>>What you are missing is that a user is passing that data using the
>>'cmd_data' hash key, so trying to retrieve it with info() is not the
>>best choice of the interface, IMHO.
>
>
> I agree that it's not the best choice. but this is also exactly the same as
> the C interface:
>
> struct command_struct {
> /** Name of this command */
> const char *name;
> /** The function to be called when this directive is parsed */
> cmd_func func;
> /** Extra data, for functions which implement multiple commands... */
> void *cmd_data;
>
> so the arguments required when setting up a directive have the same name as
> the structure elements, and the interface when retrieving them is the same
> as the C interface.
I don't like when the (sometimes badly designed) C API makes it harder on
the Perl users. 99.9% of mod_perl users will never use the C API. We have
agreed to keep the Perl API as close to the C API, while making the Perl
API intuitive and perlish. This case doesn't seem to follow that idea.
> and all of this is exactly like it was in mp1 where
> there is lots of documentation explaining it.
I think mp1 API is pretty much irrelevant here, since none of the existing
mp1 directive handlers stuff will work as is under mp2.
>>I think the key and the retrieving
>>function need to match. If you want it to be 'info', then it should be
>>'info' in both places.
>
>
> for the record, we've had this discussion before and it's no surprise that
> we came out on the exact same sides as we are now :)
>
> http://marc.theaimsgroup.com/?t=105409005700004&r=1&w=2
Consistency is good :)
> I still like the idea that the accessors match the structure slot names. I
> also think changing it at this point makes less sense - not that we can't
> change things now, but that directive handlers are confusing enough without
> going against both how Apache itself represents things and all of the
> existing documentation for mp1 wrt the accessor names.
Well, I have no strong feelings about that change. I just think we can do
better on the user interface. But whatever you feel is right, go with it.
We have a way too big API to have the luxury of spending a lot of time
discussing each method.
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2] Suggest renaming Apache::CmdParms $parms->info to $parms->cmd_data
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
>> while our code interface into it is parms->info->cmd_data that's not
>> the way
>> a C programmer would use it. over in C land the directive handler
>> interface
>> looks exactly like the current perl interface.
> What you are missing is that a user is passing that data using the
> 'cmd_data' hash key, so trying to retrieve it with info() is not the
> best choice of the interface, IMHO.
I agree that it's not the best choice. but this is also exactly the same as
the C interface:
struct command_struct {
/** Name of this command */
const char *name;
/** The function to be called when this directive is parsed */
cmd_func func;
/** Extra data, for functions which implement multiple commands... */
void *cmd_data;
so the arguments required when setting up a directive have the same name as
the structure elements, and the interface when retrieving them is the same
as the C interface. and all of this is exactly like it was in mp1 where
there is lots of documentation explaining it.
> I think the key and the retrieving
> function need to match. If you want it to be 'info', then it should be
> 'info' in both places.
for the record, we've had this discussion before and it's no surprise that
we came out on the exact same sides as we are now :)
http://marc.theaimsgroup.com/?t=105409005700004&r=1&w=2
I still like the idea that the accessors match the structure slot names. I
also think changing it at this point makes less sense - not that we can't
change things now, but that directive handlers are confusing enough without
going against both how Apache itself represents things and all of the
existing documentation for mp1 wrt the accessor names.
--Geoff
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2] Suggest renaming Apache::CmdParms $parms->info to $parms->cmd_data
Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
>>>> sub MyDirective {
>>>> my($self, $parms, $args) = @_;
>>>> my $info = $parms->info;
>>>> ^^^^
>>>>
>>>>Wouldn't $parms->cmd_data make more sense ?
>
>
>>>+1
>>>
>>>Especially since parms->info on the C level is something else. It's
>>>really parms->info->cmd_data.
>
>
> while our code interface into it is parms->info->cmd_data that's not the way
> a C programmer would use it. over in C land the directive handler interface
> looks exactly like the current perl interface. for instance, from core.c:
>
> static const char *set_server_string_slot(cmd_parms *cmd, void *dummy,
> const char *arg)
> {
> /* This one's pretty generic... */
>
>
> int offset = (int)(long)cmd->info;
>
> so, unless I'm missing something, I don't see the reason to change this
> interface at all.
What you are missing is that a user is passing that data using the
'cmd_data' hash key, so trying to retrieve it with info() is not the best
choice of the interface, IMHO. I think the key and the retrieving function
need to match. If you want it to be 'info', then it should be 'info' in
both places.
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2] Suggest renaming Apache::CmdParms $parms->info to $parms->cmd_data
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
>>> sub MyDirective {
>>> my($self, $parms, $args) = @_;
>>> my $info = $parms->info;
>>> ^^^^
>>>
>>> Wouldn't $parms->cmd_data make more sense ?
>> +1
>>
>> Especially since parms->info on the C level is something else. It's
>> really parms->info->cmd_data.
while our code interface into it is parms->info->cmd_data that's not the way
a C programmer would use it. over in C land the directive handler interface
looks exactly like the current perl interface. for instance, from core.c:
static const char *set_server_string_slot(cmd_parms *cmd, void *dummy,
const char *arg)
{
/* This one's pretty generic... */
int offset = (int)(long)cmd->info;
so, unless I'm missing something, I don't see the reason to change this
interface at all.
--Geoff
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp2] Suggest renaming Apache::CmdParms $parms->info to $parms->cmd_data
Posted by "Philippe M. Chiasson" <go...@ectoplasm.org>.
Stas Bekman wrote:
> Philippe M. Chiasson wrote:
>
>> From this example :
>>
>> our @APACHE_MODULE_COMMANDS = (
>> {
>> name => 'MyDirective1',
>> func => \&MyDirective,
>> cmd_data => 'One',
>> },
>> {
>> name => 'MyDirective2',
>> func => \&MyDirective,
>> cmd_data => 'Two',
>> },
>> );
>>
>> sub MyDirective {
>> my($self, $parms, $args) = @_;
>> my $info = $parms->info;
>> ^^^^
>>
>>Wouldn't $parms->cmd_data make more sense ?
>
>
> +1
>
> Especially since parms->info on the C level is something else. It's really
> parms->info->cmd_data.
Yeah, with a bit more magic on top of it, since parms->info->cmd_data is _really_
a void *.
>
--
--------------------------------------------------------------------------------
Philippe M. Chiasson m/gozer\@(apache|cpan|ectoplasm)\.org/ GPG KeyID : 88C3A5A5
http://gozer.ectoplasm.org/ F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3A5A5
Re: [mp2] Suggest renaming Apache::CmdParms $parms->info to $parms->cmd_data
Posted by Stas Bekman <st...@stason.org>.
Philippe M. Chiasson wrote:
> From this example :
>
> our @APACHE_MODULE_COMMANDS = (
> {
> name => 'MyDirective1',
> func => \&MyDirective,
> cmd_data => 'One',
> },
> {
> name => 'MyDirective2',
> func => \&MyDirective,
> cmd_data => 'Two',
> },
> );
>
> sub MyDirective {
> my($self, $parms, $args) = @_;
> my $info = $parms->info;
> ^^^^
>
> Wouldn't $parms->cmd_data make more sense ?
+1
Especially since parms->info on the C level is something else. It's really
parms->info->cmd_data.
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org