Skip to content

Conversation

@hranicka
Copy link
Contributor

See #198

I've disabled Session/Debugger dispatching in for CLI SAPI.

It's needed only for Ajax.

@hranicka hranicka mentioned this pull request Jun 20, 2016
@dg
Copy link
Member

dg commented Jun 20, 2016

Thanks!

@hranicka
Copy link
Contributor Author

This is the simplest possible solution, thanks @dg for your mentoring.

@JanTvrdik
Copy link
Contributor

JanTvrdik commented Jun 20, 2016

PHP_SAPI is not part of DI container cache key. The condition should IMHO be evaluated in runtime.

@hranicka
Copy link
Contributor Author

hranicka commented Jun 20, 2016

@dg
Copy link
Member

dg commented Jun 20, 2016

It is "coincidentally" part of cache key, the question is whether make it official part of key, or modify implementation of extensions or leave it as is.

@hranicka
Copy link
Contributor Author

hranicka commented Jun 20, 2016

There is also a parameter consoleMode here.

Does it have any impact on it? It's a part part of DIC cache key.


I can change this PR something like:

$initialize->addBody('if (PHP_SAPI !== 'cli' && $tmp = $this->getByType("Nette\Http\Session", FALSE)) { $tmp->start(); Tracy\Debugger::dispatch(); };');

But it seems unclear to me, I'm not sure with it.

@JanTvrdik
Copy link
Contributor

@dg I think that it should be possible to generate DI container for application from CLI.

@dg
Copy link
Member

dg commented Jun 20, 2016

It would actually be parameter for TracyExtension, like $debugMode .

@dg
Copy link
Member

dg commented Jun 20, 2016

@hranicka can you change it this way? nette/http@27c3de5#diff-e18d2abacdd85c4cc65b1a8d611ca31f

@hranicka
Copy link
Contributor Author

hranicka commented Jun 20, 2016

Ok, I edit it.

I've tried use config parameters with %consoleMode% but I must add more code because of this magic.

Constructor parameters seems better.


And what about Nette\Http packages? There is also PHP_SAPI constant. Ah, you edited this right now. Thanks :)

@hranicka hranicka force-pushed the hotfix/session-cli branch from 3bf9b11 to 916acb3 Compare June 20, 2016 10:36
@dg
Copy link
Member

dg commented Jun 20, 2016

👍

@dg dg merged commit 6f89b71 into nette:master Jun 20, 2016
@hranicka
Copy link
Contributor Author

I can prepare you PR into nette/bootstrap for your changes in nette/http and tracy. Give me a few minutes.

@dg
Copy link
Member

dg commented Jun 20, 2016

I have it finished.

@hranicka
Copy link
Contributor Author

Ok, thanks :)

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.

3 participants