Fix split wide characters when overlapping writes occur#930
Merged
Conversation
f16886a to
e910609
Compare
Collaborator
|
I think this introduces a small regression. If horizontal clipping slices the incoming write down to an empty string, the new boundary cleanup still runs and mutates the existing wide character even though nothing is actually being written. I manually verified this with a clipped write that should be a no-op: const output = new Output({width: 4, height: 1});
output.write(0, 0, 'あい', {transformers: []});
output.clip({x1: 1, x2: 1, y1: undefined, y2: undefined});
output.write(0, 0, 'Z', {transformers: []});
output.unclip();On So I think this needs a guard to skip the cleanup when the sliced line is empty, plus a regresion test for that case. |
6f52b2b to
a54cc8a
Compare
When two elements overlap in the output grid (e.g. via position="absolute" or negative margins) and the write lands in the middle of a wide (CJK) character, one half of the character was left behind, corrupting terminal output. Add boundary checks in Output.write() to replace orphaned halves with a space: - Left edge: if the first cell is a trailing placeholder, clear its leading cell. - Right edge: if the cell just past the write range is an orphaned trailing placeholder, clear it. Fixes #929
a54cc8a to
cbab8de
Compare
Contributor
Author
|
Thanks for catching that. Fixed in cbab8de:
|
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
2 participants
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.
Summary
position="absolute", negative margins, etc.) and the write lands in the middle of a wide (CJK) character, one half of the character was left behind, corrupting terminal outputOutput.write()to replace orphaned halves of wide characters with a spaceFixes #929
Design note
When a wide character is partially overwritten, the orphaned cell is replaced with a half-width space. This matches the behavior of vim, tmux, and less, which all replace a split wide character with a space to keep column alignment intact.
Test plan
overlay on 2nd cell of CJK character clears the full character— overlay at column 9 (trailing cell ofお) replacesおwith a spaceoverlay on 1st cell of CJK character clears trailing placeholder— overlay at column 10 (leading cell ofか) clears orphaned placeholderCJK overlay on 2nd cell of CJK clears both sides— wide overlay spans multiple CJK characters, both boundary characters are cleaned upclipped empty write does not corrupt existing wide characters— a write clipped to nothing does not mutate surrounding cells