NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.8k

[Console] Support formatted text cutting#22225

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

chalasr merged 1 commit intosymfony:masterfromro0NL:console/cut-format

Sep 4, 2018

Merged

[Console] Support formatted text cutting#22225

chalasr merged 1 commit intosymfony:masterfromro0NL:console/cut-format

Sep 4, 2018

Conversation

Copy link

Contributor

ro0NL commentedMar 30, 2017•
edited
Loading

Q	A
Branch?	master
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes/no
Fixed tickets	#...
License	MIT
Doc PR	symfony/symfony-docs#...

allows cutting a formatted text to a certain width. Actually needed if we want to support max. column widths in tables (see#22156)

$text ='pre <error>foo bar baz</error> post';dump('BEFORE');$output->writeln(wordwrap($output->getFormatter()->format($text),3,"\n",true), OutputInterface::OUTPUT_RAW);dump('AFTER');$output->writeln($output->getFormatter()->format($text,3), OutputInterface::OUTPUT_RAW);

carsonbot added Status: Needs Review Console Feature labels

Mar 30, 2017

nicolas-grekas reviewed

Apr 3, 2017

View reviewed changes

src/Symfony/Component/Console/Formatter/OutputFormatter.php

		* Sets the decorated flag.
		*
		* @param bool $decorated Whether to decorate the messages or not
		* {@inheritdoc}

Copy link

Member

Choose a reason for hiding this comment

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

does this patch apply to lower branches? if yes, should be done there

Copy link

ContributorAuthor

Choose a reason for hiding this comment

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

Will look into that.

nicolas-grekas requested changes

Apr 3, 2017

View reviewed changes

src/Symfony/Component/Console/Formatter/OutputFormatter.php

		* @param string $message The message to style
		*
		* @return string The styled message
		* {@inheritdoc}

Copy link

Member

Choose a reason for hiding this comment

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

that one is not possible: $width is not in the interface, thus should be documented here

src/Symfony/Component/Console/Formatter/OutputFormatterInterface.php Outdated

		* @return string The styled message
		*/
		publicfunctionformat($message);
		publicfunctionformat($message/, $width = 0/);

Copy link

Member

nicolas-grekasApr 3, 2017•
edited
Loading

Choose a reason for hiding this comment

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

Changing an interface like that is a hard BC break. Anyone implementing it without extendingOutputFormatter will get no deprecation warning.

Copy link

ContributorAuthor

Choose a reason for hiding this comment

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

Any suggestions? Took this approach fromLogoutUrlGenerator::registerListener ;) which is not bound to an interface.

Note that this way is the easiest implementation and avoids double processing the formatted text, which is nice.

nicolas-grekas added this to the3.4 milestone

Apr 3, 2017

ro0NL mentioned this pull request

Apr 7, 2017

[Console] Inherit phpdoc from OutputFormatterInterface#22342

Merged

Copy link

ContributorAuthor

ro0NL commentedApr 7, 2017•
edited
Loading

Approach updated.. not sure this makes sense either :P but im concerning users extending fromOutputFormatter as well as implementingOutputFormatterInterface.

Guess i like theformat($message, $width = 0) api.. but maybe the new interface should simply definewrapAndFormat($message, $width) or so.. and letformat() forward.

fabpot added a commit that referenced this pull request

Apr 10, 2017

minor#22342[Console] Inherit phpdoc from OutputFormatterInterface (…

f8e1098

…ro0NL)This PR was merged into the 2.7 branch.Discussion----------[Console] Inherit phpdoc from OutputFormatterInterface| Q             | A| ------------- | ---| Branch?       | 2.7| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets |#22225 (comment)| License       | MIT| Doc PR        | symfony/symfony-docs#... <!--highly recommended for new features-->Commits-------ff3cb9c [Console] Inherit phpdoc from OutputFormatterInterface

ro0NL mentioned this pull request

Apr 28, 2017

Symfony Console Exception reporting doesn't parse style tags.#22552

Closed

nicolas-grekas self-assigned this

Aug 31, 2017

nicolas-grekas removed their assignment

Sep 16, 2017

Copy link

Member

nicolas-grekas commentedOct 8, 2017

Moving to 4.1. Rebase on master might be needed, where PHP 7.1 features can be used btw.

nicolas-grekas modified the milestones:3.4,4.1

Oct 8, 2017

Copy link

Member

fabpot commentedMar 31, 2018

@ro0NL Can you tell us the status of this PR? Looks like it should be rebased.

Copy link

ContributorAuthor

ro0NL commentedMar 31, 2018•
edited
Loading

@fabpot is the current upgrade path OK? If so, suggestions for namingWrappableOutputFormatterInterface welcome :)

but i was hoping we eventually dont need this interface and haveOutputInterface::format($message, $width = 0) API... but the current BC policy doesnt allow that right? Hence we're stuck with this interface.

status: needs work

carsonbot added Status: Needs Work and removed Status: Needs Review labels

Mar 31, 2018

fabpot requested changes

Apr 4, 2018

View reviewed changes

src/Symfony/Component/Console/Formatter/OutputFormatter.php Outdated

		if (__CLASS__ !==get_class($this)) {
		$r =new \ReflectionMethod($this,__FUNCTION__);
		if (__CLASS__ !==$r->getDeclaringClass()->getName()) {
		@trigger_error(sprintf('Method "%s()" will have a 2nd `$width = 0` argument in version 5.0. Not defining it is deprecated since 4.1.',__METHOD__),E_USER_DEPRECATED);

Copy link

Member

fabpotApr 4, 2018

Choose a reason for hiding this comment

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

... in version 5.0, not defining it is deprecated since Symfony 4.1

Copy link

Member

fabpotApr 4, 2018

Choose a reason for hiding this comment

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

I don't understand why you want to force people to define the$width. Not setting it explicitly seems fine to me.

Copy link

ContributorAuthor

ro0NLApr 4, 2018

Choose a reason for hiding this comment

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

Right. I should check for defined no. of arguments on the signature.

Copy link

ContributorAuthor

ro0NL commentedApr 4, 2018

I think the upgrade path is OK now. But i need to have a 2nd look at the fix.. cause im getting different results now as before.

Copy link

ContributorAuthor

ro0NL commentedApr 4, 2018

OK something spiffy is going on

without any src changes etc. in between.. in PHPstorm it seems to consistently format wrong.

Curious if this can be confirmed on some other terminal also.

Copy link

ContributorAuthor

ro0NL commentedApr 4, 2018

It seems to break as soon as it breaks out the window, thus creates a scrollbar. WTF :)

nicolas-grekas modified the milestones:4.1,next

Apr 20, 2018

ro0NL mentioned this pull request

Jul 3, 2018

[Console] Format unset codes break with scrollbar#27832

Closed

Copy link

Member

fabpot commentedSep 4, 2018•
edited
Loading

@ro0NL What do we do here? Any ideas how to move forward?

Copy link

ContributorAuthor

ro0NL commentedSep 4, 2018•
edited
Loading

@fabpot rebased. The breaking format is unrelated (#27832), so if tests are still passing im 👍 to merge. But let me add some rendering tests tomorrow.

Copy link

ContributorAuthor

ro0NL commentedSep 4, 2018•
edited
Loading

And perhaps rethinkWrappableOutputFormatterInterface, overriding the definition ofOutputFormatterInterface::format().

Perhaps more explicitWrappableOutputFormatterInterface::formatAndWrap(string $message, int $width) and let the implementation forward toformat($message, $width), whereOutputFormatter::format() still supports the 2nd arg, but not as part of the interface..

Then again, currently it allows to updateOutputFormatterInterface::format() with the 2nd arg in 5.0 IIUC

Copy link

ContributorAuthor

ro0NL commentedSep 4, 2018

Updated. Much cleaner IMHO.

fabpot approved these changes

Sep 4, 2018

View reviewed changes

Copy link

Member

fabpot left a comment

Choose a reason for hiding this comment

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

Looks much better to me

carsonbot added Status: Reviewed and removed Status: Needs Work labels

Sep 4, 2018

chalasr approved these changes

Sep 4, 2018

View reviewed changes

[Console] Support formatted text cutting

09f8ad9

Copy link

Member

chalasr commentedSep 4, 2018•
edited
Loading

Nice one! Thanks Roland.

chalasr merged commit09f8ad9 intosymfony:master

Sep 4, 2018

chalasr pushed a commit that referenced this pull request

Sep 4, 2018

feature#22225[Console] Support formatted text cutting (ro0NL)

c3fd60d

This PR was squashed before being merged into the 4.2-dev branch (closes#22225).Discussion----------[Console] Support formatted text cutting| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | yes| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes/no| Fixed tickets | #... <!-- #-prefixed issue number(s), if any -->| License       | MIT| Doc PR        | symfony/symfony-docs#... <!--highly recommended for new features-->allows cutting a formatted text to a certain width. Actually needed if we want to support max. column widths in tables (see#22156)```php$text = 'pre <error>foo bar baz</error> post';dump('BEFORE');$output->writeln(wordwrap($output->getFormatter()->format($text), 3, "\n", true), OutputInterface::OUTPUT_RAW);dump('AFTER');$output->writeln($output->getFormatter()->format($text, 3), OutputInterface::OUTPUT_RAW);```![image](https://cloud.githubusercontent.com/assets/1047696/24519346/19c9b0ca-1585-11e7-8437-0bcfb6fab63e.png)Commits-------09f8ad9 [Console] Support formatted text cutting

ro0NL deleted the console/cut-format branch

September 5, 2018 05:58

This was referencedSep 5, 2018

[Console] Support max column width in Table#28373

Merged

[Console] Table column max width / formatted text wrappingsymfony/symfony-docs#10300

Closed

chalasr pushed a commit that referenced this pull request

Sep 11, 2018

feature#28373[Console] Support max column width in Table (ro0NL)

839dc26

This PR was squashed before being merged into the 4.2-dev branch (closes#28373).Discussion----------[Console] Support max column width in Table| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | yes| BC breaks?    | no     <!-- seehttps://symfony.com/bc -->| Deprecations? | no| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->| Fixed tickets |#22156,#27832| License       | MIT| Doc PR        |symfony/symfony-docs#10300Continuation of#22225 to better preserve spaces (which preserves background colors), using `wordwrap` it caused some issues.Also the wrapping was plain wrong by not taking the current line length into account.While at it, it comes with `Table` integration :)Given```php$table = new Table($output);$table->setColumnMaxWidth(0, 2);$table->setRow(0, ['pre <error>foo bar baz</error> post']);$table->render();$table = new Table($output);$table->setColumnMaxWidth(0, 3);$table->setRow(0, ['pre <error>foo bar baz</error> post']);$table->render();$table = new Table($output);$table->setColumnMaxWidth(0, 4);$table->setRow(0, ['pre <error>foo bar baz</error> post']);$table->render();```![image](https://user-images.githubusercontent.com/1047696/45101516-f19b5880-b12b-11e8-825f-6a1d84f68f47.png)Commits-------175f68f [Console] Support max column width in Table

nicolas-grekas modified the milestones:next,4.2

Nov 1, 2018

This was referencedNov 3, 2018

Release v4.2.0-BETA1#29072

Closed

Release v4.2.0-BETA1#29073

Merged

ro0NL mentioned this pull request

Mar 11, 2019

[Console] Upgrade word wrapping in SymfonyStyle#30519

Closed

Labels

Console Feature Status: Reviewed

Movatterモバイル変換

Uh oh!

[Console] Support formatted text cutting#22225

[Console] Support formatted text cutting#22225

Uh oh!

Conversation

ro0NL commentedMar 30, 2017• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

nicolas-grekasApr 3, 2017

Choose a reason for hiding this comment

Uh oh!

ro0NLApr 3, 2017

Choose a reason for hiding this comment

Uh oh!

nicolas-grekasApr 3, 2017

Choose a reason for hiding this comment

Uh oh!

nicolas-grekasApr 3, 2017• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ro0NLApr 3, 2017

Choose a reason for hiding this comment

Uh oh!

ro0NL commentedApr 7, 2017• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

nicolas-grekas commentedOct 8, 2017

Uh oh!

fabpot commentedMar 31, 2018

Uh oh!

ro0NL commentedMar 31, 2018• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

fabpotApr 4, 2018

Choose a reason for hiding this comment

Uh oh!

fabpotApr 4, 2018

Choose a reason for hiding this comment

Uh oh!

ro0NLApr 4, 2018

Choose a reason for hiding this comment

Uh oh!

ro0NL commentedApr 4, 2018

Uh oh!

ro0NL commentedApr 4, 2018

Uh oh!

ro0NL commentedApr 4, 2018

Uh oh!

fabpot commentedSep 4, 2018• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

ro0NL commentedSep 4, 2018• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

ro0NL commentedSep 4, 2018• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

ro0NL commentedSep 4, 2018

Uh oh!

fabpot left a comment

Choose a reason for hiding this comment

Uh oh!

chalasr commentedSep 4, 2018• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ro0NL commentedMar 30, 2017•
edited
Loading

nicolas-grekasApr 3, 2017•
edited
Loading

ro0NL commentedApr 7, 2017•
edited
Loading

ro0NL commentedMar 31, 2018•
edited
Loading

fabpot commentedSep 4, 2018•
edited
Loading

ro0NL commentedSep 4, 2018•
edited
Loading

ro0NL commentedSep 4, 2018•
edited
Loading

chalasr commentedSep 4, 2018•
edited
Loading