Optional vim bindings in the note card - #6
Open
saforem2 wants to merge 1 commit into
Open
Conversation
The card's textarea has no modal editing, so anyone with vim muscle memory types `dd` into their note. `--vim` (or RT_VIM=1) makes the card modal: normal, insert, and visual, with the motions and operators people actually reach for. Off by default and remembered once used, so nobody's existing habits change without asking. VimState owns the buffer rather than driving bubbles/textarea. That widget's cursor cannot be positioned across a wrapped line — CursorDown gets stuck, verified before committing to the design — so anything built on it would fail on exactly the long notes that make vim bindings worth having. The textarea still handles insert mode, where its native editing and wrapping are right; normal and visual render from the vim buffer with a block cursor. Scope: h j k l, w b e, 0 $, gg G, counts, i a I A o O, x dd D dw d$ d0, cw cc C, yy p P sharing one register, and charwise visual with d/c/y. Unimplemented keys are ignored rather than approximated — a key that silently does something close to vim is worse than one that does nothing. The expectations were generated by running each case through `vim -Nu NONE -es` rather than written from memory, and baked into a table so the comparison runs without vim installed. That caught five bugs my hand-written tests had agreed with: dw on a line's last word deleting nothing, cw eating the trailing space, cw over-reaching on a single-character word, and w and e failing to clamp at the last word. Where the oracle and this implementation disagreed, vim was right — except for two cases that turned out to be artifacts of vim aborting a `normal!` sequence at a failed motion, which are documented and excluded. Driving the built binary through tmux then turned up a sixth: the buffer indexed bytes, so the bullet in "• drag with the mouse" rendered as mojibake the moment the cursor passed it. Buffer and renderer are rune-indexed now, with tests over multi-byte text.
There was a problem hiding this comment.
🟡 Changes recommended
The Vim-mode renderer and textarea sync contain correctness issues (width clipping/padding and a hard-coded cursor-reset cap) that can break the note card layout and cursor positioning for large inputs.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds an opt-in Vim-style modal editing experience inside the note card, including a new VimState buffer/cursor state machine, UI rendering for normal/visual modes, and persistence via CLI flag/env var + workspace save/load.
Changes:
- Introduces
VimState(motions/operators/register/visual) with extensive unit tests and differential expectations against real Vim. - Integrates Vim mode into the note card editor and edit-mode key handling, including mode-aware rendering and buffer handoff with the textarea.
- Adds
--vim/RT_VIMopt-in plus workspace persistence, and documents Vim mode in the README.
File summaries
| File | Description |
|---|---|
| README.md | Documents the new Vim mode, activation, and supported keys. |
| internal/app/vimedit.go | Renders the Vim buffer for normal/visual modes and provides mode footer labels. |
| internal/app/vimedit_test.go | Tests editor-level wiring: key routing, buffer handoff, opt-in default, and persistence. |
| internal/app/vim.go | Implements the Vim modal editing state machine (buffer/cursor/motions/operators/visual/register). |
| internal/app/vim_test.go | Comprehensive unit + differential tests validating behavior (incl. multi-byte text handling). |
| internal/app/storage.go | Adds persisted vim flag to disk format and loads/saves it into Workspace. |
| internal/app/notes.go | Adds Workspace.Vim preference field (omitempty, off by default). |
| internal/app/model.go | Switches editor creation based on Workspace.Vim; routes edit keys into Vim handler when enabled. |
| internal/app/edit.go | Adds NewVimEditor, textarea<->vim sync helpers, and mode-dependent rendering path. |
| internal/app/app.go | Adds --vim flag + RT_VIM parsing and persists preference at exit. |
Review details
- Files reviewed: 10/10 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+58
to
+60
| for i := 0; i < 1000 && e.Ta.Line() > 0; i++ { | ||
| e.Ta.CursorUp() | ||
| } |
Comment on lines
+54
to
+75
| // Render at least one cell so an empty line can still show the | ||
| // cursor. | ||
| n := len(line) | ||
| if n == 0 { | ||
| n = 1 | ||
| } | ||
| for col := 0; col < n; col++ { | ||
| ch := " " | ||
| if col < len(line) { | ||
| ch = string(line[col]) | ||
| } | ||
| switch { | ||
| case i == v.Row && col == v.Col: | ||
| b.WriteString(curStyle.Render(ch)) | ||
| case inVisualSpan(i, col, selR1, selC1, selR2, selC2): | ||
| b.WriteString(curStyle.Render(ch)) | ||
| default: | ||
| b.WriteString(inkStyle.Render(ch)) | ||
| } | ||
| } | ||
| out = append(out, b.String()) | ||
| } |
This was referenced Aug 23, 2026
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.
What
--vimmakes the note card modal. Off by default; remembered once used.h j k l,w b e,0 $,gg G, arrows3l,2dd,5Gi a I A o O,escx dd D dw d$ d0cw cc Cyy p P—ddshares the register, soddpswaps linesv+ motion +d/c/yUnimplemented keys are ignored rather than approximated. A key that
silently does something close to what vim would do is worse than one
that does nothing.
Why the buffer moved out of the textarea
I tried to drive
bubbles/textareathrough its cursor API first, andprobed it before committing to that design.
CursorDowncannot reliablycross a wrapped line:
Which would have broken on exactly the long notes that make vim bindings
worth having. So
VimStateowns the buffer and the cursor, and thetextarea keeps insert mode, where its native editing and wrapping are
what you want. Normal and visual render from the vim buffer with a block
cursor. The buffer is handed across on every mode change.
Checked against real vim
The expectations in
vim_test.gowere generated by running each casethrough
vim -Nu NONE -es -c 'normal! gg0<keys>', not written frommemory, and baked into a table so the comparison runs without vim
installed.
That found five bugs my hand-written tests had cheerfully agreed with:
dwon a line's last word deleted nothingcwate the trailing space (vim'scwisce— a documented irregularity)cwon a single-character word over-reached into the next wordwdidn't clamp at the end of the last worderan off the endTwo apparent divergences turned out to be artifacts: inside a single
normal!command vim aborts the whole sequence at a failed motion, sobxat column 0 reports "no change". Issued as separate commands vimagrees with this implementation. Those cases are excluded, with a comment
saying why.
And one the tests couldn't find
Running the built binary in tmux showed
• drag with the mouserenderingas
⢠drag with the mouseas soon as the cursor passed the bullet. Thebuffer indexed bytes; every one of my tests used ASCII. Buffer and
renderer are rune-indexed now, with tests over multi-byte text.
Worth stating plainly: 90% statement coverage on the new code did not
catch this. Looking at the actual screen did.
Testing
go test ./...,-race,vet,gofmtclean. ~1000 lines of tests:motions, counts, operators, registers, visual mode, edge cases (empty
buffer, cursor bounds under adversarial key sequences, dangling
operators, special keys not leaking into the buffer), the differential
grid, and the model-level wiring.
Driven end to end through tmux against the real binary: normal-mode keys
navigating without typing,
i→ type →esc,x,dd, the modeindicator in the footer, multi-byte text intact, and
vim: truepersisting to
notes.json.Notes
Independent of #3, #4, and #5 — any merge order works.
Not implemented, deliberately:
.repeat, named registers,f/tmotions, text objects (
ciw), and multi-line operator motions (d2j).Each is real work and none felt essential for a sticky note. Happy to add
any of them if you'd rather have them.