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

Allow commands to auto-complete their own option/argument values #20

Merged
merged 1 commit into from
Jan 29, 2015

Conversation

aik099
Copy link
Contributor

@aik099 aik099 commented Jan 25, 2015

Closes #19

@aik099
Copy link
Contributor Author

aik099 commented Jan 26, 2015

Maybe the initializeCompletion() method needs to be added to the interface to allow command to do a simplified initialization (e.g. open db connection or something like that), that is needed for completion to work.

@aik099 aik099 changed the title Allow commands to auto-complete their own option/argument values [WIP] Allow commands to auto-complete their own option/argument values Jan 27, 2015
@aik099 aik099 force-pushed the completion-aware-commands-feat branch from f201ecf to 3561920 Compare January 27, 2015 13:18
@@ -148,6 +173,10 @@ protected function createHandler($commandLine, $cursorIndex = null)
*/
protected function getTerms($handlerOutput)
{
if (null === $handlerOutput) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it's a bug in runCompletion method it in some cases (maybe when no matching completion handlers were found) it doesn't return anything which results in null.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's more of an inconsistency than a bug, but yeah if none of the completion attempts/modes return a value, CompletionHandler::filterResults is never called and execution reaches the end of the runCompletion method (here). CompletionHandler::runCompletion just needs return ''; added at the end to handle that case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should return empty array to have consistent output since it returns list of matched entries. If no completion handler was invoked, then 0 results which is equal to empty array.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CompletionHandler::filterResults and CompletionHandler::runCompletion currently return strings, but as the returned value ends up being passed to OutputInterface::write which supports an array, this can be changed to return an array without any functional difference (the tests will just need to be updated).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've missed that implode statement with \n. Then sure we need to return a string here.

I'll send another PR about that then.

@aik099 aik099 force-pushed the completion-aware-commands-feat branch from b912ee4 to 8048d3d Compare January 28, 2015 19:06
@aik099 aik099 changed the title [WIP] Allow commands to auto-complete their own option/argument values Allow commands to auto-complete their own option/argument values Jan 28, 2015
@aik099
Copy link
Contributor Author

aik099 commented Jan 28, 2015

Requested changes done and all commits squashed. Ready for merge.

@stecman stecman merged commit 8048d3d into stecman:master Jan 29, 2015
@stecman
Copy link
Owner

stecman commented Jan 29, 2015

Awesome, thanks @aik099.

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.

Allow commands to complete their option/argument values
2 participants