Skip to content
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

Fix colors getting lost on reflow #17568

Merged
merged 3 commits into from
Jul 30, 2024
Merged

Fix colors getting lost on reflow #17568

merged 3 commits into from
Jul 30, 2024

Conversation

lhecker
Copy link
Member

@lhecker lhecker commented Jul 15, 2024

The "copy the remaining attributes" loop assumes that it has full
ownership over the rows that it copies. For that to be true,
we have to of course make sure that the current write-cursor
is at a fresh, new row in the first place.

Validation Steps Performed

  • In a new pwsh tab with 120 colums:
    Write-Host -NoNewline "`e[36m$('a'*120)`e[m"; sleep 10
  • Resize the window wider
  • Color doesn't get lost

@lhecker lhecker added Area-Output Related to output processing (inserting text into buffer, retrieving buffer text, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-3 A description (P3) labels Jul 15, 2024
@lhecker
Copy link
Member Author

lhecker commented Jul 15, 2024

I suddenly felt an itch and must close this.

@lhecker lhecker closed this Jul 15, 2024
@lhecker lhecker deleted the dev/lhecker/color-reflow branch July 15, 2024 23:48
@lhecker lhecker restored the dev/lhecker/color-reflow branch July 16, 2024 22:52
@DHowett DHowett reopened this Jul 26, 2024
@DHowett
Copy link
Member

DHowett commented Jul 26, 2024

I'm reopening your PR ;P

@lhecker lhecker enabled auto-merge (squash) July 26, 2024 18:09
@DHowett DHowett disabled auto-merge July 29, 2024 20:36
@DHowett
Copy link
Member

DHowett commented Jul 29, 2024

We broke a real tests case

},
{ 1, 0 },
{ 1, 4 },
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not entirely sure if I was able to think the test fix 100% through, but generally speaking this new value makes sense to me. If you expand this diff hunk above, you see the original buffer. The cursor is at the tail end beyond the "Q" in the bottom right corner. There are 12 spaces between the "Q" and the cursor. When we resize to a size of 2x5 the entire buffer ends up being filled with spaces. It's only natural then, that the cursor should end up at the bottom right corner of the new buffer.

DHowett pushed a commit that referenced this pull request Aug 13, 2024
The "copy the remaining attributes" loop assumes that it has full
ownership over the rows that it copies. For that to be true,
we have to of course make sure that the current write-cursor
is at a fresh, new row in the first place.

## Validation Steps Performed
* In a new pwsh tab with 120 colums:
  ``Write-Host -NoNewline "`e[36m$('a'*120)`e[m"; sleep 10``
* Resize the window wider
* Color doesn't get lost

(cherry picked from commit 2f43886)
Service-Card-Id: 93441137
Service-Version: 1.21
DHowett pushed a commit that referenced this pull request Aug 13, 2024
The "copy the remaining attributes" loop assumes that it has full
ownership over the rows that it copies. For that to be true,
we have to of course make sure that the current write-cursor
is at a fresh, new row in the first place.

## Validation Steps Performed
* In a new pwsh tab with 120 colums:
  ``Write-Host -NoNewline "`e[36m$('a'*120)`e[m"; sleep 10``
* Resize the window wider
* Color doesn't get lost

(cherry picked from commit 2f43886)
Service-Card-Id: 93522523
Service-Version: 1.20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Output Related to output processing (inserting text into buffer, retrieving buffer text, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-3 A description (P3)
Projects
Status: Cherry Picked
Development

Successfully merging this pull request may close these issues.

Resizing terminal window destroys both powerline formatting and text layout
3 participants