Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

[Console] Fix error output on windows cli #47883

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 17, 2022
Merged

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
Morty Proxy This is a proxified and sanitized view of the page, visit original site.