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

Improve validation of command-line options #203

Closed
mjordan opened this issue Apr 12, 2016 · 10 comments
Closed

Improve validation of command-line options #203

mjordan opened this issue Apr 12, 2016 · 10 comments
Assignees
Milestone

Comments

@mjordan
Copy link
Collaborator

mjordan commented Apr 12, 2016

Currently, there is little feedback from MIK if the user misspells an .ini file, or worse, misspells 'limit' (in which case MIK starts up and doesn't apply a limit).

We could perform better option validation, or incorporate a CLI library like https://github.com/nategood/commando/ that does this automatically. We'd want a CLI library that didn't interfere with our class loading or other code-level logic. We should check out the Symfony Console Component but I wouldn't want us to be forced to adopt the entire Symfony ecosystem. It's awesome but overkill for MIK.

@mjordan mjordan added this to the 1.0 milestone Apr 12, 2016
@MarcusBarnes
Copy link
Owner

👍

@mjordan
Copy link
Collaborator Author

mjordan commented Oct 21, 2016

I've integrated the Commando component (linked above) into mik and it appears to work as intended, on Linux at least (haven't tested yet on Windows). So, now, ./mik --help produces:

 ./mik                                                                                                                    


--config/-c <argument>
     Required. Path to MIK configuration file.


--checkconfig/--cc <argument>
     Applies checks to configuration, does not process objects. Must be one of 'snippets', 'urls', 'paths', 'aliases',
     'input_directories', or 'all'.


--help
     Show the help page for this command.


--limit/-l <argument>
     Number of objects to process.

We get aliases, so that ./mik --c tutorial_config.ini and ./mik --config tutorial_config.ini are equivalent.

And, most importantly, we get option validation, so that ./mik --config tutorial_config.ini --limil 10 produces:

ERROR: Unknown option, limil, specified

instead of running without a limit. Also, the value of --limit is validated to be an integer.

Only drawback I can see is that Commando doesn't support the = in options, which means that MIK's basic command syntax changes. However, I would argue that using Commando results in a safer, much more conventional command-line user interface than what we had before.

mjordan added a commit that referenced this issue Oct 21, 2016
@mjordan mjordan self-assigned this Oct 21, 2016
@mjordan
Copy link
Collaborator Author

mjordan commented Oct 21, 2016

To test, switch to the issue-203 branch, then run composer update, since we are adding

"require": {
    "nategood/commando": "*"
}

to composer.json.

@mjordan
Copy link
Collaborator Author

mjordan commented Oct 21, 2016

AFAICT, only README.md and the tutorial (including screenshots) would need to be updated to reflect the change in command-line syntax.

@MarcusBarnes
Copy link
Owner

@mjordan Thanks. I will schedule some time to test.

@mjordan
Copy link
Collaborator Author

mjordan commented Oct 21, 2016

Excellent thanks. Just did a smoke test on Windows and everything appears to work as expected.

@MarcusBarnes
Copy link
Owner

@mjordan Passed my initial basic tests.

@mjordan
Copy link
Collaborator Author

mjordan commented Oct 21, 2016

As discussed offline, I've tested this pretty thoroughly so I'll merge into master.

@mjordan mjordan closed this as completed Oct 21, 2016
@mjordan mjordan reopened this Oct 21, 2016
@mjordan
Copy link
Collaborator Author

mjordan commented Oct 21, 2016

Merged with 72b5d88; reopening because updating tutorial is yet to be done.

@mjordan
Copy link
Collaborator Author

mjordan commented Oct 30, 2016

Tutorial has been updated to show no = in MIK commands. Closing.

@mjordan mjordan closed this as completed Oct 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants