Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

incrementing Bailador::Request #50

Merged
merged 4 commits into from
Mar 31, 2016
Merged

incrementing Bailador::Request #50

merged 4 commits into from
Mar 31, 2016

Conversation

garu
Copy link
Contributor

@garu garu commented Mar 20, 2016

Hi! Thanks for porting Dancer2 to Perl 6 :)

I am trying to use Bailador in a project and realized there are still a lot of things missing. This PR implements some methods that are missing from Dancer2::Request, namely:

  • headers
  • user_agent
  • referer
  • address
  • remote_host
  • protocol
  • user
  • script_name
  • is_ajax

It also implements the following changes in the cookies() method:

  • cache results
  • considers ',' as a separator as well, as per RFC and other implementations
  • puts cookie values in array, like Dancer2::Request does
  • trims the data

Finally, I also refactored params() and allowed it to receive more than one argument (e.g.: /url&foo=42&foo=bar).

All changes and new features have tests included in the commit.

Hope this helps. Thanks again for creating Bailador :)

Cheers!

This commit implements some missing methods from Dancer2::Request,
fixes an issue with request parameter parsing and adds tests.
@@ -25,40 +27,61 @@ class Bailador::Request {
}
multi method params ($source) {
my %ret;
my $source_data = '';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the = '' ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because in line 42 (return %ret unless $source_data.chars;), $source_data could still be uninitialized, and checking $source_data.chars is issuing a warning. I'm not sure how to deal with that other than initializing the variable:

 perl6 -e 'my $src; say 42 unless $src.chars'
Method 'chars' not found for invocant of class 'Any'
  in block <unit> at -e line 1
$ perl6 -e 'my Str $src; say 42 unless $src.chars'
Use of uninitialized value of type Str in string context
Any of .^name, .perl, .gist, or .say can stringify undefined things, if needed.  in block <unit> at -e line 1
42
$ perl6 -e 'my $src = q<>; say 42 unless $src.chars'
42

If there's a more idiomatic way to write this bit in Perl 6, I'd love to hear about it and amend the PR :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. I was thinking my Str $source_data was enough. So you're right.

Btw, thanks for this PR ! :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I spoke to psch on #perl6 and he said .chars is right to trigger a warning, since it's counting something that is just not there (not empty, simply not even defined). I kind of agree with that, which led me to change these lines in commit e508dad to try and be a little more idiomatic, checking for defined-ness instead :)

@garu
Copy link
Contributor Author

garu commented Mar 30, 2016

@tadzik bump! :)

@tadzik tadzik merged commit 9ecabd3 into Bailador:master Mar 31, 2016
@tadzik
Copy link
Collaborator

tadzik commented Mar 31, 2016

Excellent work, thanks a lot!

@garu garu deleted the garu/improving_requests branch March 31, 2016 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants