h3 icon indicating copy to clipboard operation
h3 copied to clipboard

H3 CLI Hierarchical Subcommands

Open dfellis opened this issue 1 year ago • 10 comments

This adds all of the hierarchical subcommands. Everything was easy until I reached compactCells and uncompactCells.

I am not happy with the way the flexible parser ended up in a soup of looping constructs, but I couldn't figure out any way that wasn't worse in some ways (eg, using goto and labels). I also couldn't figure out how handle a completely undelimited input file without copying the characters to a temporary string since you can have more than 15 hex characters in a 64-bit integer, so it would want to slurp up follow-on characters. We can simplify a bit if you think an undelimited deluge of hex characters is not a reasonable input format we should handle.

I also didn't DRY the huge similarities between the two functions. I am willing to do that, but only after we agree on how the string parsing logic should look like.

dfellis avatar Jun 14 '24 14:06 dfellis

Coverage Status

coverage: 98.826%. remained the same when pulling 1cbcb25010ece10aaf5fe02d966e88d7d54047bc on h3-cli-hierarchical-subcommands into efdd514b2d80c49973e5dd55a842f68be73ed0e5 on master.

coveralls avatar Jun 14 '24 14:06 coveralls

I think this may be why the tests are failing. It's setting up a build directory that is not the CWD, while the files are using relative paths. I need to see how to get the directory they're actually being put in vs where make test is getting invoked.

dfellis avatar Jun 14 '24 15:06 dfellis

Coverage Status

coverage: 98.826%. remained the same when pulling e58e5df37de4e7b9d8c9b788b3c18ee833e11d2e on h3-cli-hierarchical-subcommands into efdd514b2d80c49973e5dd55a842f68be73ed0e5 on master.

coveralls avatar Jun 14 '24 16:06 coveralls

Still something funky with Windows. I'll need to dust off my VM.

dfellis avatar Jun 14 '24 16:06 dfellis

Coverage Status

coverage: 98.826%. remained the same when pulling 4f817709e4f527994df5d58bc26961df92443520 on h3-cli-hierarchical-subcommands into efdd514b2d80c49973e5dd55a842f68be73ed0e5 on master.

coveralls avatar Jun 14 '24 19:06 coveralls

Hmm... There was another error that I didn't notice before. https://github.com/uber/h3/actions/runs/9521371842/job/26248637576?pr=846#step:7:301

Let me see if I can figure that out.

dfellis avatar Jun 14 '24 19:06 dfellis

So that last failure was legit, and was a copy-paste error when I set up arg parsing in one of the new subcommands.

I have no idea what black magic the GNU libc is doing under the hood to get that casting to work "correctly" on Linux.

dfellis avatar Jun 14 '24 20:06 dfellis

Coverage Status

coverage: 98.826%. remained the same when pulling 4bea9bd4908fe6b822fcd08b537dcb62821c077f on h3-cli-hierarchical-subcommands into efdd514b2d80c49973e5dd55a842f68be73ed0e5 on master.

coveralls avatar Jun 14 '24 20:06 coveralls

Coverage Status

coverage: 98.826%. remained the same when pulling 4bea9bd4908fe6b822fcd08b537dcb62821c077f on h3-cli-hierarchical-subcommands into efdd514b2d80c49973e5dd55a842f68be73ed0e5 on master.

coveralls avatar Jun 14 '24 20:06 coveralls

Coverage Status

coverage: 98.824%. remained the same when pulling 3880fe2e91ad80c5f2177aa916c421b91d04b903 on h3-cli-hierarchical-subcommands into d6f2701c5f3fddef01c1fc85d6bbe085b397500a on master.

coveralls avatar Jul 15 '24 16:07 coveralls

Just rebased to get the new tests run on this branch. Since I have two approvals if these new tests also pass I'll merge this.

dfellis avatar Oct 03 '24 23:10 dfellis