Skip to content

Conversation

@codeaucafe
Copy link
Contributor

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 arguments

This 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:

  • For 0 arguments:
    expected 1 argument (for stdin) or 2 arguments (table and file), but received 0
  • For >2 arguments:
    "expected at most 2 arguments (table and file), but received N

Additionally, 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.csv

Testing

Added bats tests to verify the improved error messages:

@macneale4 macneale4 self-requested a review July 2, 2025 22:32
@macneale4
Copy link
Contributor

Please rebase your changes so there is a clean history.

@codeaucafe codeaucafe force-pushed the codeaucafe/fix/1083-import-arg-count-error-msg branch 2 times, most recently from 9a8a52e to 1f93d15 Compare July 4, 2025 05:08
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
@codeaucafe codeaucafe force-pushed the codeaucafe/fix/1083-import-arg-count-error-msg branch from 1f93d15 to 310c0b1 Compare July 4, 2025 05:13
@codeaucafe
Copy link
Contributor Author

done

@codeaucafe
Copy link
Contributor Author

Hey sorry just bumping this since maybe it got missed after holiday weekend. Thank you

@macneale4
Copy link
Contributor

Thanks for the bump. I'll take a look!

Copy link
Contributor

@macneale4 macneale4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Thanks for the contribution!

@macneale4 macneale4 merged commit 81b6b76 into dolthub:main Jul 8, 2025
56 of 62 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants