Closed aik099 closed 9 years ago
Nice spotting. This might be achievable with less code:
diff --git a/src/CompletionHandler.php b/src/CompletionHandler.php
index 0925df0..cce8fb4 100644
--- a/src/CompletionHandler.php
+++ b/src/CompletionHandler.php
@@ -103,6 +103,10 @@ class CompletionHandler
try {
$this->command = $this->application->find($cmdName);
+
+ // Create a new instance of the command to avoid any changes made by Command::run
+ $commandClass = get_class($this->command);
+ $this->command = new $commandClass();
} catch (\InvalidArgumentException $e) {
// Exception thrown, when multiple or none commands are found.
}
I guess that doesn't cover cases where the constructor of a command has been changed though..
That might work, but the application won't be available from the command and we need to do ->setApplication
call too. And then it would look like code duplication. So using getNativeDefinition
is recommended way. I'll make createDefinition
method protected (it's private now because I copy-pased it from ListCommand
) and people then can change it to alter command definition.
Right now the
CompletionCommand
has no arguments, but there might be added in #30. In that case without this PR the auto-complete forapp _completion
argument won't work.In unit tests we're testing all parts separately, but we don't test by really invoking completion hook. That's why I don't know how do I test the changes I've made. All existing tests still pass.
Closes #46