Implement the traversal subcommands for the H3 cli #839
Conversation
| if (err) { | ||
| return err; | ||
| } | ||
| H3Index *out = calloc(len, sizeof(H3Index)); |
There was a problem hiding this comment.
Do we need to check for failed memory allocation here?
There was a problem hiding this comment.
Technically yes, but the app is going to die if this happens no matter what if this happens. But I'd also assume anybody on a machine that cannot allocate 8 bytes is having a much harder time, which is why I left that error handling out?
There was a problem hiding this comment.
This could be much larger than 8 bytes with larger values of k. I think the program is better behaved if it explicitly tells the user that memory allocation failed rather than risk what happens with segfaulting, as in that case the user would have a much harder time tracking down what happened (they would need to attach a debugger to find out the problem is the k value.)
There was a problem hiding this comment.
Addressed. There's now a print to stderr and an exit for each if the pointer is NULL.
| .valueName = "distance", | ||
| .value = &k, | ||
| .helpText = "Maximum grid distance for the output set"}; | ||
| Arg prettyArg = { |
There was a problem hiding this comment.
FWIW, I'd skip - interested callers have access to plenty of other pretty-printing tools
There was a problem hiding this comment.
Probably true. But I wasn't sure if we should rely on the users having jq, especially if they're more on the traditional geospatial/data analyst side of things and less familiar with the terminal.
| // Since we don't know *actually* how many cells are in the output (usually | ||
| // the max, but sometimes not), we need to do a quick scan to figure out the | ||
| // true length in order to properly serialize to a JSON array | ||
| int64_t trueLen = 0; |
There was a problem hiding this comment.
This is all for comma handling, right? Could this be simplified if you output a leading comma, not a trailing one, for every cell but out[0]?
There was a problem hiding this comment.
I can't do that because I can't assume out[0] has a value.
That's what I originally had until @isaacbrodsky pointed out my mistake there.
| if (err) { | ||
| return err; | ||
| } | ||
| H3Index *out = calloc(len, sizeof(H3Index)); |
There was a problem hiding this comment.
This could be much larger than 8 bytes with larger values of k. I think the program is better behaved if it explicitly tells the user that memory allocation failed rather than risk what happens with segfaulting, as in that case the user would have a much harder time tracking down what happened (they would need to attach a debugger to find out the problem is the k value.)
| @@ -13,56 +13,57 @@ The public API of H3 is covered in the following fuzzers: | |||
|
|
|||
| | Function | File or status | |||
| | -------- | -------------- | |||
| | latLngToCell | [fuzzerLatLngToCell](./fuzzerLatLngToCell.c) | |||
| | cellToLatLng | [fuzzerCellToLatLng](./fuzzerCellToLatLng.c) | |||
| | areNeighborCells | [fuzzerDirectedEdge](./fuzzerDirectedEdge.c) | |||
There was a problem hiding this comment.
for my curiosity - was this an editor feature you used to sort this? or did you manually alphabetize?
There was a problem hiding this comment.
Simple :sort in vim after highlighting the lines I wanted to sort. :)
There was a problem hiding this comment.
I figured it must be some vim thing :)
Co-authored-by: Isaac Brodsky <isaac@isaacbrodsky.com>
This implements all of the traversal subcommands.
The test suite has not been implemented yet, as I intend to rework the way CLI tests are structured, as requested by @nrabinowitz.
But I'm putting this up now before I start on that in case there are any changes to the input and output formats to be done before I start codifying them into the tests.