search

Kdyby / Console

Symfony Console integration for Kdyby components
https://packagist.org/packages/kdyby/console
Other
52 stars 54 forks source link

Kdyby\Console requires app router to be a RouteList instance #37

Closed Andrewsville closed 8 years ago

Andrewsville commented 9 years ago

Recently we've run into the problem that Kdyby\Console is not happy with a router being an IRouter but insist it has to be a RouteList.

Unfortunatelly, in our case that is not technically possible. And even if it was (we can subclass the RouteList, right), the whole reason it has to be (to be able to prepend the CliRouter to the router list) wouldn't work anyway.

It is obvious that the whole reason for the CliRouter is that one is able to fire up the Nette app from the console just like from the browser and it redirects its requests to the Kdyby\Console\Application via the CliPresenter.

I have two possible solutions.

  1. Make the route juggling optional. Just like @JanTvrdik here (#30) we run the Kdyby\Console\Application directly when in CLI. So we don't need to modify the router in the first place.
  2. Make it more robust. Don't require the router to be a RouteList.
    • If it is, do your route juggling.
    • If it isn't, replace it with a RouteList instance, where the first route is your CliRouter and the second is the original IRoute implementation.

The second solution looks way better to me (although there is probably a cave eat - if other DI extensions modify the router too, the ConsoleExtension would have to be the last because this modification would (could?) not be done at run time but at DIC compile time. Anyway, I can prepare a pull request if you decide to go that way.

fprochazka commented 8 years ago

@Andrewsville is this still an issue after https://github.com/Kdyby/Console/pull/42 ?

fprochazka commented 8 years ago

If it is, please drop me a line here and we can discuss it further :)

Andrewsville commented 8 years ago

Yes, it is. #42 solves a somewhat related but different issue. #42 is about "not having to have nette/application as dependency". This issue is about "not having to have the application router an instance of RouteList but an IRouter".

fprochazka commented 8 years ago

Merged https://github.com/Kdyby/Console/commit/5fddef95acac0aaab992d56b0cf63c9b97dbd28e