Re: [Dancer-users] Routes with captures
hello,
- r() already supports full regex patterns (perhaps without special extras such as "/x" but that can be, I believe, patched.)
it does! i wrote any [qw| post get |] , r( qr( / ([^/]+) # the class / ([^/]+) # the object / ([^/]+) # the action /?$ )x ) , sub { "Yeah" } and it worked! and then i was confused because the r() became useless noise.
r('/.+/\w+_\w/'); is easier and more readable than: qr/\/.+\/\w+_\w\//;
It's true - however - that we could try and encourage a bit more readable regex patters: qr{/.+/\w+_\w/} Still, readability suffers.
qr(/.+/\w+_\w/); r('/.+/\w+_\w/'); Both are readable for me.
However, there are certain benefits to discuss and perhaps we'll add your patched. Since it's not something so trivial, a discussion will ensue. :)
Ok so i try to sell my point of view :) * r('') is code to maintain :-) * i really agreed your concern about readability but i don't understand why qr() is harder to read than r(''). I think qr() could be more readable in large applications: r('') does not use /x for the moment. as /x is a very good practice that is: - recommended in the "Perl Best Practice" - defaut behavior in perl6 - very addictive for those whom still use it so i think qr( / ( borrower | book | shelf ) # Bizness objects / ( delete | create ) # Crud actions / ( \d+ ) # id ) is way easier to maintain than r('/(borrower|book|shelf)/(delete|create)/(\d+)') About the feature proposed in my former message: qr( / (?<class> borrower | book | shelf ) # Bizness objects / (?<action> delete | create ) # Crud actions / (?<id> \d+ ) # id ) , sub { splat->{capture}->{action} } In the current Dancer behavior, you'll write '/:class/:action/:id' seems easier but: - you have to test all the captures in the code, which i think is a big readability issue: unless ( my ( $action = splat->{action} ) ~~ [qw/ delete create/] ) { not_found("$action isn't a valid action"); } - you'll match tons of urls too soon so it could be very hard to write smart dispatcher. for exemple, i think '/:class/:action/:id' can't be splitted in 2 rules like this get qr( / (?<class> program | library ) / (?<action> test | make ) / (?<id> \d+ ) )x, sub { "rule 1" }; get qr( / (?<class> documentation | tickets ) / (?<action> publish | alert ) / (?<id> \d+ ) )x, sub { "rule 2" } Finally, and even i prefer the qr() approach, i don't see why both syntaxes can't cohabitate in Dancer. regards, marc
Hi, Just a global remark about that: r('') is *not* similar to qr// : It's pseudo-regexps, not real ones: all slashes are automatically escaped so you can write: r('/stuff/(.*+)) which would be equivalent to: qr/\/stuff\/(.*+)/ This is very convinient when writing path patterns. this being said, I can clearly see why you want support for real Perl regular expression in Dancer patterns, and I see no reason why we should not let the user use them. So I would vote for applying your patch, but without replacing r('') which has a good reason to live, I think. -- Alexis Sukrieh
Alexis Sukrieh <sukria@sukria.net> writes:
Just a global remark about that: r('') is *not* similar to qr// : It's pseudo-regexps, not real ones: all slashes are automatically escaped so you can write:
r('/stuff/(.*+))
which would be equivalent to:
qr/\/stuff\/(.*+)/
Er, isn't 'r' it literally identical to 'qr(/stuff/(.*+))' in this case? Not that I object to the helper, but given the ability to use markers other than '/' around any 'q' operator, including qr... [...]
So I would vote for applying your patch, but without replacing r('') which has a good reason to live, I think.
The only really good reason I can see, right now, is backward compatibility, but I confess that I only glanced through the implementation rather than performing an exhaustive search — so it may add a behaviour that I have so far missed in my understanding of the tool. Daniel -- ✣ Daniel Pittman ✉ daniel@rimspace.net ☎ +61 401 155 707 ♽ made with 100 percent post-consumer electrons
On Mon, May 31, 2010 at 10:20:48PM +1000, Daniel Pittman wrote:
r('/stuff/(.*+)) qr/\/stuff\/(.*+)/
Er, isn't 'r' it literally identical to 'qr(/stuff/(.*+))' in this case?
yes it is! plus: you don't have to know what r() do: qr is just a perl operator.
So I would vote for applying your patch, but without replacing r('') which has a good reason to live, I think.
The only really good reason I can see, right now, is backward compatibility,
+1 marc
On 31/05/2010 20:18, Marc Chantreux wrote:
On Mon, May 31, 2010 at 10:20:48PM +1000, Daniel Pittman wrote:
r('/stuff/(.*+)) qr/\/stuff\/(.*+)/
Er, isn't 'r' it literally identical to 'qr(/stuff/(.*+))' in this case?
yes it is! plus: you don't have to know what r() do: qr is just a perl operator.
So I would vote for applying your patch, but without replacing r('') which has a good reason to live, I think.
The only really good reason I can see, right now, is backward compatibility,
+1
After reading the whole thread, I must say this makes sense. Now I see r() as something akward and unlogical. We can say that any call to r() will now trigger a warning like: "the r() method is deprectated, use qr// instead". This warning can be kept until version 1.2. In Dancer 1.2, we will drop it. About the captures in splat (mentioned in another mail) I'm OK with it but maybe splat isn't the appropriate accessor. It's intended to get all anonymous captures as a list. Here we may want to add another keyword, like "captures" for instance. Any thought about that? -- Alexis Sukrieh
Alexis Sukrieh <sukria@sukria.net> writes:
On 31/05/2010 20:18, Marc Chantreux wrote:
On Mon, May 31, 2010 at 10:20:48PM +1000, Daniel Pittman wrote:
r('/stuff/(.*+)) qr/\/stuff\/(.*+)/
Er, isn't 'r' it literally identical to 'qr(/stuff/(.*+))' in this case?
yes it is! plus: you don't have to know what r() do: qr is just a perl operator.
So I would vote for applying your patch, but without replacing r('') which has a good reason to live, I think.
The only really good reason I can see, right now, is backward compatibility,
+1
After reading the whole thread, I must say this makes sense. Now I see r() as something akward and unlogical. We can say that any call to r() will now trigger a warning like: "the r() method is deprectated, use qr// instead".
If I may, suggesting 'qr()' or 'qr{}' is probably nicer, not least because this will also work with them: qr{/one/{[-0-9a-f]{24}}/frob} Nesting brackets, as long as they are balanced, is nice, and is one of the attractions of the 'r' method to me. Daniel -- ✣ Daniel Pittman ✉ daniel@rimspace.net ☎ +61 401 155 707 ♽ made with 100 percent post-consumer electrons
On Tue, Jun 01, 2010 at 09:01:51AM +0200, Alexis Sukrieh wrote:
About the captures in splat (mentioned in another mail) I'm OK with it but maybe splat isn't the appropriate accessor. It's intended to get all anonymous captures as a list. Here we may want to add another keyword, like "captures" for instance.
Any thought about that?
- agreed about another accessor. - lot of code makes reference to something else using the capture word so i would choose another one. - perhaps a name which remains the perlvar documentation ( %+ = %LAST_PAREN_MATCH)? - what about paren-> ? regards marc
Le 01/06/2010 15:40, Marc Chantreux a écrit :
On Tue, Jun 01, 2010 at 09:01:51AM +0200, Alexis Sukrieh wrote:
About the captures in splat (mentioned in another mail) I'm OK with it but maybe splat isn't the appropriate accessor. It's intended to get all anonymous captures as a list. Here we may want to add another keyword, like "captures" for instance.
Any thought about that?
- agreed about another accessor. - lot of code makes reference to something else using the capture word so i would choose another one. - perhaps a name which remains the perlvar documentation ( %+ = %LAST_PAREN_MATCH)? - what about paren-> ?
I'd rather have "match" or "matches" here, it's more meaningful for a newcomer. Yes, newcomers are a targeted audience for PerlDancer ;)
On Tue, Jun 01, 2010 at 03:44:10PM +0200, Alexis Sukrieh wrote:
%LAST_PAREN_MATCH)? - what about paren-> ?
I'd rather have "match" or "matches" here, it's more meaningful for a newcomer.
paren, match, paren_match, url_chunk, url_part, url_match, .... little preference for url_match for the moment.
Yes, newcomers are a targeted audience for PerlDancer ;)
I'm a newcomer! I Actually never used dancer to complete a task. Not yet :) marc
hello, The last commit on my repo (http://github.com/eiro/Dancer) is able to run successfully the following code. #! /usr/bin/perl use Modern::Perl; use Dancer; for ( [qw/ log core /] , [qw/ logger Console /] , [qw/ warnings 1 /] , [qw/ show_errors 1 /] , [qw/ auto_reload 0 /] ) { set @$_ } any [qw| get post |] , qr{ / (?<class> user | post ) / (?<action> C|R|U|D | create | read | update | delete ) / (?<id> \d+) /? $ }x , sub { my $url = url_paren; content_type 'text/plain'; join "\n", map {"$_ = $$url{$_}"} keys %$url; }; any [qw| get post |], qr/.*/, sub { "nothing" }; dance; I tested it with
curl http://localhost:3000/user/C/324 action = C class = user id = 324
but there is nothing new in the test suite: this is only a working proof of concept waiting for code review. beware if you clone my repo: i added some stuff broking the test suite and which aren't related to this topic. Regards. marc
On Mon, May 31, 2010 at 01:15:11PM +0200, Alexis Sukrieh wrote:
this being said, I can clearly see why you want support for real Perl regular expression in Dancer patterns, and I see no reason why we should not let the user use them.
The other question was: is everyone ok about the captures? can i work on it ? Regards, marc
participants (3)
-
Alexis Sukrieh -
Daniel Pittman -
Marc Chantreux