Repository navigation
fix(state): account for dropped break whitespace in wrapped selection - #172
Open
Shaisolaris wants to merge 1 commit into
Open
Shaisolaris wants to merge 1 commit into
Shaisolaris wants to merge 1 commit into
Conversation
selection_col_in_line summed the lengths of the preceding wrapped rows to find where a selected row starts in its line. textwrap drops the whitespace at each break, so on text with spaces the offset came out one character short per preceding row and the copied text was shifted left of the highlighted text. Find each wrapped row in the line in order instead, and use the real starting offset.
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
1 participant
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.
With line wrapping on, copying a selection that starts on a wrapped row copies text shifted to the left of what is highlighted.
selection_col_in_linefinds where a wrapped row starts in its line by summing the character counts of the preceding rows fromtextwrap::wrap.textwrapdrops the whitespace at each break, so for text with spaces the result is one character short for every preceding row. For example, withcols = 6,hello world foowraps tohello/world/foo. Selecting from the start ofworldto the end offoohighlightsworld foobut copiesworld f.This finds each wrapped row in the line in order and uses its real starting offset. The highlighting (
selection_bounds_for_row) already works on display rows and is unchanged.Testing:
extract_selection_across_wrapped_rows_with_spaces. It fails on main (left: Some(" world f")) and passes with this change. The existingextract_selection_across_wrapped_rows(text without spaces) still passes.cargo test --all-features --lib -- --skip pager::tests: 116 passed. I skippedpager::testsbecausebasic_dynamic_pagingneeds a real terminal in my environment.cargo fmt --checkis clean, andcargo clippy --all-features --all-targetsreports the same 4 warnings as main, all in files this PR does not touch.The change does not address terminal column width for wide (e.g. CJK) characters, where a screen column and a character index differ. That is a separate issue.