Skip to content
This repository has been archived by the owner on Jun 19, 2020. It is now read-only.

Providing casper.getCurrentHeader #188

Closed
wants to merge 14 commits into from
Closed

Providing casper.getCurrentHeader #188

wants to merge 14 commits into from

Conversation

thom4parisot
Copy link
Contributor

Helper to easilly access a response header from anywhere.

@n1k0
Copy link
Member

n1k0 commented Jul 18, 2012

I'm not sure for the getCurrentHeader* names, mostly because it's not obvious if we're talking about response or request ones.

Also, I'm thinking a neat API would be something more like:

var accept = casper.currentResponse.headers.get('Content-Length');

What do you think?


var headerValue = null;

this.getCurrentHeaders().some(function(header){
Copy link
Member

Choose a reason for hiding this comment

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

Why not using forEachhere (or maybe more appropriately filter)?

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 some stops the loop on the first true value returned during iterations.
But I forgot the return true 2 lines below :-D

The advantage is to stop the loop as soon as the header is found.

Copy link
Member

Choose a reason for hiding this comment

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

Ah ok, now I understand it better ;)

@n1k0
Copy link
Member

n1k0 commented Jul 19, 2012

Now, I'm wondering if I shouldn't pass the response object to the step callback:

casper.start('http://google.com/', function(response) {
    if (/json/.test(response.headers.get('Content-Type'))) {
        this.echo('JSON spotted');
    }
});

That would break any old codebase relying on the deprecated self argument passed as a first parameter though…

Thoughts?

@thom4parisot
Copy link
Contributor Author

Yay, it would be even easier. And shorter to write. And less ambiguous than retrieving casper.currentResponse.

@thom4parisot
Copy link
Contributor Author

Okay, pushed a cleaner API and case-insensitive retrieval.

@@ -1583,6 +1583,34 @@ Casper.extend = function(proto) {

exports.Casper = Casper;

/*
Copy link
Member

Choose a reason for hiding this comment

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

What about putting this in a dedicated http module?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep why not. I don't know your plans for this module, as request headers are just a plain object.

@n1k0
Copy link
Member

n1k0 commented Jul 19, 2012

Very nice, things are shaping up nicely.

Regarding my comment about passing the response as an argument of the step callback, any thought?

@thom4parisot
Copy link
Contributor Author

Yep indeed. I may do that in another PR, especially as I've not digged too much in steps and it seems there are some subtle stuff with history etc.

@thom4parisot
Copy link
Contributor Author

Finally put the response in the step.

As expected, browsing back and forward does not match with currentResponse values.
It would imply to save the whole response state (status etc.)

@@ -1100,7 +1100,7 @@ Casper.prototype.runStep = function runStep(step) {
}, this.options.stepTimeout, this, new Date().getTime(), this.step);
}
this.emit('step.start', step);
stepResult = step.call(this, this);
stepResult = step.call(this, this.currentResponse || undefined);
Copy link
Member

Choose a reason for hiding this comment

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

Just this.currentResponse would suffice

@thom4parisot
Copy link
Contributor Author

Do you need anything more for this PR?

@donl
Copy link

donl commented Oct 17, 2012

How do you access the response headers with the latest pull request?

n1k0 added a commit that referenced this pull request Oct 18, 2012
n1k0 added a commit that referenced this pull request Oct 18, 2012
@n1k0
Copy link
Member

n1k0 commented Oct 18, 2012

Merged

@n1k0 n1k0 closed this Oct 18, 2012
n1k0 added a commit that referenced this pull request Oct 18, 2012
@n1k0
Copy link
Member

n1k0 commented Oct 18, 2012

@Doni documentation is here http://casperjs.org/api.html#casper.then.callbacks

@thom4parisot thom4parisot deleted the feature-getHeader branch November 14, 2013 10:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants