Skip to content

[Console] Support iterable in SymfonyStyle::write/writeln #26863

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
Apr 9, 2018
Merged

[Console] Support iterable in SymfonyStyle::write/writeln #26863

merged 1 commit into from
Apr 9, 2018

Conversation

ogizanagi
Copy link
Contributor

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

Relates to #26847.

This would enable iterables benefits even when using SymfonyStyle output.

@ogizanagi ogizanagi added this to the 4.1 milestone Apr 8, 2018
@ro0NL
Copy link
Contributor

ro0NL commented Apr 8, 2018 •
edited
Loading

Should/Can we update string|array to string|iterable PHPdoc in StyleInterface etc?

@ogizanagi
Copy link
Contributor Author

These methods are not part of StyleInterface, but OutputInterface which is already up to date. The StyleInterface methods are unchanged.

@ogizanagi
Copy link
Contributor Author

(I'm not convinced enabling support for iterables in StyleInterface methods is worth it, appart maybe listing but there is already an array typehint).

@ro0NL
Copy link
Contributor

ro0NL commented Apr 8, 2018 •
edited
Loading

StyleInterface could benefit the same upgrade (we could yield the lines from createBlock()).

edit:

I'm not convinced enabling support for iterables in StyleInterface methods is worth it

Well we did it for write() why not success() and such =/ I agree it's probably less common to use here 😅

@ogizanagi
Copy link
Contributor Author

ogizanagi commented Apr 8, 2018 •
edited
Loading

I don't see many use cases for generators when displaying blocks, title, sections, etc... while write/writeln are important to update because the SymfonyStyle can be passed along to any method/class requiring an OutputInterface and that could really benefit from generators memory saving.

chalasr
chalasr approved these changes Apr 9, 2018
View reviewed changes
Copy link
Member

@chalasr chalasr left a comment

Choose a reason for hiding this comment

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

with minor comment

...ony/Component/Console/Tests/Fixtures/Style/SymfonyStyle/command/command_4_with_iterators.php

$output->writeln('Lorem ipsum dolor sit amet');
$output->newLine(2); //Should append an extra blank line
$output->title('Fifth title');
Copy link
Member

Choose a reason for hiding this comment

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

Sixth :)

@chalasr
Copy link
Member

chalasr commented Apr 9, 2018

Thank you @ogizanagi.

@chalasr chalasr merged commit d66827e into symfony:master Apr 9, 2018
chalasr added a commit that referenced this pull request Apr 9, 2018
@chalasr
feature #26863 [Console] Support iterable in SymfonyStyle::write/writ…
5736321 
…eln (ogizanagi)

This PR was merged into the 4.1-dev branch.

Discussion
----------

[Console] Support iterable in SymfonyStyle::write/writeln

| Q             | A
| ------------- | ---
| Branch?       | master <!-- see below -->
| Bug fix?      | no
| New feature?  | no <!-- don't forget to update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no <!-- don't forget to update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | N/A   <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        | N/A

Relates to #26847.

This would enable iterables benefits even when using `SymfonyStyle` output.

Commits
-------

d66827e [Console] Support iterable in SymfonyStyle::write/writeln
@ogizanagi ogizanagi deleted the console/sf_style_write_iterable branch April 9, 2018 07:51
Tobion
Tobion reviewed Apr 9, 2018
View reviewed changes
src/Symfony/Component/Console/Style/SymfonyStyle.php
{
// We need to know if the two last chars are PHP_EOL
// Preserve the last 4 chars inserted (PHP_EOL on windows is two chars) in the history buffer
return array_map(function ($value) {
return substr($value, -4);
}, array_merge(array($this->bufferedOutput->fetch()), (array) $messages));
Copy link
Contributor

Choose a reason for hiding this comment

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

so the fetch part was useless all along?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It wasn't useless, but as we now only handle strings one by one and are writing to the buffer directly (instead of concatenating arrays of buffered output), it is not required anymore.

@Tobion
Copy link
Contributor

Tobion commented Apr 9, 2018

Nice change @ogizanagi . Thanks

@javiereguiluz javiereguiluz mentioned this pull request Apr 20, 2018
Closed
@fabpot fabpot mentioned this pull request May 7, 2018
Merged
@wenjy wenjy mentioned this pull request Nov 24, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Tobion Tobion Tobion left review comments

@chalasr chalasr chalasr approved these changes

Assignees
No one assigned
Labels
Console Status: Reviewed
Projects
None yet
Milestone
4.1
Development

Successfully merging this pull request may close these issues.
5 participants
@ogizanagi @ro0NL @chalasr @Tobion @carsonbot