slimphp / Slim-Console

Slim Framework Console
https://slimframework.com
MIT License
28 stars 12 forks source link

[PoC] Auto-register internal commands #11

Closed RyanNerd closed 4 years ago

RyanNerd commented 4 years ago

DO NOT MERGE Proof of Concept for illustration and discussion purposes only. Please comment on if this is a good method to register internal commands:

Any xxxxCommand.php located in src/Command will be auto-registered when the application initializes.

There are two commands (class files) slim echo <echo-string> and slim hello-world <username> to test this concept.

If this not a methodology we want to use please close this PR.

l0gicgate commented 4 years ago

I think that we should probably take a different approach to detecting commands. Since we have a preset commandsDir from the config file, we can iterate through the files within that directory, determine if they're a class that implements the base Command class and then add it to our Application.

<?php

use Slim\Console\Application;
use Slim\Console\Config;
use Symfony\Component\Console\Command\Command;

$app = new Application(); // Implicitly loaded through bootstrapping
$config = new Config(); // Just for example sake

function isCommandClass(string $className): bool
{
    $class = new ReflectionClass($className);
    return $class->implementsInterface(Command::class);
}

$commands = [];
foreach (glob($config->getCommandsDir() . '/*.php') as $file) {
    $filePath = $config->getCommandsDir() . '/'. $file;
    $contents = file_get_contents($filePath);
    $tokens = token_get_all($contents, TOKEN_PARSE);
    foreach ($tokens as $token) {
        $normalizedToken = is_array($token) ? $token[0] : $token;
        if (token_name($normalizedToken) === T_CLASS) {
           require_once $filePath;
           if (isCommandClass($normalizedToken)) {
               $commands[] = new $normalizedToken();
           }
        }
    }
}

$app->addCommands($commands);

This is untested code obviously but I think this would be a more sound way of approaching the problem. We could wrap this in a class obviously, I'm just laying it out for PoC sake.

RyanNerd commented 4 years ago

@l0gicgate I like your approach better. Should we leave this PR open for now and reference it in the discussion threads?

l0gicgate commented 4 years ago

@RyanNerd absolutely, let's keep it open for now.

@ABGEO07 and @flangofas feel free to chime in on this as well!

RyanNerd commented 4 years ago

Unless there's an objection I'm going to close this PR as it appears this has been implemented already. I'll wait 2 days for any reply then close it. Note: Sorry I've not been participating in this much. My regular work unfortunately got busy and that's what pays my bills. :smiley: