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

Implement better validation of how CLI arguments are used together #1788

Closed
florelis opened this issue Dec 10, 2021 · 3 comments
Closed

Implement better validation of how CLI arguments are used together #1788

florelis opened this issue Dec 10, 2021 · 3 comments
Labels
Issue-Feature This is a feature request for the Windows Package Manager client.
Milestone

Comments

@florelis
Copy link
Member

Description of the new feature / enhancement

The current validation of arguments seems to miss many combinations of arguments that don't make sense together. For example, the upgrade commands allows winget upgrade --all -v 1.0 (version is used only when upgrading single package, not all of them) or winget upgrade -m myManifest.yaml --accept-source-agreements (sources are not used with local manifest, so there are no agreements).

We should improve that validation, preferably in a way that makes it easier to extend as we add more arguments.

Proposed technical implementation details

I think we could create "categories" of arguments and define the rules on top of them. For example, for upgrade there are arguments for selecting the package (e.g. id, version), arguments for selecting multiple packages (--all, --include-unknown), and arguments for the sources (source name, REST source header, agreements). We could define rules on top of them like disallowing arguments for both single and multiple packages, or for sources when including a local manifest. Adding a new argument to one category would include it in all existing rules easily.

@florelis florelis added the Issue-Feature This is a feature request for the Windows Package Manager client. label Dec 10, 2021
@ghost ghost added the Needs-Triage Issue need to be triaged label Dec 10, 2021
@denelon denelon removed the Needs-Triage Issue need to be triaged label Dec 10, 2021
@JohnMcPMS
Copy link
Member

Indeed, Powershell's parameter sets was something I had considered when this was originally written. At the time it wasn't really worth it, but I assumed we could just add it later. I still think we could do so easily.

I would argue for keeping them separate as in sets, rather than tagging each argument with flags. That way, each set can have custom documentation (if needed) for the argument.

@Trenly
Copy link
Contributor

Trenly commented May 3, 2023

@florelis - Is this resolved with the above PR?

@florelis
Copy link
Member Author

florelis commented May 3, 2023

@florelis Is this resolved with the above PR?

I think the PR does what I wanted so I'll close this. Maybe in the future we'll want to do what @JohnMcPMS suggested about sets, but that can be a different issue.

@florelis florelis closed this as completed May 3, 2023
@denelon denelon added this to the v1.6 Client milestone Aug 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Feature This is a feature request for the Windows Package Manager client.
Projects
None yet
Development

No branches or pull requests

4 participants