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 CompletionAwareInterface commands to use/register Completion instances #32

Open
stecman opened this issue Jan 31, 2015 · 1 comment
Milestone

Comments

@stecman
Copy link
Owner

stecman commented Jan 31, 2015

Currently, CompletionAwareInterface can't cleanly use CompletionInterface implementations. (eg. deferring path completion to the shell with ShellPathCompletion). A simple solution might be another interface that exposes CompletionHandler to commands:

interface CompletionConfigurationInterface
{
    public function configureCompletion(CompletionHandler $handler);
}

In saying that, this makes me think that the responsibility for completion configuration (command name, argument/option, name) shouldn't really belong to CompletionInterface. It was a convenience when the code was originally created to have the API like that, but it doesn't entirely make sense and it means that completions require information they don't actually need to run.

@aik099
Copy link
Contributor

aik099 commented Jan 31, 2015

In other words we want to allow commands to register own completion handlers.

Would we require command name in the handlers registered that way to match current command name? What I'm afraid of is that somebody can register auto-complete in command A for command B and wonder why it doesn't work.

Won't it be too confusing for users whose commands implement both interfaces to decide in which method to put completions?

Approach with handler registration is better in the way, that it allows to proxied completion handlers to be registred, that will only calculate their completions when handler is invoked. However with a bunch of IF statements same can be done in completeOptionsValues/completeArgumentValues methods as well.

@stecman stecman added this to the 1.0 milestone Aug 30, 2015
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