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

Validate CLI flags #647

Closed
mrazauskas opened this issue Aug 4, 2024 · 5 comments · Fixed by #699
Closed

Validate CLI flags #647

mrazauskas opened this issue Aug 4, 2024 · 5 comments · Fixed by #699

Comments

@mrazauskas
Copy link
Contributor

mrazauskas commented Aug 4, 2024

Thanks for CLI improvements!

Here is another idea. I have noticed that it is too easy to make a mistake like this: poku --failFast. The failFast is primarily documented using camel case (look at TOC, or heading of the page). Or someone (like me) might be used to run tests with --failFast.

It would be useful to see some error like: Unknown flag --failFast.

Edit: And another mistake I just made: poku --paralel.


Support for camel case flags is another feature, but I will mention it here too. Interestingly tsc supports only camel case flags. That simplifies documentation of compiler config options and CLI flags.

@wellwelwel
Copy link
Owner

Hey, @mrazauskas 🙋🏻‍♂️

I'd have no problem patterning the camelCase, but it would be more feasible in a future major version (v3).

As for validation, it's a good idea.

@mrazauskas
Copy link
Contributor Author

Sure. This issue is about validation only.

The rest is totally up to you. Since that is opinionated and also depends on how you see Poku's future.

@mrazauskas
Copy link
Contributor Author

I understand this is not a priority right now. Just to draw you attention: not only flag names, but also their values need validation; the same is applicable for config file options.

For example, imagine some option being removed / renamed / changed its behaviour.

Perhaps it is worth to keep this issue open?

@wellwelwel
Copy link
Owner

wellwelwel commented Aug 20, 2024

In a way, this functionality would frequently imply breaking changes. So, to have more flexibility, I released it as "Early Development" and documented some plans for the enforce option:

- [x] Forces an error if any _CLI_ flags are invalid.
- [ ] Forces an error if no file is found _(soon)_.
- [ ] Forces an error if the environment has multiple platforms and the `--platform` option isn't explicit _(soon)_.

values need validation; the same is applicable for config file options

  • More ideas are welcome 🙋🏻‍♂️

Also:

:::info
This feature will be included in the configuration file when the development stage is stable.
:::


Perhaps it is worth to keep this issue open?

For now, I'm using the documentation itself as if it were an issue (with the idea of emphasizing that this feature is still under development). Honest question, do you think this is a bad idea?

@wellwelwel
Copy link
Owner

wellwelwel commented Aug 20, 2024

For example, imagine some option being removed / renamed / changed its behaviour.

About this, I tend to take versioning pretty carefully, for example, to make it easier to migrate on camel case, I've allowed both kebab and camel cases until a new major version is actually released.

Despite that, the intention is for the enforce to notify depreciated options. For example, even allowing fast-fail, when using enforce it will throw an error because it expects for failFast.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants