Repository navigation
fix: parse sample count in inspect mode - #261
Merged
Merged
Conversation
--inspect sample N and --inspect=sample N now consume an immediately following positive integer as the sample count, with the same rules as the deprecated --sample flag (flags, --, and non-numeric names are not consumed; zero is a usage error with exit 1). Previously the count was left as a positional argument and treated as an input file. Clarify the positive-integer contract and the ./N / -- disambiguation in the CLI help, README, and man page. Fixes #237
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
--inspect sample Nand--inspect=sample Ntreated the countNas a positionalinput filename, so e.g.
sql-pipe --inspect sample 2failed withcannot open file '2'(exit 2) instead of printing the schema plus 2 rows.
The parser now consumes an immediately following positive integer as the sample count
for both canonical
--inspectsyntaxes, using exactly the same rules the deprecated--sampleflag already used: flags and--are not consumed, non-numeric names areleft as file arguments, and
0is a usage error (stderr diagnosticsample count must be a positive integer (--inspect sample or --sample), exit 1). To sample a file whosename is a number, use
./Nor put it after--.build.zig(9 new Bash integration tests); the fivebehavioral ones (
--inspect sample 2,--inspect=sample 2, count-not-file with--no-stdin, zero-count usage error, missing-source diagnostic with count) failedagainst the unmodified source (exit 2 / default count) and pass after the fix.
(
src/args.zig, example changed from--sample 5to--inspect sample 5), READMEinspect section and
--inspecttable row, and the man page source(
docs/sql-pipe.1.scd).--sample [n],--sample=n, the deprecation warning, and the exit-codecontract (0/1/2/3) are unchanged.
Fixes #237
Validation
zig build -Dbundle-sqlite=true -Doptimize=ReleaseSafe— passzig build unit-test -Dbundle-sqlite=true— passzig build test -Dbundle-sqlite=true— pass (all integration tests green;before the fix the 5 new regression tests failed as expected)
printf 'name,age\nAlice,30\nBob,25\nCarol,35\n' | ./zig-out/bin/sql-pipe --inspect sample 2prints schema on stderr and header + 2 rows on stdout, exit 0;
--inspect=sample 2same;--inspect sample 0exits 1 with the new diagnosticzig fmt --checkon tracked*.zigfiles reports pre-existing violations on master(also flagged by CI lint setup); the files touched here are already nonconforming on
master and none of the reported diffs intersect the new code
Note: GitHub CI does not run the Bash integration suite on Windows; Windows jobs only
compile and run unit tests, so the new CLI cases are verified on Linux/macOS only.