feat: adding word actions, not done, and lots of failing tests #4

azpect · 2026-04-03T13:19:12-07:00

azpect commented

2026-04-03 13:19:12 -07:00

This is a test for the new AI reviewing tool

azpect added 1 commit 2026-04-03 13:19:12 -07:00

feat: adding word actions, not done, and lots of failing tests

Run Test Suite / test (push) Failing after 15s

Details

Run Test Suite / test (pull_request) Failing after 15s

Details

9938f0d5d3

This is a test for the new AI reviewing tool

azpect added 1 commit 2026-04-03 13:20:04 -07:00

Merge branch 'master' into feature/word-motions

Run Test Suite / test (push) Failing after 15s

Details

Run Test Suite / test (pull_request) Failing after 14s

Details

Qodo AI PR Reviewer / qodo_review (pull_request) Successful in 22s

Details

4a59451d90

azpect added 1 commit 2026-04-03 13:27:54 -07:00

fix: trying to fix qodo

Run Test Suite / test (push) Failing after 18s

Details

Run Test Suite / test (pull_request) Failing after 15s

Details

Qodo AI PR Reviewer / qodo_review (pull_request) Successful in 7s

Details

3efba6d575

azpect added 1 commit 2026-04-03 13:35:29 -07:00

fix: more qodo fixes

Run Test Suite / test (push) Failing after 17s

Details

Run Test Suite / test (pull_request) Failing after 14s

Details

Qodo AI PR Reviewer / qodo_review (pull_request) Successful in 7s

Details

24eea1d08e

azpect added 1 commit 2026-04-03 13:41:42 -07:00

fix: trying to fix ci/cd qodo

Run Test Suite / test (push) Failing after 15s

Details

Run Test Suite / test (pull_request) Failing after 16s

Details

Qodo AI PR Reviewer / qodo_review (pull_request) Successful in 7s

Details

e54d49109e

azpect added 1 commit 2026-04-03 13:45:00 -07:00

fix: again dude wtf

Run Test Suite / test (push) Failing after 14s

Details

Run Test Suite / test (pull_request) Failing after 15s

Details

Qodo AI PR Reviewer / qodo_review (pull_request) Successful in 8s

Details

b9e9fb2f5f

azpect added 1 commit 2026-04-03 14:26:57 -07:00

fix: gemini pro attempt

Run Test Suite / test (push) Failing after 18s

Details

Run Test Suite / test (pull_request) Failing after 17s

Details

Qodo PR Agent (Gemini) / Run PR Agent (pull_request) Successful in 13s

Details

47a867a537

azpect added 1 commit 2026-04-03 14:30:45 -07:00

fix: copied from docs

Run Test Suite / test (push) Failing after 17s

Details

Run Test Suite / test (pull_request) Failing after 16s

Details

4a2c8895af

azpect added 1 commit 2026-04-03 14:32:56 -07:00

fix: forgot to add sync

Run Test Suite / test (push) Failing after 16s

Details

Run Test Suite / test (pull_request) Failing after 15s

Details

PR Agent (Gemini) / pr_agent_job (pull_request) Successful in 14s

Details

c58ec77dba

azpect added 1 commit 2026-04-03 14:36:01 -07:00

fix: added gitea to provider

Run Test Suite / test (push) Failing after 15s

Details

Run Test Suite / test (pull_request) Failing after 14s

Details

PR Agent (Gemini) / pr_agent_job (pull_request) Successful in 7s

Details

194b848d6b

azpect added 1 commit 2026-04-03 14:38:20 -07:00

fix: removing action

Run Test Suite / test (push) Failing after 15s

Details

Run Test Suite / test (pull_request) Failing after 14s

Details

5833f2312b

azpect commented

2026-04-03 14:54:51 -07:00

Review please :)

azpect commented

2026-04-03 14:55:37 -07:00

/review

azpect commented

2026-04-03 15:03:32 -07:00

/review

qodo-bot commented

2026-04-03 15:03:36 -07:00

Preparing review...

azpect commented

2026-04-03 15:06:08 -07:00

/review

qodo-bot commented

2026-04-03 15:06:22 -07:00

Preparing review...

azpect commented

2026-04-03 15:09:38 -07:00

/review

qodo-bot commented

2026-04-03 15:09:43 -07:00

Preparing review...

azpect commented

2026-04-03 15:12:00 -07:00

/review

qodo-bot commented

2026-04-03 15:12:08 -07:00

Preparing review...

azpect commented

2026-04-03 15:13:47 -07:00

/review

qodo-bot commented

2026-04-03 15:13:52 -07:00

Preparing review...

azpect commented

2026-04-03 15:16:34 -07:00

/review

qodo-bot commented

2026-04-03 15:16:38 -07:00

Preparing review...

azpect commented

2026-04-03 15:17:08 -07:00

/review

qodo-bot commented

2026-04-03 15:17:37 -07:00

PR Reviewer Guide 🔍

(Review updated until commit `6b8845b245`)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review

Failing Tests

The tests for 'yB' and 'ygE' are explicitly marked as failing because the cursor is not moving to the start of the yanked region. This indicates a bug in how these backward WORD/word-end motions interact with the yank operator, specifically regarding cursor positioning after the operation.

// BUG: This is a failing tests, cursor is not moving at start of yank
t.Run("test 'yB' yanks WORD including punctuation", func(t *testing.T) {
	lines := []string{"hello.world next"}
	tm := newTestModelWithLinesAndCursorPos(t, lines, core.Position{Col: 15, Line: 0})
	sendKeys(tm, "y", "B")

	m := getFinalModel(t, tm)
	// Text should remain unchanged
	if m.ActiveBuffer().Lines[0].String() != "hello.world next" {
		t.Errorf("Line(0) = %q, want 'hello.world next'", m.ActiveBuffer().Lines[0].String())
	}
	// Cursor should be at start of yanked region (index 12)
	if m.ActiveWindow().Cursor.Col != 12 {
		t.Errorf("CursorX() = %d, want 12", m.ActiveWindow().Cursor.Col)
	}
})

Test Logic Review

The tests comparing 'dB' with 'db' (lines 1851-1871) and 'dgE' with 'dge' (lines 2380-2401) show identical expected outcomes for deleting text like 'hello.world next'. Given the distinction between 'word' and 'WORD' (where 'WORD' treats punctuation as part of the word), it's worth verifying if the expected behavior for deletion is truly identical in these cases, or if the tests should be adjusted to reflect a difference.

t.Run("test 'dB' vs 'db' on dotted text", func(t *testing.T) {
	lines := []string{"hello.world next"}

	// First test 'db'
	tm1 := newTestModelWithLinesAndCursorPos(t, lines, core.Position{Col: 15, Line: 0})
	sendKeys(tm1, "d", "b")
	m1 := getFinalModel(t, tm1)

	if m1.ActiveBuffer().Lines[0].String() != "hello.world t" {
		t.Errorf("'db': Line(0) = %q, want 'hello.world t'", m1.ActiveBuffer().Lines[0].String())
	}

	// Now test 'dB'
	tm2 := newTestModelWithLinesAndCursorPos(t, lines, core.Position{Col: 15, Line: 0})
	sendKeys(tm2, "d", "B")
	m2 := getFinalModel(t, tm2)

	if m2.ActiveBuffer().Lines[0].String() != "hello.world t" {
		t.Errorf("'dB': Line(0) = %q, want 'hello.world t'", m2.ActiveBuffer().Lines[0].String())
	}
})

## PR Reviewer Guide 🔍 #### (Review updated until commit https://git.gophernest.net/azpect/Gim/commit/6b8845b245724555a647fe1852db4572b605c3b5) Here are some key observations to aid the review process: <table> <tr><td>⏱️ <strong>Estimated effort to review</strong>: 3 🔵🔵🔵⚪⚪</td></tr> <tr><td>🧪 <strong>PR contains tests</strong></td></tr> <tr><td>🔒 <strong>No security concerns identified</strong></td></tr> <tr><td>⚡ <strong>Recommended focus areas for review</strong><br><br> <details><summary><a href='https://git.gophernest.net/azpect/Gim/src/branch/feature/word-motions/internal/editor/integration_motion_word_test.go#L1885-L1900'><strong>Failing Tests</strong></a> The tests for 'yB' and 'ygE' are explicitly marked as failing because the cursor is not moving to the start of the yanked region. This indicates a bug in how these backward WORD/word-end motions interact with the yank operator, specifically regarding cursor positioning after the operation. </summary> ```go // BUG: This is a failing tests, cursor is not moving at start of yank t.Run("test 'yB' yanks WORD including punctuation", func(t *testing.T) { lines := []string{"hello.world next"} tm := newTestModelWithLinesAndCursorPos(t, lines, core.Position{Col: 15, Line: 0}) sendKeys(tm, "y", "B") m := getFinalModel(t, tm) // Text should remain unchanged if m.ActiveBuffer().Lines[0].String() != "hello.world next" { t.Errorf("Line(0) = %q, want 'hello.world next'", m.ActiveBuffer().Lines[0].String()) } // Cursor should be at start of yanked region (index 12) if m.ActiveWindow().Cursor.Col != 12 { t.Errorf("CursorX() = %d, want 12", m.ActiveWindow().Cursor.Col) } }) ``` </details> <details><summary><a href='https://git.gophernest.net/azpect/Gim/src/branch/feature/word-motions/internal/editor/integration_motion_word_test.go#L1851-L1871'><strong>Test Logic Review</strong></a> The tests comparing 'dB' with 'db' (lines 1851-1871) and 'dgE' with 'dge' (lines 2380-2401) show identical expected outcomes for deleting text like 'hello.world next'. Given the distinction between 'word' and 'WORD' (where 'WORD' treats punctuation as part of the word), it's worth verifying if the expected behavior for deletion is truly identical in these cases, or if the tests should be adjusted to reflect a difference. </summary> ```go t.Run("test 'dB' vs 'db' on dotted text", func(t *testing.T) { lines := []string{"hello.world next"} // First test 'db' tm1 := newTestModelWithLinesAndCursorPos(t, lines, core.Position{Col: 15, Line: 0}) sendKeys(tm1, "d", "b") m1 := getFinalModel(t, tm1) if m1.ActiveBuffer().Lines[0].String() != "hello.world t" { t.Errorf("'db': Line(0) = %q, want 'hello.world t'", m1.ActiveBuffer().Lines[0].String()) } // Now test 'dB' tm2 := newTestModelWithLinesAndCursorPos(t, lines, core.Position{Col: 15, Line: 0}) sendKeys(tm2, "d", "B") m2 := getFinalModel(t, tm2) if m2.ActiveBuffer().Lines[0].String() != "hello.world t" { t.Errorf("'dB': Line(0) = %q, want 'hello.world t'", m2.ActiveBuffer().Lines[0].String()) } }) ``` </details> </td></tr> </table>

azpect added 1 commit 2026-04-03 15:20:54 -07:00

doc: added notes to qodo.md

Run Test Suite / test (push) Failing after 17s

Details

Run Test Suite / test (pull_request) Failing after 14s

Details

7ba94eaeea

azpect added 1 commit 2026-04-04 11:09:09 -07:00

feat: finally got the tests passing.

Run Test Suite / test (push) Successful in 19s

Details

Run Test Suite / test (pull_request) Successful in 16s

Details

8b7a479ecb

Most of the tests were just written poorly, the code was right. Though
the yank related questions were actually broken.

azpect added 1 commit 2026-04-04 11:10:02 -07:00

doc: updated FEATURES.md

Run Test Suite / test (push) Successful in 16s

Details

Run Test Suite / test (pull_request) Successful in 15s

Details

b23072b43f

azpect commented

2026-04-04 11:10:28 -07:00

/review

qodo-bot commented

2026-04-04 11:10:44 -07:00

Persistent review updated to latest commit 6b8845b245

**[Persistent review](https://git.gophernest.net/azpect/Gim/pulls/4#issuecomment-875)** updated to latest commit https://git.gophernest.net/azpect/Gim/commit/6b8845b245724555a647fe1852db4572b605c3b5

azpect commented

2026-04-04 11:19:13 -07:00

/improve

qodo-bot commented

2026-04-04 11:19:17 -07:00

PR Code Suggestions ✨

Category	Suggestion	Impact
Possible issue	Set cursor to yank start The `yankNormalMode` function should explicitly set the cursor position to the start of the yanked region after the operation. Currently, the cursor remains at the position determined by the motion, which causes failing tests for `yB`, `yge`, and `ygE` as noted in `integration_motion_word_test.go`. internal/operator/yank.go [85-88] `endX = min(endX, len(line)) // Catch overflow cnt := line[startX:endX] m.UpdateDefaultRegister(core.CharwiseRegister, []string{cnt}) + win.SetCursorCol(startX) + win.SetCursorLine(start.Line)` Suggestion importance[1-10]: 9 __ Why: This suggestion correctly identifies and addresses a bug where the cursor position is not updated after a yank operation, as indicated by failing tests in `integration_motion_word_test.go`. Setting the cursor to the start of the yanked region is crucial for correct Vim behavior.	High
	Correct backward word end calculation The `prevWordEnd` function incorrectly identifies the end of the previous word. After skipping whitespace, the cursor `x` is positioned at the start of the target word, not its end. An additional step is needed to advance `x` to the last character of that word, based on its character class, to match Vim's `ge` behavior. internal/motion/word.go [519-539] // Skip whitespace backward, crossing lines if needed for { for x >= 0 && (line[x] == ' ' \|\| line[x] == '\t') { x-- } if x >= 0 { break // landed on a non-whitespace char, this is our word end! } if y == 0 { return 0, 0 } y-- line = buf.Line(y) x = len(line) - 1 if len(line) == 0 { return 0, y // empty line acts as a word boundary } } - // We're now at the end of the previous word - that's the answer! + // Now x,y is at the start of the target word. Move forward to its end. + if x >= 0 { // Ensure x is valid + if isWordChar(line[x]) { + for x+1 < len(line) && isWordChar(line[x+1]) { + x++ + } + } else if isWordPunctuation(line[x]) { + for x+1 < len(line) && isWordPunctuation(line[x+1]) { + x++ + } + } + } return x, y Suggestion importance[1-10]: 9 __ Why: The `prevWordEnd` function, as implemented, incorrectly positions the cursor at the start of the word after skipping whitespace. The suggestion provides a correct fix by adding logic to advance the cursor to the actual end of the word, which is essential for the `ge` motion to function as expected.	High
	Correct backward WORD end calculation The `prevWORDEnd` function also incorrectly identifies the end of the previous WORD. Similar to `prevWordEnd`, after skipping whitespace, the cursor `x` is at the start of the target WORD. An additional loop is required to advance `x` to the last character of that WORD, matching Vim's `gE` behavior. internal/motion/word.go [579-599] // Skip whitespace backward, crossing lines if needed for { for x >= 0 && (line[x] == ' ' \|\| line[x] == '\t') { x-- } if x >= 0 { break // landed on a non-whitespace char, this is our WORD end! } if y == 0 { return 0, 0 } y-- line = buf.Line(y) x = len(line) - 1 if len(line) == 0 { return 0, y // empty line acts as a word boundary } } - // We're now at the end of the previous WORD - that's the answer! + // Now x,y is at the start of the target WORD. Move forward to its end. + if x >= 0 { // Ensure x is valid + for x+1 < len(line) && line[x+1] != ' ' && line[x+1] != '\t' { + x++ + } + } return x, y Suggestion importance[1-10]: 9 __ Why: Similar to `prevWordEnd`, the `prevWORDEnd` function also incorrectly places the cursor at the start of the WORD. The suggested change correctly adds the necessary logic to move the cursor to the end of the WORD, ensuring the `gE` motion behaves as intended.	High

## PR Code Suggestions ✨  <table><thead><tr><td><strong>Category</strong></td><td align=left><strong>Suggestion                                                                                                                                    </strong></td><td align=center><strong>Impact</strong></td></tr><tbody><tr><td rowspan=3>Possible issue</td> <td> <details><summary>Set cursor to yank start</summary> ___ **The <code>yankNormalMode</code> function should explicitly set the cursor position to the start <br>of the yanked region after the operation. Currently, the cursor remains at the <br>position determined by the motion, which causes failing tests for <code>yB</code>, <code>yge</code>, and <code>ygE</code> <br>as noted in <code>integration_motion_word_test.go</code>.** [internal/operator/yank.go [85-88]](https://git.gophernest.net/azpect/Gim/src/branch/feature/word-motions/internal/operator/yank.go#L85-L88) ```diff endX = min(endX, len(line)) // Catch overflow cnt := line[startX:endX] m.UpdateDefaultRegister(core.CharwiseRegister, []string{cnt}) + win.SetCursorCol(startX) + win.SetCursorLine(start.Line) ``` <details><summary>Suggestion importance[1-10]: 9</summary> __ Why: This suggestion correctly identifies and addresses a bug where the cursor position is not updated after a yank operation, as indicated by failing tests in `integration_motion_word_test.go`. Setting the cursor to the start of the yanked region is crucial for correct Vim behavior. </details></details></td><td align=center>High </td></tr><tr><td> <details><summary>Correct backward word end calculation</summary> ___ **The <code>prevWordEnd</code> function incorrectly identifies the end of the previous word. After <br>skipping whitespace, the cursor <code>x</code> is positioned at the *start* of the target word, <br>not its end. An additional step is needed to advance <code>x</code> to the last character of that <br>word, based on its character class, to match Vim's <code>ge</code> behavior.** [internal/motion/word.go [519-539]](https://git.gophernest.net/azpect/Gim/src/branch/feature/word-motions/internal/motion/word.go#L519-L539) ```diff // Skip whitespace backward, crossing lines if needed for { for x >= 0 && (line[x] == ' ' || line[x] == '\t') { x-- } if x >= 0 { break // landed on a non-whitespace char, this is our word end! } if y == 0 { return 0, 0 } y-- line = buf.Line(y) x = len(line) - 1 if len(line) == 0 { return 0, y // empty line acts as a word boundary } } - // We're now at the end of the previous word - that's the answer! + // Now x,y is at the start of the target word. Move forward to its end. + if x >= 0 { // Ensure x is valid + if isWordChar(line[x]) { + for x+1 < len(line) && isWordChar(line[x+1]) { + x++ + } + } else if isWordPunctuation(line[x]) { + for x+1 < len(line) && isWordPunctuation(line[x+1]) { + x++ + } + } + } return x, y ``` <details><summary>Suggestion importance[1-10]: 9</summary> __ Why: The `prevWordEnd` function, as implemented, incorrectly positions the cursor at the start of the word after skipping whitespace. The suggestion provides a correct fix by adding logic to advance the cursor to the actual end of the word, which is essential for the `ge` motion to function as expected. </details></details></td><td align=center>High </td></tr><tr><td> <details><summary>Correct backward WORD end calculation</summary> ___ **The <code>prevWORDEnd</code> function also incorrectly identifies the end of the previous WORD. <br>Similar to <code>prevWordEnd</code>, after skipping whitespace, the cursor <code>x</code> is at the *start* of <br>the target WORD. An additional loop is required to advance <code>x</code> to the last character <br>of that WORD, matching Vim's <code>gE</code> behavior.** [internal/motion/word.go [579-599]](https://git.gophernest.net/azpect/Gim/src/branch/feature/word-motions/internal/motion/word.go#L579-L599) ```diff // Skip whitespace backward, crossing lines if needed for { for x >= 0 && (line[x] == ' ' || line[x] == '\t') { x-- } if x >= 0 { break // landed on a non-whitespace char, this is our WORD end! } if y == 0 { return 0, 0 } y-- line = buf.Line(y) x = len(line) - 1 if len(line) == 0 { return 0, y // empty line acts as a word boundary } } - // We're now at the end of the previous WORD - that's the answer! + // Now x,y is at the start of the target WORD. Move forward to its end. + if x >= 0 { // Ensure x is valid + for x+1 < len(line) && line[x+1] != ' ' && line[x+1] != '\t' { + x++ + } + } return x, y ``` <details><summary>Suggestion importance[1-10]: 9</summary> __ Why: Similar to `prevWordEnd`, the `prevWORDEnd` function also incorrectly places the cursor at the start of the WORD. The suggested change correctly adds the necessary logic to move the cursor to the end of the WORD, ensuring the `gE` motion behaves as intended. </details></details></td><td align=center>High </td></tr></tr></tbody></table>

azpect added 1 commit 2026-04-04 11:38:38 -07:00

fix: updated via using feedback from Qodo

Run Test Suite / test (push) Successful in 20s

Details

Run Test Suite / test (pull_request) Successful in 15s

Details

1166a67c64

azpect merged commit a9dd5c008f into master

2026-04-04 11:43:47 -07:00

azpect deleted branch feature/word-motions

2026-04-04 11:43:47 -07:00

azpect referenced this issue from a commit

2026-04-04 11:43:47 -07:00

Merge pull request 'feat: adding word actions, not done, and lots of failing tests' (#4) from feature/word-motions into master

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: azpect/Gim#4

feat: adding word actions, not done, and lots of failing tests #4

PR Reviewer Guide 🔍

(Review updated until commit 6b8845b245)

PR Code Suggestions ✨

(Review updated until commit `6b8845b245`)