8000 [Console] Fix clearing sections containing questions by chalasr · Pull Request #28638 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[Console] Fix clearing sections containing questions #28638

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
Oct 3, 2018

Conversation

chalasr
Copy link
Member
@chalasr chalasr commented Sep 29, 2018
Q A
Branch? 4.1
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #28615
License MIT
Doc PR n/a

Given:

$section = $output->section();
$question = "What's your name?";
$name = (new QuestionHelper())->ask($input, $section, new Question($question));
$section->clear();
$section->writeln("$question (<info>$name</info>)");

Before:

After:

@@ -123,6 +124,10 @@ private function doAsk(OutputInterface $output, Question $question)
$ret = trim($this->autocomplete($output, $question, $inputStream, \is_array($autocomplete) ? $autocomplete : iterator_to_array($autocomplete, false)));
}

if ($output instanceof ConsoleSectionOutput) {
$output->addContent($ret);
Copy link
Contributor

Choose a reason for hiding this comment

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

How will this work with hidden input? There might not be any additional lines to clear in that case

Copy link
Member Author

Choose a reason for hiding this comment

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

Answering questions still implies to hit enter, empty and hidden inputs result in a blank line on the screen. Just tried it, works as expected with hidden questions

@nicolas-grekas
Copy link
Member

(4.1 or master? see target branch vs description)

@chalasr chalasr changed the base branch from master to 4.1 October 3, 2018 06:53
@chalasr
Copy link
Member Author
chalasr commented Oct 3, 2018

@nicolas-grekas 4.1, fixed thanks

@chalasr chalasr force-pushed the cli-section-inputs branch from 0e9e6b4 to 81ec57d Compare October 3, 2018 07:17

class ConsoleSectionOutputTest extends TestCase
{
private $stream;

protected function setUp()
{
$this->stream = fopen('php://memory', 'r+', false);
$this->stream = fopen('php://memory', 'r+b', false);
Copy link
Member

Choose a reason for hiding this comment

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

should be reverted /cc @nicolas-grekas


public function testClearSectionContainingQuestion()
{ 8000
$inputStream = fopen('php://memory', 'r+b', false);
Copy link
Member

Choose a reason for hiding this comment

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

same here, no b needed

@fabpot
Copy link
Member
fabpot commented Oct 3, 2018

Thank you @chalasr.

@fabpot fabpot merged commit 81ec57d into symfony:4.1 Oct 3, 2018
fabpot added a commit that referenced this pull request Oct 3, 2018
…asr)

This PR was merged into the 4.1 branch.

Discussion
----------

[Console] Fix clearing sections containing questions

| Q             | A
| ------------- | ---
| Branch?       | 4.1
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #28615
| License       | MIT
| Doc PR        | n/a

Given:
```php
$section = $output->section();
$question = "What's your name?";
$name = (new QuestionHelper())->ask($input, $section, new Question($question));
$section->clear();
$section->writeln("$question (<info>$name</info>)");
```

Before:
![](https://i.imgur.com/nJdKXAo.gif)

After:
![](https://i.imgur.com/WdMtKvV.gif)

Commits
-------

81ec57d [Console] Fix clearing sections containing questions
@cha
8C56
lasr chalasr deleted the cli-section-inputs branch October 3, 2018 08:20
@fabpot fabpot mentioned this pull request Oct 3, 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
0