Skip to content

[Console] default to stderr if available #13730

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

Closed
wants to merge 1 commit into from
Closed

[Console] default to stderr if available #13730

wants to merge 1 commit into from

Conversation

alcohol
Copy link
Contributor

@alcohol alcohol commented Feb 18, 2015

[Console]

Interactive input/output and informational output such as progress should go to stderr if available.

Q A
Bug fix? yes
New feature? no
BC breaks? possibly / unlikely ?
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT
Doc PR

/cc @Seldaek @dzuelke

@kingcrunch
Copy link
Contributor

I'd like to disagree, because that aren't errors.

"Bug fix? Yes" -- which one? 😕

@alcohol
Copy link
Contributor Author

alcohol commented Feb 18, 2015

stderr is not only for errors, though the name can be misleading for the uninformed. Unfortunately there isn't a whole lot of reading material readily available on the subject. But can I suggest you read this and then let me know if you still think this PR is bad?

@kingcrunch
Copy link
Contributor

Primary I don't understand, what this PR solves. If you want to redirect the output you are usually not interested in a progress bar and interaction at all and so I'd expect --no-interaction-/--no-progress-flags.

Also I think that some tools (in my opinion) misuse stderr for something else than errors doesn't make it right. In some seldom cases there is no way around [1] and that's fine. But I don't think that's the main use-case.
I'll read your link and we'll see what I'll think afterwards :)

[1] For example curl, which by default uses stdout for the response and one usually redirects this into a file. Of course you don't want to keep this in your file.

@dzuelke
Copy link
Contributor

dzuelke commented Feb 19, 2015

Also I think that some tools (in my opinion) misuse stderr for something else than errors doesn't make it right. In some seldom cases there is no way around [1] and that's fine. But I don't think that's the main use-case.

Take this for instance:

$ curl --user Joe http://someprotectedsite.com/file.tar.gz > erp.tar.gz

cURL will now (correctly) prompt for the password on stderr, which makes total sense.

The Unix philosophy where you have specialized, simple command line tools is about building pipelines. You can only build pipelines if your stdout output is predictable and focused or else the entire thing won't be very powerful.

@fabpot fabpot added the Console label Feb 25, 2015
@pjcdawkins
Copy link
Contributor

What about $output->write() and $output->writeln()?

Should there also be writeInfo() or writeError() (or log()) methods on OutputInterface, which default to stderr?

@alcohol
Copy link
Contributor Author

alcohol commented May 26, 2015

While it might be convenient, I do think it should be up to the user to use the proper output (stdout vs. stderr) when calling write() and such. No need to add more helpers here imho.

@jenkoian
Copy link
Contributor

Chris Fidao explains this quite well also http://fideloper.com/laravel-symfony-console-commands-stderr

@alcohol
Copy link
Contributor Author

alcohol commented May 26, 2015

Thanks, nice link :-).

@Seldaek
Copy link
Member

Seldaek commented May 26, 2015

👍

@Tobion
Copy link
Contributor

Tobion commented May 26, 2015

I wouldn't change this in 2.3 but only in 2.7/2.8 as it might have side effects. Also in later version there are alot more classes that would need to be adjusted as well like the ProgressBar class to be consistent.

Also I feel this is the developers task to pass the correct output to these classes. It's not the responsibility of the classes to magically use a different output. It goes against the single responsibility principle.
What if somebody actually wants to use the stdout? It would magically change it. And who determines which helper is considered pertinent and which not?

@alcohol
Copy link
Contributor Author

alcohol commented May 26, 2015

What if somebody actually wants to use the stdout? It would magically change it.

Aye, there is something to be said for that as well.

I think it would help convention if stderr was the default, but it would certainly be nice if this could still be switched out for stdout instead. Not sure how to resolve that approach properly though. Thoughts?

@Tobion
Copy link
Contributor

Tobion commented Sep 14, 2015

Many people do not know the history of unix file descriptors and best practices, so it is not clear that stderr should be used for informational messages. Considering that, we might want to reconsider this PR.
My previous argument was that it makes it impossible for people to actually use stdout if they want to. But there is a workaround: In this edge case they can just create a new StreamOutput(STDOUT) and pass that to the helpers instead of the ConsoleOutput.

@Seldaek what do you think?

@Seldaek
Copy link
Member

Seldaek commented Sep 14, 2015

@Tobion well, IMO there are literally no reasons to ever write a question or interactive thing on STDOUT, so I don't really see it as a problem either way to default to the right thing (STDERR), and yes if you really want to force it to be STDOUT you can still do it as you say.

@Tobion
Copy link
Contributor

Tobion commented Sep 14, 2015

@alcohol can you create a new PR please?

@alcohol
Copy link
Contributor Author

alcohol commented Sep 14, 2015

With the same approach? (sorry I was not following the latest additions to the discussion)

@Seldaek
Copy link
Member

Seldaek commented Sep 14, 2015

This one could just be reopened/merged no?

@Tobion
Copy link
Contributor

Tobion commented Sep 14, 2015

Yes same approach. Cannot be reopened because repo was deleted.

@alcohol
Copy link
Contributor Author

alcohol commented Sep 14, 2015

I don't have my fork anymore. So no...

@lyrixx
Copy link
Member

lyrixx commented Sep 14, 2015

@alcohol
Copy link
Contributor Author

alcohol commented Sep 14, 2015

Do you still want me to base this off of 2.3 though?

fabpot added a commit that referenced this pull request Mar 4, 2016
This PR was squashed before being merged into the 2.3 branch (closes #15794).

Discussion
----------

[Console] default to stderr in the console helpers

Interactive input/output and informational output such as progress should go to `stderr` if available.

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        |

See #13730 also for previous discussion.

If someone explicitly wants to use `stdout`, they can simply pass `$output->getStream()` instead of `$output` in most use-cases.

Commits
-------

3d4e95e [Console] default to stderr in the console helpers
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants