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

Fragmenta cmd should not ignore invalid arguments #27

Closed
GeertJohan opened this issue Apr 9, 2018 · 3 comments
Closed

Fragmenta cmd should not ignore invalid arguments #27

GeertJohan opened this issue Apr 9, 2018 · 3 comments

Comments

@GeertJohan
Copy link

GeertJohan commented Apr 9, 2018

It can be very confusing when fragmenta runs without an error even though there are invalid arguments.

I believe it would be beneficial to the cmd to start using a cli framework such as https://github.com/jawher/mow.cli

This will make development on the CLI easier and different developers working on fragmenta will have to adhere to the same CLI structure when proposing changes.
I would be happy to create a PR which converts the existing structure to mow.cli.

@kennygrant
Copy link
Contributor

I’m happy to consider suggestions for different behaviour (for example it should perhaps only run a server if it has zero arguments, not typos). I agree this should be fixed. I’ll leave this issue open for that. The fix would be relatively simple.

I don’t want to add another dependency though, and don’t think it would add much in this case. The aim is zero external dependencies.

@GeertJohan
Copy link
Author

GeertJohan commented Apr 12, 2018

Why would you not want to add dependencies? A package to handle arguments would add just that: properly handling arguments (commands and flags).

@kennygrant
Copy link
Contributor

kennygrant commented Apr 14, 2018

Why would you not want to add dependencies? A package to handle arguments would add just that: properly handling arguments (commands and flags).

Because a small dependency graph is a good thing. I don't think a large change is required here though to get the behaviour you want (which I agree with), it's a one line change, and I prefer the simpler code in the app at present to importing another dependency as I don't anticipate changing commands much. This is a small tool to help build, run and deploy apps and I'd like to keep the surface area as small as possible.

So on to the problem:

At present, fragmenta runs the server when run with no commands if you're in a valid project directory - I'd like to keep that as it's a useful shortcut which I use all the time, however it should not run the server when invalid arguments are given (the problem you have experienced). I've seen this myself a couple of times with a typo and would like to fix it.

So the simple fix is simply to check on line 127 if command == "" and only run the server in that case, happy to accept a pull request for that change.

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

No branches or pull requests

2 participants