Don't redraw a blank line just because its style differs - #2088
Open
afonsojanu wants to merge 1 commit into
Open
Don't redraw a blank line just because its style differs#2088afonsojanu wants to merge 1 commit into
afonsojanu wants to merge 1 commit into
Conversation
get_max_column_index already ignores trailing unstyled whitespace when figuring out how far along a line has real content, but that only decides how far the column loop goes. It doesn't stop a fully blank line's single remaining cell from being redrawn anyway, which happens on the very first render since a freshly reset previous screen's default cell has a different (transparent) style than whatever the new blank cell got assigned. The visible result is a stray space in an otherwise empty spot on the terminal, for example the bottom-left corner right after starting something like ipython. The obvious one-line fix (skip drawing when the line's max column index is 0) doesn't actually work here: a line whose only visible character sits at column 0 also has a max index of 0, so that would silently drop real single-character lines instead. This tracks whether a line has any real content at all, separately from where that content ends, and only skips the redraw when there's truly nothing to show.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2008
get_max_column_indexalready ignores trailing unstyled whitespace when figuring out how far along a line has real content, but that only decides how far the column loop goes. It doesn't stop a fully blank line's single remaining cell from being redrawn anyway, since a freshly reset previous screen's default cell has a different (transparent) style than whatever the new blank cell got assigned, so the diff check treats it as changed even though nothing visible actually is. That's the stray space the issue describes, e.g. the bottom-left corner right after starting something like ipython in an empty terminal.The one-line fix suggested in the issue (skip drawing when a line's max column index is 0) doesn't quite work: a line whose only visible character sits at column 0 also has a max index of 0, since
get_max_column_indexreturns the highest index with real content and there's only one column to check. Applying that fix as written would silently drop any single-character line. This instead tracks whether a line has any real content at all, computed once per row the same wayget_max_column_indexalready does, and only skips the redraw when there's genuinely nothing to show.Added
tests/test_renderer.py(there wasn't one before) exercising_output_screen_diffdirectly against a capturing output, covering: a blank line no longer gets a space written, a line whose only character is at column 0 still gets drawn, and a line with content after column 0 is unaffected either way. Verified the first case fails without the fix (a bare space shows up in the output) and the second would have failed against the naive one-line version.Ran the full test suite (159 passed) plus
ruff format --checkandruff checkon the touched files.