Skip to content

Conversation

maxbeckers
Copy link
Contributor

Q A
Branch? 6.1
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #47862
License MIT
Doc PR

In the issue #47862 a console output bug was detected.

Before the fix on a windows cli:
2022_10_17_09_44_07_test_console_bug

After the fix:
2022_10_17_10_34_19_fixed_cli_output

The problem have been the linebreaks in the message.

@maxbeckers maxbeckers requested a review from chalasr as a code owner October 17, 2022 08:36
@carsonbot carsonbot added this to the 6.1 milestone Oct 17, 2022
@wouterj wouterj modified the milestones: 6.1, 4.4 Oct 17, 2022
Copy link
Member

@wouterj wouterj left a comment

Choose a reason for hiding this comment

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

I can confirm this does not change the output on Linux.

Should be merged in 4.4 imho (we can switch branches during the merge)

@chalasr
Copy link
Member

chalasr commented Oct 17, 2022

Note that the issue might be SymfonyStyle::block() not properly handling inputs with a leading line break, but I'm fine merging this and revisit if the problem comes out again in the future.

@maxbeckers
Copy link
Contributor Author

maxbeckers commented Oct 17, 2022

@chalasr it's a bug with linebreaks at all on windows cli, not just leading. see the screenshot with a linebreak in the middle.
2022_10_17_09_44_07_test_console_bug_2

The code behind the screenshot:
$style->block(sprintf("Command \"%s\" is\nnot defined.", $name), null, 'error', " ", true);

@chalasr chalasr changed the base branch from 6.1 to 4.4 October 17, 2022 12:37
@chalasr
Copy link
Member

chalasr commented Oct 17, 2022

Thank you @maxbeckers.

@maxbeckers
Copy link
Contributor Author

@chalasr, just a info because i figured out now ... using the constand PHP_EOL what is on windows \r\n will keep the block as expected:
Screenshot 2022-10-18 101417

So in future we should use the constant for linebreaks.

@maxbeckers maxbeckers deleted the patch-47862 branch October 18, 2022 08:15
@stof
Copy link
Member

stof commented Oct 18, 2022

to me, we should rather make SymfonyStyle compatible with both PHP_EOL and \n in the input string.

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