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

Check for cli environment #196

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

cewlbird
Copy link

Hi,

first of all thank your for this great package!

As I am using your package for creating users with some cli scripts I always got the "Headers already sent" error as soon as I echoed out some informations inside my cli script.

So I added a quick environment check by checking for the current SAPI_NAME.
Maybe this will help some other users as well.

Best Regards,

CewlBird

@ocram
Copy link
Contributor

ocram commented Jan 20, 2020

Thanks!

Not sure if this solution is really the solution we need.

For example, you could simply change the structure of your application code and change the sequence of calls, and the problem should disappear. That’s because you’d have the same problem on the web: You simply have create the instance before sending your output. Or use output buffering.

Wouldn’t you agree?

@cewlbird
Copy link
Author

Hi, sorry for the late reply.

I understand what you mean, but I don't know if this will work in all cases, as I have some cli outputs before I have created the instance, like prompts or some other stuff that gets written to the console.

I tried output buffering before but it didn't work as expected.

At least in my case changing the sequence of calls is not a solution, as the current app order works properly in the web but my cli scripts will always output some stuff before this class instantiated.

So this was my way to solve this use case.

I shared this snippet just in case anyone had the same use cases like me, so feel free to close this pull request or to merge it someday in the future if some other people need this kind of solution.

Best Regards,

CewlBird

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants