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

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

Closed
wants to merge 21 commits into from

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