Multiline comments #228
Multiline comments #228
Conversation
Since there were no tests for t.comment(), which I'm going to modify, I want to make sure of what the current behaviour is, to ensure I don't change more than I need to.
|
We should probably split on |
| @@ -102,7 +102,10 @@ Test.prototype.test = function (name, opts, cb) { | |||
| }; | |||
|
|
|||
| Test.prototype.comment = function (msg) { | |||
| this.emit('result', trim(msg).replace(/^#\s*/, '')); | |||
| var that = this; | |||
| String(trim(msg)).split('\n').forEach(function (aMsg) { | |||
There was a problem hiding this comment.
No need to cast to string here. Trim helper should return a string.
|
Really nice. Tests look good 👍 will merge after 24hr to let other reviewers leave comments |
|
@Raynos LGTM as well, pending #228 (comment) |
|
Thanks for the feedback! |
|
👍 Thanks for doing this! Just came across this issue as well |
| t.comment([ | ||
| 'c', | ||
| 'd', | ||
| ].join('\r\n')); |
There was a problem hiding this comment.
won't this result in invisible \r characters appearing in the output? it's likely that this is getting obscured in the test.
There was a problem hiding this comment.
They're actually being trim()med in test.js:107: removing the inner call to trim() makes this part of the test fail.
There was a problem hiding this comment.
aha, ok that makes sense then :-) thanks for clarifying
This should address #196.
Knowing nothing of tap I probably added more test code than needed, but I wanted to try and make sure I don't break existing functionality.
All feedback is welcome!