Skip to content
New issue

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

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

Already on GitHub? Sign in 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

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
internal/cmd/util/validation.go Outdated Show resolved Hide resolved
internal/cmd/util/validation_test.go Outdated Show resolved Hide resolved
internal/cmd/util/validation_test.go Outdated Show resolved Hide resolved
internal/cmd/base/delete.go Outdated Show resolved Hide resolved
internal/cmd/util/validation.go Outdated Show resolved Hide resolved
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
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.

Improve error message when required argument is not set
3 participants