Initial commit of booleanPointOnLine #858
Conversation
|
@rowanwins @DenisCarriere |
👍 I like this change for a next major release, it is a big confusing when they aren't grouped and the name is too generalized. |
| module.exports = function (point, linestring, ignoreEndVertices) { | ||
| var pointCoords = getCoords(point); | ||
| var lineCoords = getCoords(linestring); | ||
| for (var i = 0; i < lineCoords.length - 1; i++) { |
There was a problem hiding this comment.
👍 Code looks good.
We really need a segmentEach module (also mentioned here #856 (comment))
There was a problem hiding this comment.
The only thing I find tricky about the invariant type modules is a lack of flow control, but in a lot of cases they are super-handy!
There was a problem hiding this comment.
What do you mean by lack of flow control?
| * @param {Geometry|Feature<Point>} point GeoJSON Feature or Geometry | ||
| * @param {Geometry|Feature<LineString>} linestring GeoJSON Feature or Geometry | ||
| * @param {Boolean} [ignoreEndVertices=false] whether to ignore the start and end vertices. | ||
| * @returns {Boolean} true/false |
There was a problem hiding this comment.
I remember having this conversation with @tmcw about Boolean vs. boolean.
From what I can read on JSDocs, it's saying to use boolean
http://usejsdoc.org/tags-type.html
Also DocumentationJS linting flags Boolean as a warning (which we will be adding soon)
This change applies to many modules, so don't need to fix this right now.
| type Feature = GeoJSON.Feature<any> | GeoJSON.GeometryObject; | ||
|
|
||
| /** | ||
| * http://turfjs.org/docs/#booleanPointOnLine |
There was a problem hiding this comment.
All lowercase for the URL.
Example http://turfjs.org/docs/#centerOfMass (doesn't work)
| * PointOnLineStart: 0.021ms | ||
| * PointOnLineEnd x 13,957,263 ops/sec ±0.53% (91 runs sampled) | ||
| * PointOnLineMidpoint x 17,388,052 ops/sec ±0.46% (94 runs sampled) | ||
| * PointOnLineStart x 17,036,405 ops/sec ±1.34% (91 runs sampled) |
|
|
||
| /** | ||
| * http://turfjs.org/docs/#booleanpointonline | ||
| */ | ||
| declare function booleanPointOnLine(feature1: Point, feature2: Line, ignoreEndVertices: boolean): boolean; | ||
| declare function booleanPointOnLine(point: Point, linestring: Line, ignoreEndVertices?: boolean): boolean; |
There was a problem hiding this comment.
@rowanwins Another thing with these definitions, I try to use the same parameter names as the index.js
And ignoreEndVertices is optional, you can add ? to define optional params.
Please fill in this template.
Use a meaningful title for the pull request. Include the name of the package modified.
Have read How To Contribute.
Run
npm testat the sub modules where changes have occurred.Run
npm run lintto ensure code style at the turf module level.Overview description of proposed module.
Include JSDocs with a basic example.
Execute
./scripts/generate-readmesto createREADME.md.Add yourself to contributors in
package.jsonusing "Full Name <@github Username>".booleanPointOnLine
Returns true if a point is on a line. Accepts a optional parameter to ignore the start and end vertices of the linestring.
This is a snippet that we've used across a number of the boolean modules now so figured it was worth separating out.
Ref. #809