Skip to content

[Console] fix progress bar messing output in section when there is an EOL #59874

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
fabpot merged 1 commit into from
Mar 3, 2025
Merged

[Console] fix progress bar messing output in section when there is an EOL #59874

fabpot merged 1 commit into from
Mar 3, 2025
+84 −0

Conversation

@joelwurtz
Copy link
Contributor

@joelwurtz joelwurtz commented Feb 27, 2025

Q A
Branch? 6.4
Bug fix? yes
New feature? no
Deprecations? no
Issues #52446
License MIT

This fix the progress bar making the output erase previous line when there is an EOL at the end of it

@joelwurtz joelwurtz requested a review from chalasr as a code owner February 27, 2025 11:01
@carsonbot carsonbot added this to the 6.4 milestone Feb 27, 2025
@joelwurtz joelwurtz mentioned this pull request Feb 27, 2025
Closed
@joelwurtz joelwurtz changed the title fix(console): fix progress bar messing output in section when there i… fix(console): fix progress bar messing output in section when there is an EOL Feb 27, 2025
@joelwurtz joelwurtz force-pushed the fix/fix-progress-bar-eol branch 2 times, most recently from 8283b46 to c93d5e0 Compare February 27, 2025 11:06
@joelwurtz joelwurtz changed the title fix(console): fix progress bar messing output in section when there is an EOL [Console] fix progress bar messing output in section when there is an EOL Feb 27, 2025
cedric-anne
cedric-anne reviewed Feb 27, 2025
View reviewed changes
Copy link
Contributor

@cedric-anne cedric-anne left a comment

Choose a reason for hiding this comment

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

With the proposed fix, it works as expected with an ending PHP_EOL, but it still remove some lines when the ending PHP_EOL is not present, at least as long as the $message is an empty string.

@joelwurtz joelwurtz force-pushed the fix/fix-progress-bar-eol branch from 9be3790 to ad6661e Compare February 27, 2025 14:42
@joelwurtz
Copy link
Contributor Author

joelwurtz commented Feb 27, 2025

Thanks for the insight @cedric-anne i added a test and changed the fix to make it work in both cases

@cedric-anne
Copy link
Contributor

cedric-anne commented Feb 27, 2025

I still have the issue if I use the following format: '[%bar%] %percent:3s%%' . PHP_EOL . '<comment>%message%</comment>'. I guess it is because it produces PHP_EOL . <comment></comment> that is not detected as an ending new line but is acting the same when rendered int he terminal.

@joelwurtz joelwurtz force-pushed the fix/fix-progress-bar-eol branch from ad6661e to 1edcaa1 Compare February 27, 2025 17:36
@joelwurtz
Copy link
Contributor Author

joelwurtz commented Feb 27, 2025

I still have the issue if I use the following format:

Should be good, it now checks if last line is an empty message without formatting, also added a test case for your use case

cedric-anne
cedric-anne approved these changes Feb 27, 2025
View reviewed changes
Copy link
Contributor

@cedric-anne cedric-anne left a comment

Choose a reason for hiding this comment

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

👍

@carsonbot carsonbot changed the title [Console] fix progress bar messing output in section when there is an EOL fix progress bar messing output in section when there is an EOL Feb 27, 2025
@carsonbot carsonbot changed the title fix progress bar messing output in section when there is an EOL [Console] fix progress bar messing output in section when there is an EOL Feb 27, 2025
chalasr
chalasr reviewed Feb 27, 2025
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.

Tested locally

@joelwurtz joelwurtz force-pushed the fix/fix-progress-bar-eol branch from c85392c to 2a3c28c Compare February 28, 2025 09:35
fabpot
fabpot requested changes Mar 1, 2025
View reviewed changes
@joelwurtz joelwurtz force-pushed the fix/fix-progress-bar-eol branch from 2a3c28c to 68b4c1b Compare March 3, 2025 10:01
mtarld
mtarld reviewed Mar 3, 2025
View reviewed changes
@joelwurtz joelwurtz force-pushed the fix/fix-progress-bar-eol branch from 68b4c1b to 3f9fa83 Compare March 3, 2025 17:15
@joelwurtz joelwurtz force-pushed the fix/fix-progress-bar-eol branch from 3f9fa83 to cae2241 Compare March 3, 2025 17:16
@fabpot
Copy link
Member

fabpot commented Mar 3, 2025

Thank you @joelwurtz.

@fabpot fabpot merged commit 647dba0 into symfony:6.4 Mar 3, 2025
11 checks passed
@joelwurtz joelwurtz deleted the fix/fix-progress-bar-eol branch March 3, 2025 19:22
This was referenced Mar 28, 2025
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mtarld mtarld mtarld left review comments

@chalasr chalasr chalasr left review comments

@fabpot fabpot fabpot approved these changes

+1 more reviewer

@cedric-anne cedric-anne cedric-anne approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Bug Console Status: Reviewed

Projects

None yet

Milestone

6.4

Development

Successfully merging this pull request may close these issues.

7 participants

@joelwurtz @cedric-anne @fabpot @mtarld @chalasr @OskarStark @carsonbot