Fix that ViFindBrace doesn't search for brace when current char is not a brace by sharpchen · Pull Request #4862 · PowerShell/PSReadLine

sharpchen · 2025-07-22T23:11:27Z

PR Summary

Vi searches right pair of first left brace, or first right brace when current char is not a brace. This pr added this functionality.

After the update, ViGotoBrace should work like this(of course this will also affect other methods such as ViDeleteBrace):

PR Checklist

PR has a meaningful title
- Use the present tense and imperative mood when describing your changes
Summarized changes
Make sure you've added one or more new tests
Make sure you've tested these changes in terminals that PowerShell is commonly used in (i.e. conhost.exe, Windows Terminal, Visual Studio Code Integrated Terminal, etc.)
User-facing changes
- Not Applicable
- OR
- Documentation needed at PowerShell-Docs
  - Doc Issue filed:

Microsoft Reviewers: Open in CodeFlow

Aston32 · 2026-03-17T23:37:31Z

S

andyleejordan · 2026-03-18T01:40:58Z

You are right that's how it works. I'm hoping we can do two things: verify the order of precedence in vanilla Vi (I was playing around trying to determine it and couldn't make heads or tails of it), and add some unit tests. Thanks! Love the fix though.

sharpchen · 2026-03-18T14:58:56Z

verify the order of precedence in vanilla Vi

Not sure what it means? If you mean the standard behavior, :h % documented the expected behavior in vim:

Find the next item in this line after or under the cursor and jump to its match.

And :h cpo-% described how the Vi behavior differs the way of Vim enhanced:

Parens inside single and double quotes are also counted, causing a string that contains a paren to disturb the matching. For example, in a line like "if (strcmp("foo(", s))" the first paren does not match the last one.

It indeed requires some tests. I did find at least one inconsistent behavior.

sharpchen · 2026-03-18T19:14:56Z

Hi @andyleejordan. I've rewrote the implementation, I think it's now good to go.
But I fail to run the test using ./build.ps1 -Test, it seems test/MovementTest.VI.cs is ignored? Tests pass even on false conditions.

andyleejordan · 2026-03-18T20:12:00Z

PSReadLine/Movement.vi.cs

+                    // find next of any kind of paren
+                    for (; nextParen < _buffer.Length; nextParen++)
+                        for (int idx = 0; idx < parenthese.Length; idx++)
+                            if (parenthese[idx] == _buffer[nextParen]) goto Outer;


The rare great use of goto

andyleejordan · 2026-03-18T20:15:49Z

I'm not sure but if this build is like others (looking at PSScriptAnalyzer right now), -Test won't rebuild so you have to run ./build.ps1 after any change and then follow it with ./build.ps1 -Test.

AppVeyor is at least running tests, looks like Test.en_US_Windows.ViGotoBrace and Test.ScreenReader.ViGotoBrace need to be checked and updated.

…t a brace

sharpchen · 2026-03-19T15:17:37Z

Looks like tests are only available on Windows.

Shouldn't call ReadKey when there are no more keys

I honestly have no idea why test's failing. There's is a input but claims no key.

andyleejordan · 2026-03-19T16:20:28Z

Looks like tests are only available on Windows.

I forgot about that but yes this is unfortunately true.

andyleejordan · 2026-03-31T20:19:18Z

@sharpchen I will get this built on Windows and see if I can't fix the test so we can get this in!

andyleejordan · 2026-04-01T00:11:42Z

Claude Opus 4.6 was actualy very adapt at fixing this error (it helps that the test was failing for the same reason as other similar tests, which then already had a fix it applied).

These tests need to be cleared with `ddi` and then asserted to be empty. It's the same fix for the other tests whose input contains unmatched braces. Copilot's plausible explanation is that the unmatched braces cause the input to be flagged by PowerShell's parser as incomplete, so `AcceptLineImpl` instead waits for more input, causing the exception.

andyleejordan

LGTM let's get it in @daxian-dbw

Copilot

Pull request overview

This PR updates vi-mode brace matching so that when the cursor is not on a brace/paren/bracket, % searches forward for the next one and then jumps to its match (affecting ViGotoBrace, ViDeleteBrace, etc.).

Changes:

Extend ViFindBrace to scan forward for the next { } ( ) [ ] when the current character is not a brace.
Add new ViGotoBrace test cases covering “cursor not on a brace” scenarios and unmatched-brace behavior.
Minor test-file formatting/comment adjustments.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File	Description
PSReadLine/Movement.vi.cs	Implements the new “scan forward for next brace” behavior in `ViFindBrace`.
test/MovementTest.VI.cs	Adds/extends xUnit coverage for `%` behavior when the cursor is not positioned on a brace.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-06T17:20:42Z

PSReadLine/Movement.vi.cs

+                        for (int idx = 0; idx < parenthese.Length; idx++)
+                            if (parenthese[idx] == _buffer[nextParen]) goto Outer;
+
+                Outer:


In the default case, if no brace/paren/bracket is found from the current position to the end of the buffer, nextParen will end up equal to _buffer.Length, and _buffer[nextParen] will throw IndexOutOfRangeException. Add an explicit check after the scan (e.g., if nextParen >= _buffer.Length return i) before indexing into _buffer.

Suggested change

Outer:

Outer:

if (nextParen >= _buffer.Length)

{

return i;

}

In the default case, if no brace/paren/bracket is found from the current position to the end of the buffer, nextParen will end up equal to _buffer.Length, and _buffer[nextParen] will throw IndexOutOfRangeException

This is a real problem. I prefer to not using the Outer label here. For the goto statement, we should avoid using it unless it's really needed.

Copilot · 2026-04-06T17:20:43Z

PSReadLine/Movement.vi.cs

+                    ReadOnlySpan<char> parenthese = stackalloc char[] { '{', '}', '(', ')', '[', ']' };
+                    int nextParen = i;
+                    // find next of any kind of paren
+                    for (; nextParen < _buffer.Length; nextParen++)
+                        for (int idx = 0; idx < parenthese.Length; idx++)
+                            if (parenthese[idx] == _buffer[nextParen]) goto Outer;


Variable name parenthese appears to be a typo (should be parentheses or similar). Renaming would improve readability and avoid spreading the misspelling.

Suggested change

ReadOnlySpan<char> parenthese = stackalloc char[] { '{', '}', '(', ')', '[', ']' };

int nextParen = i;

// find next of any kind of paren

for (; nextParen < _buffer.Length; nextParen++)

for (int idx = 0; idx < parenthese.Length; idx++)

if (parenthese[idx] == _buffer[nextParen]) goto Outer;

ReadOnlySpan<char> parentheses = stackalloc char[] { '{', '}', '(', ')', '[', ']' };

int nextParen = i;

// find next of any kind of paren

for (; nextParen < _buffer.Length; nextParen++)

for (int idx = 0; idx < parentheses.Length; idx++)

if (parentheses[idx] == _buffer[nextParen]) goto Outer;

Copilot · 2026-04-06T17:20:43Z

test/MovementTest.VI.cs

+using System.Collections.Generic;
+using Xunit;


using System.Collections.Generic; appears unused in this file (no List, Dictionary, etc.). Please remove it to avoid compiler warnings and keep usings minimal.

Suggested change

using System.Collections.Generic;

using Xunit;

using Xunit;

Copilot · 2026-04-06T17:20:43Z

test/MovementTest.VI.cs

+                // Closing paren without backward match
+                string input2 = $"0]2)4foo{closing}";
+                Test(input2, Keys(
+                    input2,
+                    CheckThat(() => AssertCursorLeftIs(9)),
+                    _.Escape, CheckThat(() => AssertCursorLeftIs(8)),
+                    "0ff", CheckThat(() => AssertCursorLeftIs(5)),
+                    _.Percent, CheckThat(() => AssertCursorLeftIs(5)), // stay still
+                    _.Percent, CheckThat(() => AssertCursorLeftIs(5))
+                ));


In the "Closing paren without backward match" case, % should still trigger a ding (consistent with other unmatched-brace cases in this test and with ViGotoBrace dinging when ViFindBrace returns the current position). Consider using TestMustDing here so the test actually asserts the expected ding behavior, not just that the cursor stays still.

PSReadLine/Movement.vi.cs

daxian-dbw · 2026-04-06T17:51:31Z

PSReadLine/Movement.vi.cs

+                    for (; nextParen < _buffer.Length; nextParen++)
+                        for (int idx = 0; idx < parenthese.Length; idx++)


We can find the next parenthesis by the code below, instead of using nested loop here:

ReadOnlySpan<char> parens = stackalloc char[] { '{', '}', '(', ')', '[', ']' }; // Find next parenthesis of any type int idx = _buffer.AsSpan(i).IndexOfAny(parens); if (idx < 0) { // No parenthesis, so nothing to match return i; } int nextParen = i + rel; int match = _buffer[nextParen] switch { ... } return match == nextParen ? i : match;

There's no AsSpan extension for StringBuilder though, did I miss something?
If that's true I think goto is succinct choice here to avoid extra boolean flag, we just need one more guard for the implicit increment on exit.

Right, I incorrectly thought _buffer was a string.
Then I'm fine keeping the goto. You just need to add an early return before int match = _buffer[nextParen] switch.

sharpchen force-pushed the vifindbrace branch from 4137b4e to 7d07953 Compare March 18, 2026 18:28

sharpchen force-pushed the vifindbrace branch from 7d07953 to 93916be Compare March 18, 2026 19:29

andyleejordan reviewed Mar 18, 2026

View reviewed changes

sharpchen force-pushed the vifindbrace branch 3 times, most recently from 611243c to fee3566 Compare March 18, 2026 23:26

Fix that ViFindBrace doesn't search for brace when current char is no…

277b071

…t a brace

sharpchen force-pushed the vifindbrace branch from fee3566 to 277b071 Compare March 18, 2026 23:31

andyleejordan force-pushed the vifindbrace branch from 48bf1d5 to 85ec2c2 Compare April 1, 2026 00:18

andyleejordan approved these changes Apr 1, 2026

View reviewed changes

daxian-dbw requested a review from Copilot April 6, 2026 17:17

Copilot started reviewing on behalf of daxian-dbw April 6, 2026 17:18 View session

Copilot AI reviewed Apr 6, 2026

View reviewed changes

daxian-dbw requested changes Apr 6, 2026

View reviewed changes

microsoft-github-policy-service bot added the Needs-Author Feedback label Apr 6, 2026

Merge branch 'PowerShell:master' into vifindbrace

0c3f945

microsoft-github-policy-service bot removed the Needs-Author Feedback label Apr 7, 2026

-                Outer:
+                Outer:
+                    if (nextParen >= _buffer.Length)
+                    {
+                        return i;
+                    }

		for (; nextParen < _buffer.Length; nextParen++)
		for (int idx = 0; idx < parenthese.Length; idx++)

Conversation

sharpchen commented Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Checklist

Microsoft Reviewers: Open in CodeFlow

Uh oh!

Aston32 commented Mar 17, 2026

Uh oh!

andyleejordan commented Mar 18, 2026

Uh oh!

sharpchen commented Mar 18, 2026

Uh oh!

sharpchen commented Mar 18, 2026

Uh oh!

andyleejordan Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

andyleejordan commented Mar 18, 2026

Uh oh!

sharpchen commented Mar 19, 2026

Uh oh!

andyleejordan commented Mar 19, 2026

Uh oh!

andyleejordan commented Mar 31, 2026

Uh oh!

andyleejordan commented Apr 1, 2026

Uh oh!

andyleejordan left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

daxian-dbw Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

daxian-dbw Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

sharpchen Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

daxian-dbw Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sharpchen commented Jul 22, 2025 •

edited

Loading

daxian-dbw Apr 7, 2026 •

edited

Loading