[BREAKING] Make take/drop/slice to validate their arguments and throw an appropriate error.

CHANGELOG.md

@@ -7,6 +7,30 @@ this library.
 
 3.0.0
 -----
+### Breaking changes
+* `drop`
+  [#559](https://github.com/caolan/highland/pull/595)
+  Fixes [#594](https://github.com/caolan/highland/issues/594)
+  * **Old Behavior**: if `n` isn't a number or is negative, it defaults to `0`.
+  * **New Behavior**: if `n` is not a number or if it is negative, an error is
+    thrown.
+* `slice`
+  [#559](https://github.com/caolan/highland/pull/595)
+  Fixes [#594](https://github.com/caolan/highland/issues/594)
+  * **Old Behavior**: if `start` isn't a number, it defaults to `0`, and if
+    `end` isn't a number, it defaults to `Infinity`. If `start` or `end` are
+    negative, they are treated as if they are `0` instead.
+  * **New Behavior**: if `start` or `end` are not numbers or if they are
+    negative, an error is thrown. `start` and `end` are optional and default to
+    `0` and `Infinity`, respectively.
+* `take`
+  [#559](https://github.com/caolan/highland/pull/595)
+  Fixes [#594](https://github.com/caolan/highland/issues/594)
+  * **Old Behavior**: if `n` isn't a number, it defaults to `Infinity`. If `n`
+    is negative, it is treated as if it is `0`.
+  * **New Behavior**: if `n` is not a number or if it is negative, an error is
+    thrown.
+
 ### New additions
 * `wrapAsync`: Wraps a function that returns a promise, transforming it to a
   function which accepts the same arguments and returns a Highland Stream instead.

docs/templates/base.html

@@ -258,6 +258,11 @@ <h3 id="currying">Currying</h3>
 _.map(doubled, mystream)</code></pre>
       <p>By convention, all top-level functions are "curryable", meaning you can partially apply their arguments. In the above example, this could be called as:</p>
       <pre><code class="javascript">_.map(doubled)(mystream);</code></pre>
+      <p><b>Warning:</b> curried transforms do not respect optional arguments.
+      You must pass every declared argument to the function, using
+      <code>null</code> for arguments that you don't want to provide. For
+      example, <code>stream.slice(1)</code> is equivalent to <code>_.slice(1,
+      null)(stream)</code> and not <code>_.slice(1)(stream)</code>.</p>
       <p>In real-world use, this means you can define the behaviour you'd like before knowing what stream you'd like to perform it on:</p>
       <pre><code class="javascript">// partially apply the filter() function to create a new function
 var getBlogposts = _.filter(function (doc) {
@@ -317,12 +322,24 @@ <h2 id="{{name}}">{{name}}</h2>
                 <li>
                   <code>{{name}}</code>
                   {{#if type}} - <em>{{type}}</em>{{/if}}
-                  {{#if description}} - {{description}}{{/if}}
+                  {{#if description_html}} - {{{description_html}}}{{/if}}
                 </li>
               {{/each}}
             </ul>
             {{/if}}
 
+            {{#if throws}}
+            <div class="doc-params-heading">Throws</div>
+            <ul>
+              {{#each throws}}
+              <li>
+                <code>{{type}}</code>
+                {{#if description}} - {{{description_html}}}{{/if}}
+              </li>
+              {{/each}}
+            </ul>
+            {{/if}}
+
             {{#if example}}
             <pre><code class="javascript">{{example}}</code></pre>
             {{/if}}

lib/index.js

@@ -278,6 +278,17 @@ var isES5 = (function () {
     return Function.prototype.bind && !this;
 }());
 
+function checkIsNumber(parameter, paramName) {
+    if (typeof parameter != 'number') {
+        throw new TypeError('Parameter "' + paramName + '" is not a number.');
+    }
+}
+
+function checkRange(predicate, message) {
+    if (!predicate) {
+        throw new RangeError(message);
+    }
+}
 
 _.isUndefined = function (x) {
     return typeof x === 'undefined';
@@ -2960,25 +2971,39 @@ addMethod('split', function () {
 });
 
 /**
- * Creates a new Stream with the values from the source in the range of `start` (inclusive) to `end` (exclusive).
- * `start` and `end` must be of type `Number`, if `start` is not a `Number` it will default to `0`
- * and, likewise, `end` will default to `Infinity`: this could result in the whole stream being be
- * returned.
+ * Creates a new Stream with the values from the source in the range of `start`
+ * (inclusive) to `end` (exclusive).
  *
  * @id slice
  * @section Transforms
  * @name Stream.slice(start, end)
- * @param {Number} start - integer representing index to start reading from source (inclusive)
- * @param {Number} stop - integer representing index to stop reading from source (exclusive)
+ * @param {Number} start - (optional) integer representing index to start
+ *     reading from source (inclusive). Defaults to `0` if not specified.
+ * @param {Number} stop - (optional) integer representing index to stop
+ *     reading from source (exclusive). Defaults to `Infinity` if not
+ *     specified.
+ * @throws {TypeError} if either parameters are not numbers.
+ * @throws {RangeError} if either parameters are negative.
  * @api public
  *
  * _([1, 2, 3, 4]).slice(1, 3) // => 2, 3
  */
 
 addMethod('slice', function(start, end) {
+    if (start == null) {
+        start = 0;
+    }
+
+    if (end == null) {
+        end = Infinity;
+    }
+
+    checkIsNumber(start, 'start');
+    checkIsNumber(end, 'end');
+    checkRange(start >= 0, 'start cannot be negative.');
+    checkRange(end >= 0, 'end cannot be negative.');
+
     var index = 0;
-    start = typeof start != 'number' || start < 0 ? 0 : start;
-    end = typeof end != 'number' ? Infinity : end;
 
     if (start === 0 && end === Infinity) {
         return this;
@@ -3007,13 +3032,14 @@ addMethod('slice', function(start, end) {
 });
 
 /**
- * Creates a new Stream with the first `n` values from the source. `n` must be of type `Number`,
- * if not the whole stream will be returned.
+ * Creates a new Stream with the first `n` values from the source.
  *
  * @id take
  * @section Transforms
  * @name Stream.take(n)
  * @param {Number} n - integer representing number of values to read from source
+ * @throws {TypeError} if `n` is not a number.
+ * @throws {RangeError} if `n` is negative.
  * @api public
  *
  * _([1, 2, 3, 4]).take(2) // => 1, 2
@@ -3026,14 +3052,16 @@ addMethod('take', function (n) {
 });
 
 /**
- * Acts as the inverse of [`take(n)`](#take) - instead of returning the first `n` values, it ignores the
- * first `n` values and then emits the rest. `n` must be of type `Number`, if not the whole stream will
- * be returned. All errors (even ones emitted before the nth value) will be emitted.
+ * Acts as the inverse of [`take(n)`](#take) - instead of returning the first
+ * `n` values, it ignores the first `n` values and then emits the rest. All
+ * errors (even ones emitted before the nth value) will be emitted.
  *
  * @id drop
  * @section Transforms
  * @name Stream.drop(n)
  * @param {Number} n - integer representing number of values to read from source
+ * @throws {TypeError} if `n` is not a number.
+ * @throws {RangeError} if `n` is negative.
  * @api public
  *
  * _([1, 2, 3, 4]).drop(2) // => 3, 4

package.json

@@ -34,6 +34,7 @@
     "orchestrator": "^0.3.7",
     "rsvp": "~3.3.1",
     "scrawl": "0.0.5",
+    "showdown": "^1.6.4",
     "sinon": "~1.17.3",
     "stream-array": "~1.1.2",
     "through": "~2.3.4",

tasks/docs.js

@@ -1,7 +1,8 @@
 var handlebars = require('handlebars'),
     scrawl = require('scrawl'),
     path = require('path'),
-    fs = require('fs');
+    fs = require('fs'),
+    showdown = require('showdown');
 
 
 var src_file = path.resolve(__dirname, '../lib/index.js');
@@ -34,6 +35,40 @@ function groupBySectionsInSectionsOrder(c1, c2){
   return ( idx1 - idx2 );
 }
 
+function renderMarkdown(markdown, stripPTags) {
+    var converter = new showdown.Converter();
+    var html = converter.makeHtml(markdown);
+
+    if (stripPTags) {
+        html = html
+            .replace(/^<p>/, '')
+            .replace(/<\/p>$/, '');
+    }
+
+    return html;
+}
+
+function parseThrowsTag(throwsTag) {
+    // Looks like @throws {Type} my comment.
+    if (!Array.isArray(throwsTag)) {
+        throwsTag = [throwsTag];
+    }
+
+    var throwsTagRegex = /^\{([^\}]+)\}(.*)/;
+    return throwsTag.map(function (tag) {
+        var match = throwsTagRegex.exec(tag);
+        var description = match[2];
+        var descriptionHtml = renderMarkdown(description, true);
+
+        // Strip starting and ending tags.
+        return {
+            type: match[1],
+            description: description,
+            description_html: descriptionHtml
+        };
+    });
+}
+
 module.exports = function (grunt) {
 
     grunt.registerTask('docs',
@@ -66,6 +101,20 @@ module.exports = function (grunt) {
                     throw new Error('Duplicate id:' + c.id);
                 }
                 c.tag = grunt.config.get('pkg.version')
+
+                if (c.throws) {
+                    c.throws = parseThrowsTag(c.throws);
+                }
+
+                if (c.params) {
+                    c.params.forEach(function (param) {
+                        if (param.description) {
+                            param.description_html =
+                                renderMarkdown(param.description, true);
+                        }
+                    });
+                }
+
                 items.push(c);
             });