Skip to content

Don't reflow a line as wrapped if it broke on the last cell#5368

Closed
zadjii-msft wants to merge 21 commits intomainfrom
dev/migrie/b/3088-weird-exact-wrap-resize
Closed

Don't reflow a line as wrapped if it broke on the last cell#5368
zadjii-msft wants to merge 21 commits intomainfrom
dev/migrie/b/3088-weird-exact-wrap-resize

Conversation

@zadjii-msft
Copy link
Member

Summary of the Pull Request

If we write a character to the last cell of a row, we'll mark that row as wrapped.

If we linefeed at that point, we'll unmark that row as wrapped. (Linefeeds are important here, we learned in #5291/#5294 that all cursor movements don't actually break a line).

However, in TextBuffer::Reflow, if we increase the width of that row, we'll actually reflow it and the following row as though the first row wrapped. This is due to a iRight < cOldColsTotal that should have been iRight <= cOldColsTotal.

References

PR Checklist

Validation Steps Performed

Tested with the following command in WSL

printf "%*s\\n" "$(tput cols)" "" | tr " " "X"
  • In conhost, this works like a charm
  • This DOESN'T work in the Terminal. Presumably, the last cell getting written like this causes us to print the 'X' line as wrapped, but we never re-render that character as not wrapped when the line breaks. This seems timing related. I've got a test ready, but I want to make sure the test gets all possible cases here, including WriteCharsLegacy ones

I'm drafting this to get feedback on the < to <= fix so far. I'll work on the conpty fix in the meantime

@zadjii-msft zadjii-msft marked this pull request as draft April 16, 2020 12:15
// the cursor to the second line.
//
// Unlike BreakLinesOnCursorMovement, we're only testing the cases where a
// cursor movement actaully broke the line. This test would be much too
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// cursor movement actaully broke the line. This test would be much too
// cursor movement actually broke the line. This test would be much too

@zadjii-msft zadjii-msft mentioned this pull request May 12, 2020
31 tasks
@microsoft-github-updates microsoft-github-updates bot changed the base branch from master to main October 22, 2020 00:27
@ghost ghost 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-2 A description (P2) Product-Conhost For issues in the Console codebase labels Oct 22, 2020
@github-actions

This comment was marked as outdated.

@zadjii-msft
Copy link
Member Author

This PR is super old and I'm not coming back to it any time soon. The buffer has changed quite a bit. As noted in #14821 (comment), there might be new ways of fixing this. The buffer's also going through pretty majir rewrites, so best to not have too many cooks.

@zadjii-msft zadjii-msft closed this Apr 6, 2023
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-2 A description (P2) Product-Conhost For issues in the Console codebase

Projects

None yet

Development

Successfully merging this pull request may close these issues.

WSL Terminal Line Endings (the "exact wrap" bug)

3 participants