Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: improve missing argument error messages #711

Merged
merged 9 commits into from
Apr 2, 2024
Merged

feat: improve missing argument error messages #711

merged 9 commits into from
Apr 2, 2024

Conversation

phm07
Copy link
Contributor

@phm07 phm07 commented Mar 8, 2024

This PR adds a custom Cobra command validator that outputs useful errors when positional arguments are missing during command execution. For example, such an error could look like this:

$ hcloud server describe   
hcloud server describe [options] <server>
                                  ^^^^^^
hcloud: expected argument server at position 1

Where previously it would look like this:

$ hcloud server describe
hcloud: accepts 1 arg(s), received 0

Additionally, if there are more arguments provided than necessary, the following error message will appear:

$ hcloud server describe arg1 arg2
hcloud server describe [options] <server>
                                          ^
hcloud: expected exactly 1 positional argument(s), but got 2

This is done by parsing the Use property of a Cobra command with a regular expression and then matching the corresponding missing arguments.

Fixes #700

@phm07 phm07 self-assigned this Mar 8, 2024
Copy link

codecov bot commented Mar 8, 2024

Codecov Report

Attention: Patch coverage is 55.38462% with 29 lines in your changes are missing coverage. Please review.

Project coverage is 58.48%. Comparing base (ab6ef0e) to head (103f542).

Files Patch % Lines
internal/cmd/base/labels.go 0.00% 4 Missing ⚠️
internal/cmd/util/validation.go 90.32% 2 Missing and 1 partial ⚠️
internal/cmd/base/set_rdns.go 0.00% 2 Missing ⚠️
internal/cmd/base/update.go 0.00% 2 Missing ⚠️
internal/cmd/all/all.go 0.00% 1 Missing ⚠️
internal/cmd/base/cmd.go 0.00% 1 Missing ⚠️
internal/cmd/certificate/certificate.go 0.00% 1 Missing ⚠️
internal/cmd/datacenter/datacenter.go 0.00% 1 Missing ⚠️
internal/cmd/firewall/firewall.go 0.00% 1 Missing ⚠️
internal/cmd/floatingip/floatingip.go 0.00% 1 Missing ⚠️
... and 12 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #711      +/-   ##
==========================================
- Coverage   58.77%   58.48%   -0.29%     
==========================================
  Files         179      180       +1     
  Lines        6539     6497      -42     
==========================================
- Hits         3843     3800      -43     
  Misses       2088     2088              
- Partials      608      609       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@phm07 phm07 marked this pull request as ready for review March 12, 2024 11:58
@phm07 phm07 requested a review from a team as a code owner March 12, 2024 11:58
@phm07 phm07 changed the base branch from main to docopt March 12, 2024 11:59
Copy link
Member

@apricote apricote left a comment

Choose a reason for hiding this comment

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

Looks good to me :) Can be merged once #709 is in main :)

Base automatically changed from docopt to main March 25, 2024 06:58
@phm07 phm07 merged commit e7f9e74 into main Apr 2, 2024
4 checks passed
@phm07 phm07 deleted the error-message branch April 2, 2024 10:33
phm07 pushed a commit that referenced this pull request Apr 3, 2024
🤖 I have created a release *beep* *boop*
---


##
[1.43.0](v1.42.0...v1.43.0)
(2024-04-03)


### Features

* allow deletion of multiple resources at once
([#719](#719))
([3b896fe](3b896fe))
* improve missing argument error messages
([#711](#711))
([e7f9e74](e7f9e74))
* **server:** allow JSON & YAML output in reset-password
([#716](#716))
([373287b](373287b)),
closes [#715](#715)


### Bug Fixes

* removing last rule from firewall fails with invalid_input error
([#696](#696))
([acab17c](acab17c))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
jooola added a commit that referenced this pull request Apr 11, 2024
jooola added a commit that referenced this pull request Apr 17, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve error message when required argument is not set
3 participants