Fix issue where certain characters would break files with multiple responses (#58)

[kevinschumacher]

Per discussion on #58 and #59, split responses in Canned.prototype.getEachResponse() by lines starting "//!" instead of using the regex to match.

[sideshowcoder]

Sorry to be a little picky here :( but this change would break a lot of internal uses for me as I can no longer just require("canned") to make stuff work. Thanks so much for your work, so happy to see people working on this.

[kevinschumacher]

Yeah I thought that might be controversial, but I'm not at all familiar with node.js and wasn't sure on the best approach to testing it.

Could break out the functionality around multiple responses per file into its own module -- at slightly deeper glance, doesn't look like much effort at all. I'll update the PR like so:

new module, lib/variable-response.js, to include: Canned.prototype.getVariableResponse --> loose getVariableResponse Canned.prototype.getEachResponse --> loose getEachResponse getSelectedResponse

And move canned.stripBodyComments --> util.stripBodyComments

[sideshowcoder]

Sounds like a good idea :)

On Fri, Mar 27, 2015 at 1:30 PM -0700, "kevinschumacher" notifications@github.com<mailto:notifications@github.com> wrote:

Yeah I thought that might be controversial, but I'm not at all familiar with node.js and wasn't sure on the best approach to testing it.

Could break out the functionality around multiple responses per file into its own module -- at slightly deeper glance, doesn't look like much effort at all. I'll update the PR like so:

new module, lib/variable-response.js, to include: Canned.prototype.getVariableResponse --> loose getVariableResponse Canned.prototype.getEachResponse --> loose getEachResponse getSelectedResponse

And move canned.stripBodyComments --> util.stripBodyComments

Reply to this email directly or view it on GitHubhttps://github.com/sideshowcoder/canned/pull/60#issuecomment-87079566.

[kevinschumacher]

@sideshowcoder would appreciate any comments on whether I've done anything "weird" here (as I said I'm not at all familiar with node)

[sideshowcoder]

Using your tests I was able to take a look at this without pulling my hair out what I need todo :+1: I think the split can be done using a quite simple regex with lookahead to split at the //! line. What do you think about #61 personally I would go for the simple regex if it solves the problem, which we can hopefully confirm maybe? @simonprickett

I love the idea of pulling this out in a different object so, I'm going to take a closer look at your code in a minute as well.

[kevinschumacher]

Really appreciate the feedback! Once #61 gets merged I'm happy to make the suggested adjustments around the response parser / response set object.

[kevinschumacher]

Or I can close/rebase/new PR into that branch as well if you prefer to put this all in the same rev

[sideshowcoder]

@kevinschumacher whatever you prefer... I cherry picked your tests into my branch so it should rebase fine.

[sideshowcoder]

I'm gonna close this for now, reopen if it is convinient.