fix: stop the insert caret from shifting the text - #325
Merged
Merged
Conversation
The Insert-mode caret was drawn as a `▏` spliced between two characters of the block's text. A terminal is a cell grid and a glyph costs a cell, so everything right of the cursor sat one column over and walked back and forth by one as the cursor moved through the line. The row measured one cell wider than its text too, so a block near the pane edge wrapped a character earlier while it was being edited than it did the moment Esc was pressed. The Normal-mode block cursor never had it, because it inverts the character under it rather than adding one. The caret marks the character it sits before now, on the cell that is already there. The underline is load-bearing rather than decoration, since cursor_caret_fg is a foreground colour and a foreground colour paints nothing on a space, which is where a caret in prose spends much of its time. Past the end of the line there is no character to mark, so the `▏` stays. It has nothing to its right to shift, which is also why the overlay and property inputs keep theirs, PropertyEdit having no cursor column at all. Making the caret a text cell handed it a second set of rules and the first version of this broke against them. view::wrap treats a space as a separator it may discard, absorbed at a wrap boundary and trimmed off the end of a row, which was sound while the caret was its own glyph. A caret parked on a space near a break was thrown away and the user saw no cursor at all, at columns 9 and 19 of a 43-cell block in a 16-cell pane, found by walking every column rather than picking one. push_wrapped takes the style the cursor cell was painted with now, so it can tell a load-bearing space from a separator. A blanket "a styled space is never a separator" rule was the wrong shape, because it would also catch the spaces inside `**bold**`, `` `code` `` and `[[a page ref]]`, which really are separators. The block cursor had the same defect quietly and gets the fix too. The style lives on Theme rather than in the view module. theme.rs declares itself the owner of the modifier formula, so a modifier decided next to the renderer would be a second owner of it, and DESIGN.md and docs/theming.md both claimed underline belonged to the three link roles alone. outline.rs crossed its file-size baseline on the way, so its tests move to a file of their own. Two things left alone. The caret styles a single char, not a grapheme cluster, so on a zero-width continuation code point (a combining accent, a ZWJ inside an emoji sequence) it paints a zero-width cell and disappears. Arrow keys step per char, so the position is reachable, and closing it needs grapheme segmentation this workspace does not depend on. And the caret is still painted into the cell grid instead of handed to the terminal's own cursor, which would blink and take the shape the user configured. That one needs the caret's screen coordinates after wrapping and scrolling, and an arbiter for the single terminal cursor between the outline and every overlay that draws its own. Fixes #320 Signed-off-by: Avelino <31996+avelino@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
A critical test compilation issue and a moderate cursor-wrapping bug remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (3)
What changed in this PR
Fixes TUI caret rendering so in-line carets no longer shift text, while preserving visibility during wrapping.
Changes:
- Paints the caret on the existing character with underline styling.
- Makes wrapping cursor-aware and adds regression tests.
- Updates theme documentation, changelog, and file-size baseline.
| File | Review summary |
|---|---|
docs/theming.md |
Two nit findings: clarify cursor_caret behavior and use semantic line breaks. |
DESIGN.md |
Modifier documentation updated; no findings. |
crates/outl-tui/src/view/wrap.rs |
Moderate finding: style equality can misidentify bold spaces as the past-end caret. |
crates/outl-tui/src/view/row_chrome.rs |
Wrapper call updated; no findings. |
crates/outl-tui/src/view/outline/tests.rs |
Critical finding: distinct closure types prevent the new test module from compiling. |
crates/outl-tui/src/view/outline.rs |
Cursor rendering updated; no findings. |
crates/outl-tui/src/view/embed.rs |
Wrapper call updated; no findings. |
crates/outl-tui/src/theme.rs |
Adds caret styling helper; no findings. |
crates/outl-tui/CLAUDE.md |
Nit finding: avoid duplicating canonical theming details and follow semantic line breaks. |
CHANGELOG.md |
Nit finding: use semantic line breaks. |
.github/file-size-baseline.txt |
Baseline updated; no findings. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…mantic line breaks Review follow-up on #325. outl-tui/CLAUDE.md keeps only the contracts the crate has to hold up (single owner of the style, the wrap-separator rule, the append-only inputs, the grapheme gap) and links docs/theming.md for how the caret looks and is themed. The #320 changelog entry and the theming tips are reflowed to one sentence per line. Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
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.


The Insert-mode caret was drawn as a
▏spliced between two characters of the block's text. A terminal is a cell grid and a glyph costs a cell, so everything right of the cursor sat one column over and walked back and forth by one as the cursor moved through the line. The row measured one cell wider than its text too, so a block near the pane edge wrapped a character earlier while it was being edited than it did the moment Esc was pressed. The Normal-mode block cursor never had it, because it inverts the character under it rather than adding one.The caret marks the character it sits before now, on the cell that is already there. The underline is load-bearing rather than decoration, since cursor_caret_fg is a foreground colour and a foreground colour paints nothing on a space, which is where a caret in prose spends much of its time. Past the end of the line there is no character to mark, so the
▏stays. It has nothing to its right to shift, which is also why the overlay and property inputs keep theirs, PropertyEdit having no cursor column at all.Making the caret a text cell handed it a second set of rules and the first version of this broke against them. view::wrap treats a space as a separator it may discard, absorbed at a wrap boundary and trimmed off the end of a row, which was sound while the caret was its own glyph. A caret parked on a space near a break was thrown away and the user saw no cursor at all, at columns 9 and 19 of a 43-cell block in a 16-cell pane, found by walking every column rather than picking one. push_wrapped takes the style the cursor cell was painted with now, so it can tell a load-bearing space from a separator. A blanket "a styled space is never a separator" rule was the wrong shape, because it would also catch the spaces inside
**bold**,`code`and[[a page ref]], which really are separators. The block cursor had the same defect quietly and gets the fix too.The style lives on Theme rather than in the view module. theme.rs declares itself the owner of the modifier formula, so a modifier decided next to the renderer would be a second owner of it, and DESIGN.md and docs/theming.md both claimed underline belonged to the three link roles alone. outline.rs crossed its file-size baseline on the way, so its tests move to a file of their own.
Two things left alone. The caret styles a single char, not a grapheme cluster, so on a zero-width continuation code point (a combining accent, a ZWJ inside an emoji sequence) it paints a zero-width cell and disappears. Arrow keys step per char, so the position is reachable, and closing it needs grapheme segmentation this workspace does not depend on. And the caret is still painted into the cell grid instead of handed to the terminal's own cursor, which would blink and take the shape the user configured. That one needs the caret's screen coordinates after wrapping and scrolling, and an arbiter for the single terminal cursor between the outline and every overlay that draws its own.
Fixes #320
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is enabled.