8000 [Console] Make getTerminalWith & getTerminalHeight public by egeloen · Pull Request #6571 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Console] Make getTerminalWith & getTerminalHeight public #6571

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 5, 2013
Merged

[Console] Make getTerminalWith & getTerminalHeight public #6571

merged 1 commit into from
Jan 5, 2013

Conversation

egeloen
Copy link
@egeloen egeloen commented Jan 5, 2013

Bug fix: yes
Feature addition: yes
Backwards compatibility break: no
Fixes the following tickets: ~
Todo: -
License of the code: MIT
Documentation PR: ~

After demand in #6567, I open an other one on the 2.0 branch.

fabpot added a commit that referenced this pull request Jan 5, 2013
@fabpot
merged branch egeloen/f-2.0-terminal-width (PR #6571)
5157693 
This PR was merged into the 2.0 branch.

Commits
-------

f299bd0 [Console] Make getTerminalWith & getTerminalHeight public

Discussion
----------

[Console] Make getTerminalWith & getTerminalHeight public

Bug fix: yes
Feature addition: yes
Backwards compatibility break: no
Fixes the following tickets: ~
Todo: -
License of the code: MIT
Documentation PR: ~

After demand in #6567, I open an other one on the 2.0 branch.
@fabpot fabpot merged commit f299bd0 into symfony:2.0 Jan 5, 2013
@egeloen egeloen deleted the f-2.0-terminal-width branch January 5, 2013 13:39
@everzet
Copy link
Contributor
everzet commented Jan 5, 2013

Actually IT IS an effective API BC break. If you have extended an Application class before and overrided any of those methods in your app - you'll have a Fatal error in 2.2 & horrible BC break preventing you from overriding this method and providing support for both 2.0-2.1 and 2.2+ in your app. And I still don't see the reason why.

ping @fabpot

@everzet everzet mentioned this pull request Jan 5, 2013
Closed
@everzet
Copy link
Contributor
everzet commented Jan 5, 2013

I mean, I can see that you might need this information, but introducing BC break just in sake of getting it sounds bad to me.

@everzet
Copy link
Contributor
everzet commented Jan 5, 2013

Oh my, it's even in 2.0 :/

@everzet
Copy link
Contributor
everzet commented Jan 5, 2013

@fabpot IMO, we need to at least revert this PR for versions prior 2.2 and either provide another way to get this information in user-space or do apply this BC break for 2.2+ only (and I still don't think it's a good thing to do).

@fabpot
Copy link
Member
fabpot commented Jan 5, 2013

reverted

fabpot added a commit that referenced this pull request Jan 5, 2013
@fabpot
Revert "merged branch egeloen/f-2.0-terminal-width (PR #6571)"
151f2c1 
This reverts commit 5157693, reversing
changes made to cd0a9d7.
@egeloen
Copy link
Author
egeloen commented Jan 5, 2013

@fabpot I agree it is a BC break but now, how do I determine the terminal width in a command (#6550)?

fabpot added a commit that referenced this pull request Jan 5, 2013
@fabpot
Merge branch '2.0' into 2.1
5699a34 
* 2.0:
  Revert "merged branch egeloen/f-2.0-terminal-width (PR #6571)"
@fabpot
Copy link
Member
fabpot commented Jan 5, 2013

As of 2.1, you can use Application::getTerminalDimensions() instead, which is public now and gives you the same information.

@everzet
Copy link
Contributor
everzet commented Jan 5, 2013

@fabpot just wanted to propose introducing this method :D Didn't know it's already there since 2.1. I think that's it then.

fabpot added a commit that referenced this pull request Jan 5, 2013
@fabpot
Merge branch '2.1'
583d999 
* 2.1:
  [Console] made Application::getTerminalDimensions() public
  Revert "merged branch egeloen/f-2.0-terminal-width (PR #6571)"
  [2.1] [Console] Added getTerminalDimensions() with fix for osx/freebsd
  Restrict Monolog version to be in version `<1.3`. Because of conflict between `HttpKernel/Log/LoggerInterface` and `Psr\Log\LoggerInterface` (PSR-3)

Conflicts:
	composer.json
@everzet
Copy link
Contributor
everzet commented Jan 5, 2013

@fabpot thanks for a quick answer, btw ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@egeloen @everzet @fabpot @GeLoLabs
0