Skip to content

Conversation

@brasswood
Copy link
Contributor

@brasswood brasswood commented Oct 24, 2025

What this PR does / why we need it:
Make counted motions work when the result type is Position and the count is high enough that the motion fails eventually.

For motions with a count (e.g., 3[{), save the previous iteration's result when that result is a Position.

With a motion like 99[{, eventually (in typical files), an iteration will fail to find an unmatched { and therefore return prevResult. However, <count>[{ returns results of type Position, and therefore, prevResult was not being updated for it. This caused execActionWithCount to return prevResult's initial value of failedMovement. This in turn led to the cursor not moving at all, when it should have moved to the last unmatched {, i.e., the Position of the last successful iteration.

This commit fixes this issue by making sure prevResult is updated when result is a Position.
Which issue(s) this PR fixes
#9801

Special notes for your reviewer:
I don't actually know if it is intentional or not that prevResult was only updated when result was an IMovement. I hope I am not introducing wrong behavior with this. Hopefully the reviewer will know.

I tried writing test cases for this behavior, but I couldn't see how to call an action with a count from within the tests, so I have given up on it for now.

brasswood and others added 2 commits October 23, 2025 18:13
For motions with a count (e.g., 3[{), save the previous iteration's
result when that result is a Position.

With a motion like 99[{, eventually (in typical files), an iteration
will fail to find an unmatched { and therefore return prevResult.
However, <count>[{ returns results of type Position, and therefore,
prevResult was not being updated for it. This caused execActionWithCount
to return prevResult's initial value of failedMovement. This in turn led
to the cursor not moving at all, when it should have moved to the last
unmatched {, i.e., the Position of the last successful iteration.

This commit fixes this issue by making sure prevResult is updated when
result is a Position.
Copy link
Member

@J-Fields J-Fields left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like an oversight to me. Thanks!

@J-Fields J-Fields merged commit d9f6e96 into VSCodeVim:master Nov 5, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants