Skip to content

Fixed the line length of the new Symfony Styles #14136

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 3 commits into from

Conversation

javiereguiluz
Copy link
Member

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

After/Before screenshots

example_1

example_2

example_3

And now my question: is there any way to get the terminal dimensions from SymfonyStyles class without having to copy/paste the code found on \Console\Application?

}
}

return;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this has to be removed to comply with symfony CS

@stof
Copy link
Member

stof commented Mar 31, 2015

@javiereguiluz the class should have a way to get them from the Application IMO

@javiereguiluz
Copy link
Member Author

@stof I agree with you, but I don't know how to do it 😢

@1ed
Copy link
Contributor

1ed commented Apr 6, 2015

@javiereguiluz
Copy link
Member Author

@1ed that's a nice trick! Thanks for the reference. I've just updated the code as of 70a1f80

@javiereguiluz
Copy link
Member Author

The errors reported by Travis seems to be caused by volatile tests (as reported by @stof in #14078):

1) Symfony\Component\Process\Tests\SigchildDisabledProcessTest::testIdleTimeoutNotExceededWhenOutputIsSent
Failed asserting that false is true.
/home/travis/build/symfony/symfony/src/Symfony/Component/Process/Tests/AbstractProcessTest.php:854

2) Symfony\Component\Process\Tests\SimpleProcessTest::testIdleTimeoutNotExceededWhenOutputIsSent
Failed asserting that false is true.
/home/travis/build/symfony/symfony/src/Symfony/Component/Process/Tests/AbstractProcessTest.php:854

3) Symfony\Component\Stopwatch\Tests\StopwatchTest::testStop
Failed asserting that 229 matches expected 200.
/home/travis/build/symfony/symfony/src/Symfony/Component/Stopwatch/Tests/StopwatchTest.php:79

@fabpot
Copy link
Member

fabpot commented Apr 7, 2015

Thank you @javiereguiluz.

@fabpot fabpot closed this Apr 7, 2015
fabpot added a commit that referenced this pull request Apr 7, 2015
…iluz)

This PR was squashed before being merged into the 2.7 branch (closes #14136).

Discussion
----------

Fixed the line length of the new Symfony Styles

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

**After/Before screenshots**

![example_1](https://cloud.githubusercontent.com/assets/73419/6919599/be6b2cc4-d7b8-11e4-99dc-0848a3cb94cf.png)

![example_2](https://cloud.githubusercontent.com/assets/73419/6919603/c031478c-d7b8-11e4-9887-13dcd8635434.png)

![example_3](https://cloud.githubusercontent.com/assets/73419/6919606/c2498ab6-d7b8-11e4-89ef-1e0e5db9425a.png)

And now my question: is there any way to get the terminal dimensions from SymfonyStyles class without having to copy/paste the code found on \Console\Application?

Commits
-------

e9c7912 Fixed the line length of the new Symfony Styles
@ogizanagi
Copy link
Contributor

@javiereguiluz : Now that this PR is merged and #14032 too, Travis reports the following error :

  1. Symfony\Bundle\SecurityBundle\Tests\Functional\UserPasswordEncoderCommandTest::testEncodePasswordEmptySalt
    str_repeat(): Second argument has to be greater than or equal to 0
    ...
    /home/travis/build/symfony/symfony/src/Symfony/Bundle/SecurityBundle/vendor/symfony/console/Symfony/Component/Console/Style/SymfonyStyle.php:88
    /home/travis/build/symfony/symfony/src/Symfony/Bundle/SecurityBundle/vendor/symfony/console/Symfony/Component/Console/Style/SymfonyStyle.php:149
    ...

I think this might be related to this PR ?
See #14032 (comment)

@1ed
Copy link
Contributor

1ed commented Apr 7, 2015

See #14250

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.

7 participants