Repository navigation
Conversation
|
This PR resolves #18 |
|
Heads up: #62 just merged, which restructured the vim engine from a single
Also note CI is now stricter: Biome runs with |
fb1a334 to
296f116
Compare
|
I rebased onto main and ported the changes to the new modular vim engine. Thanks for the heads up. |
| const lineStart = text.lastIndexOf("\n", target - 1) + 1; | ||
|
|
||
| editor.cursorOffset = lineStart; | ||
| for (let i = lineStart; i < target; i++) editor.moveCursorRight(); |
There was a problem hiding this comment.
Thanks for adding these motions and updating the PR after the refactor!
I found a regression in this loop. When text is selected, moveCursorRight() jumps to the selection’s end instead of moving one character.
To reproduce, start with hello world, put the cursor on e, then press ve<Esc>x.
Vim leaves hell world. With this PR, I get helloworld in OpenCode 1.18.21. It deletes the space instead of the o.
Could we set the cursor position without using selection-aware movement here? A test for this sequence would help catch it.
| } | ||
|
|
||
| function moveToFirstNonBlank(actions: Action[], line: string) { | ||
| actions.push({ type: "cmd", cmd: "input.line.home" }); |
There was a problem hiding this comment.
input.line.home has a surprising behavior in OpenTUI. At the start of a later line, it moves to the end of the previous line.
For example, start with:
previous
next
Put the cursor on n, then press Ix<Esc>. I get previousx on the first line instead of xnext on the second.
Can we keep this move on the current line? Please add a test for this case too.
| const target = firstNonBlankOnLine(text, offset); | ||
| const start = Math.min(offset, target); | ||
| const end = Math.max(offset, target) - 1; |
There was a problem hiding this comment.
Tabs expose a mismatch here. The helper counts positions in the text string, but the editor's cursor position includes the tab's display width.
Start with a tab and two spaces before hello world. Put the cursor on w, then press d^.
Vim leaves "\t world". In OpenCode 1.18.21, this PR leaves "\t world". One indentation space gets deleted.
Can we make sure both positions use the same units before calculating the range? A test that checks the resulting text would catch this.
|
|
||
| export function firstNonBlankOnLine(text: string, offset: number, linesDown = 0): number { | ||
| const safeOffset = Math.min(Math.max(offset, 0), text.length); | ||
| let start = text.lastIndexOf("\n", safeOffset - 1) + 1; |
There was a problem hiding this comment.
This skips the first line when the buffer starts with a newline. JavaScript's lastIndexOf("\n", -1) still checks position zero, so start becomes 1 instead of 0.
With an empty line followed by hello, pressing ^ on the empty line jumps to h. It should stay on the empty line.
The same expression in cursorTo also makes gg skip that first line.
Could we handle offset zero explicitly in both places and add a test for a buffer that starts with a newline?
296f116 to
e468604
Compare
|
Rebased onto main again and addressed those issues. I also added tests like you recommended. Replaced selection-aware movement with direct cursor positioning for visual motions. Replaced input.line.home for I with direct current-line positioning. Normalized vim string offsets to OpenTUI display columns for tabs before applying cursor, selection, and delete actions. And changed it to handle offset zero for initial blank lines. Also fixed direct visual h/l selection extension and I after tab indentation. |
2 participants
This PR adds support for the
I,_, and^Vim motions, all of which operate on the first non-blank character of a line.^was previously mapped to Home, which moves the cursor to the beginning of the line rather than the first non-blank character. This PR fixes that behavior.It also adds support for
Iand_. The_motion supports counts; for example,3_moves the cursor to the first non-blank character of the third line, equivalent to2j^in Vim.Also adds tests covering the new behavior.