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

12 views
Skip to first unread message

Langning Zhang

unread,
Aug 12, 2026, 2:11:49 AMAug 12
to vim/vim, Subscribed

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)

Patch Links:

—
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

unread,
Aug 12, 2026, 3:43:06 PMAug 12
to vim/vim, Subscribed
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

unread,
Aug 12, 2026, 11:21:56 PMAug 12
to vim/vim, Subscribed
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

unread,
Aug 13, 2026, 12:24:25 AMAug 13
to vim/vim, Subscribed
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>

Langning Zhang

unread,
Sep 30, 2026, 2:10:31 PM (2 days ago) Sep 30
to vim/vim, Subscribed
Boulea7 left a comment (vim/vim#21023)

Could you share the first failing test or assertion from the normal GCC Linux job? Its check annotation only reports exit code 2. That would help me separate the CI failure from the crossing-semantics question above before revising the patch.

—
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/c5916970278@github.com>

Christian Brabandt

unread,
Sep 30, 2026, 6:05:37 PM (2 days ago) Sep 30
to vim/vim, Subscribed
chrisbra left a comment (vim/vim#21023)

This is the error:

Failures: 
	From test_visual.vim:
	Found errors in Test_visual_oper_pending_mode_maps():
	command line..script /home/runner/work/vim/vim/src/testdir/runtest.vim[636]..function RunTheTest[63]..Test_visual_oper_pending_mode_maps line 33: Expected 'LemonNewNewZ' but got 'LemoNewNewZ'

you should be able to reproduce this locally when running (from the testdir): rm -f test_visual.res; make test_visual.res
This is the function, the error seems to be on the last assert:

  " Operator-pending mode maps (movement and text object)
  "   - Simple
  "   - With Ex command moving the cursor
  "   - With Ex command and Visual selection (custom text object)
  func Test_visual_oper_pending_mode_maps()
    new
    call append(0, '')

    func MoveToCap()
      call search('\u', 'W')
    endfunction

    func SelectInCaps()
      let [line1, col1] = searchpos('\u', 'bcnW')
      let [line2, col2] = searchpos('.\u', 'nW')
      call setpos("'<", [0, line1, col1, 0])
      call setpos("'>", [0, line2, col2, 0])
      normal! gv
    endfunction

    onoremap W /\u/<CR>
    onoremap <Leader>W :<C-U>call MoveToCap()<CR>
    onoremap iW :<C-U>call SelectInCaps()<CR>

    call setline(1, 'PineappleQuinceLoganberryOrangeGrapefruitKiwiZ')
    call cursor(1, 1)
    exe "normal cW-\<Esc>l.l2.l."
    call assert_equal('----Z', getline(1))

    call setline(1, 'JuniperDurianZ')
    call cursor(1, 1)
    exe "normal g?\WfD."
    call assert_equal('WhavcreQhevnaZ', getline(1))

    call setline(1, 'LemonNectarineZ')
    call cursor(1, 1)
    exe "normal yiWPlciWNew\<Esc>fr."
    call assert_equal('LemonNewNewZ', getline(1))

    ounmap W
    ounmap <Leader>W
    ounmap iW
    bwipe!
    delfunc MoveToCap
    delfunc SelectInCaps
  endfunc

—
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/c5920524643@github.com>

Langning Zhang

unread,
Sep 30, 2026, 7:30:27 PM (2 days ago) Sep 30
to vim/vim, Subscribed
Boulea7 left a comment (vim/vim#21023)

I reproduced the line-33 failure at cc83269. Using fresh builds with the same configuration, Test_visual_oper_pending_mode_maps() passes on the base ecfea491 and fails on the PR head (LemoNewNewZ).

The first ciW already selects an extra character, before dot-repeat. From the previous range [1,5], setting '< to 6 gives [5,6]. Setting '> to 10 then selects the same storage slot again, yielding [5,10] rather than [6,10].

This also occurs outside an operator mapping, including after a reversed selection (v4lo instead of v4l). Source this script in vim --clean:

new
call setline(1, 'LemonLemonNectarineZ')
execute "normal! gg0v4l\<Esc>"
call setpos("'<", [0, 1, 6, 0])
call setpos("'>", [0, 1, 10, 0])
normal! gvy
echo getreg('"')

The base yanks Lemon; the head yanks nLemon.

Would you prefer preserving consecutive setters that construct [6,10] through stable endpoint identity, or following the current sorted marks on every setter, producing [5,10] and changing existing mapping usage? I recommend preserving the existing workflow. Would a broader change with explicit endpoint-identity metadata be acceptable?

—
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/c5921554107@github.com>

Langning Zhang

unread,
Oct 1, 2026, 2:16:01 PM (yesterday) Oct 1
to vim/vim, Subscribed
Boulea7 left a comment (vim/vim#21023)

I've updated the setters to keep the endpoint identities established by the saved Visual selection. Consecutive writes now preserve both endpoints, and the unchanged Test_visual_oper_pending_mode_maps() passes with LemonNewNewZ.

Getters remain ordered by position, so after endpoints cross, setpos(name, getpos(name)) can change the marks; that tradeoff is documented. Local marks, Visual, undo, and codestyle tests finished with 156 passes and 2 clipboard-feature skips. Actual old/new undo writers and all four reader combinations also passed, with legacy behavior preserved for older readers and files.

—
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/c5937640654@github.com>

Reply all
Reply to author
Forward
0 new messages