Skip to content

refactors; improved errors; accept -- arbitrary positionals#7

Merged
joshprk merged 21 commits into
joshprk:mainfrom
sodiboo:buncha-refactoring
Jul 24, 2025
Merged

refactors; improved errors; accept -- arbitrary positionals#7
joshprk merged 21 commits into
joshprk:mainfrom
sodiboo:buncha-refactoring

Conversation

@sodiboo

@sodiboo sodiboo commented Jul 23, 2025

Copy link
Copy Markdown
Contributor

I was going to make some changes to the formatting, but i got distracted and i ended up doing a lot of cleaning up and refactoring and other misc improvements.

  • the command-line argument parsing is now more correct.
  • some error messages are a bit more consistent or accurate.
  • some obvious error cases that were ignored, are now actually errors.
  • some allocation has been eliminated.
  • a lot of internal names have been made more consistent.

there are a lot of individual changes; i tried to keep a tidy commit history when doing them. each commit is fairly small and individually valid; it's probably easier to review this commit-by-commit instead of looking at the final diff of all the commits and trying to review that.

sodiboo added 21 commits July 23, 2025 21:18
it behaves identically; just it's nicer to read imo
`node.insert` pulls in a bunch of logic about not having duplicate keys;
but that's already guaranteed by a json map, so there's no reason to do
that again in our code. this is simpler, faster, and smaller (yay!
premature optimization!)
names can be empty; and we never did check that they're nonempty
"an array" over "json array" to match "arguments must be an array"
previously, this would throw "name must be a string"; since name field
is obligatory.
because options now canonically come "before" the positional arguments,
i also flipped the help text
@joshprk

joshprk commented Jul 24, 2025

Copy link
Copy Markdown
Owner

Thanks for cleaning up the code!

@joshprk
joshprk merged commit 5be4f39 into joshprk:main Jul 24, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants