[vim/vim] setpos() updates the wrong logical Visual mark (PR #21023)

[Langning Zhang]

Problem

For a reversed Visual selection, getpos("'<") reports the buffer-relative start while setpos("'<", ...) updates the endpoint associated with the selection direction. setcharpos() follows the same path, so the get/set APIs are inconsistent. Fixes #19049.

Solution

Resolve '< and '> to the logical buffer-relative endpoints before updating them. When a new endpoint crosses the other, clamp the range; when one endpoint is deleted, preserve the remaining endpoint so the range can be recreated. Update builtin.txt and add regression coverage for forward and reverse selections, crossing, deletion and recreation, independent initialization, and setcharpos().

Tests

• make -C src
• make -C src/testdir -W test_marks.vim -W test_visual.vim -W test_codestyle.vim test_marks.res test_visual.res test_codestyle.res
• git diff --check

Focused test counts: test_marks 14/14, test_visual 81/81, test_codestyle 5/5.

AI-assisted: Codex

---

You can view, comment on, or merge this pull request online at:

  https://github.com/vim/vim/pull/21023

Commit Summary

• e218171 patch 9.2.XXXX: setpos() updates the wrong logical Visual mark

File Changes

(3 files)

• M runtime/doc/builtin.txt (16)
• M src/mark.c (42)
• M src/testdir/test_marks.vim (71)

Patch Links:

• https://github.com/vim/vim/pull/21023.patch
• https://github.com/vim/vim/pull/21023.diff

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/21023@github.com>

[Christian Brabandt]

chrisbra left a comment (vim/vim#21023)

Thanks, but that visual marks are clamped is not correct I think. Take the following test:

diff --git a/src/testdir/test_visual.vim b/src/testdir/test_visual.vim
index beea4c431..f2e7a1489 100644
--- a/src/testdir/test_visual.vim
+++ b/src/testdir/test_visual.vim
@@ -3065,4 +3065,14 @@ func Test_visual_ended_in_unloaded_buffer()
   %bw!
 endfunc

+func Test_visual_clamp()
+  new
+  call setline(1, ['one', 'two', 'three'])
+  normal! 2GVjy
+  call setpos("'>", [0, 1, 1, 0])
+  '<,'>d
+  call assert_equal(['three'], getline(1,'$'))
+  bw!
+endfunc
+
 " vim: shiftwidth=2 sts=2 expandtab

clamping here means after setting the '> mark both visual marks point to line 1 and we suddenly lost the original visual selection, which means we are deleting an unexpected range. That is too much surprising and I think we should not do this.

Is there a reason we need to do the clamping on setting the mark? I believe we don't need to do it and leave it to getmark() to return the marks in the correct order.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/21023/c5271974570@github.com>

[Langning Zhang]

Boulea7 left a comment (vim/vim#21023)

You're right. Your example exposed a bug in the clamping logic: moving '> before '< discarded the previous endpoint. I removed the clamping. setpos() now resolves the requested logical mark and updates only that endpoint, while getmark() handles the ordering.

I added your linewise case and the inverse crossing case; both now preserve the full range.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/21023/c5275551367@github.com>

[Langning Zhang]

Boulea7 left a comment (vim/vim#21023)

I found an existing compatibility constraint while testing the revised approach. Consider a reversed stored selection with endpoints 7,1:

• setpos("\x27<", 2) followed by setpos("\x27>", 8) would ideally produce logical range 2..8.
• Existing custom text objects also replace both endpoints consecutively. Moving the same selection wholly right (setpos("\x27<", 8), then setpos("\x27>", 10)) must produce 8..10.

After the first call, both cases are reversed stored endpoints followed by setting \x27> beyond the high endpoint. Without tracking call history, choosing the logical endpoint preserves the first case but breaks the existing pair-replacement behavior; choosing the stored endpoint does the reverse.

Which behavior should take precedence when a call crosses the old range? I can keep this fix limited to non-crossing updates (the original report), or make logical endpoint naming take precedence across crossings.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/21023/c5275935467@github.com>