refactor(import): show actual arg count in import error messages#9429
Merged
macneale4 merged 1 commit intodolthub:mainfrom Jul 8, 2025
Merged
Conversation
Contributor
|
Please rebase your changes so there is a clean history. |
9a8a52e to
1f93d15
Compare
Update import command validation to display the number of arguments received when validation fails. Previously, errors only showed "expected 1 or 2 arguments" without indicating how many were actually provided. Now shows "expected 1 argument (for stdin) or 2 arguments (table and file), but received N" for clearer debugging. The argparser already shows argument counts for too many arguments (e.g. "Expected at most 2, found 3"), but this change ensures consistent messaging for all validation cases. Refs: dolthub#1083
1f93d15 to
310c0b1
Compare
Contributor
Author
|
done |
Contributor
Author
|
Hey sorry just bumping this since maybe it got missed after holiday weekend. Thank you |
Contributor
|
Thanks for the bump. I'll take a look! |
macneale4
approved these changes
Jul 8, 2025
Contributor
macneale4
left a comment
There was a problem hiding this comment.
Looks great! Thanks for the contribution!
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.
Description
This addresses the second comment/request in #1083. It improves the error messages for dolt table import when incorrect argument counts are provided.
Problem
Previously, when users provided the wrong number of arguments to dolt table import, they would see:
expected 1 or 2 argumentsThis message didn't indicate how many arguments were actually provided, making it difficult to debug command issues.
Solution
Updated the argument validation to show the actual argument count:
expected 1 argument (for stdin) or 2 arguments (table and file), but received 0"expected at most 2 arguments (table and file), but received NAdditionally, the argparser now shows all provided arguments when there are too many, e.g.:
error: import has too many positional arguments.
Expected at most 2, found 3: year,state_fips, precinct_results, test.csvTesting
Added bats tests to verify the improved error messages: