Skip to content

[Process] Create a "isTtySupported" static method #25142

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

Merged
merged 1 commit into from
Dec 7, 2017

Conversation

nesk
Copy link
Contributor

@nesk nesk commented Nov 24, 2017

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

Currently, there is no way to enable the TTY mode without risking an exception. This PR extracts the code checking for TTY support and provides it in a isTtySupported static method.

Now we can enable the TTY mode everywhere it's available without risking an exception:

$process = (new Process)->setTty(Process::isTtySupported());

Old comment:

I'm targeting the 2.7 branch since this is not really a new feature, just a little refactoring of the existent code, and it is fully backward compatible.

@stof
Copy link
Member

stof commented Nov 24, 2017

Adding a new public method is a new feature. So this cannot go in 2.7

@nesk
Copy link
Contributor Author

nesk commented Nov 24, 2017

OK! Would you like me to create a new PR for the master branch?

@chalasr
Copy link
Member

chalasr commented Nov 24, 2017

You can change the target branch on this PR and rebase accordingly.

@chalasr chalasr added this to the 4.1 milestone Nov 24, 2017
@nesk nesk changed the base branch from 2.7 to master November 24, 2017 12:07
@nesk nesk force-pushed the is-tty-supported-method branch from c3b9b13 to a1398f6 Compare November 24, 2017 12:08
@nesk
Copy link
Contributor Author

nesk commented Nov 24, 2017

The target branch has been changed and the rebase too.

@fabpot
Copy link
Member

fabpot commented Dec 7, 2017

Thank you @nesk.

@fabpot fabpot merged commit a1398f6 into symfony:master Dec 7, 2017
fabpot added a commit that referenced this pull request Dec 7, 2017
This PR was merged into the 4.1-dev branch.

Discussion
----------

[Process] Create a "isTtySupported" static method

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

Currently, there is no way to enable the TTY mode without risking an exception. This PR extracts the code checking for TTY support and provides it in a `isTtySupported` static method.

Now we can enable the TTY mode everywhere it's available without risking an exception:

```php
$process = (new Process)->setTty(Process::isTtySupported());
```

_Old comment_:
> I'm targeting the 2.7 branch since this is not really a new feature, just a little refactoring of the existent code, and it is fully backward compatible.

Commits
-------

a1398f6 Create a "isTtySupported" static method
@fabpot fabpot mentioned this pull request May 7, 2018
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.

5 participants