From 277b071336268e2fa241e535ed49ce87a0842d54 Mon Sep 17 00:00:00 2001 From: sharpchen Date: Wed, 23 Jul 2025 07:00:58 +0800 Subject: [PATCH 1/3] Fix that ViFindBrace doesn't search for brace when current char is not a brace --- PSReadLine/Movement.vi.cs | 23 ++++++++++++++++- test/MovementTest.VI.cs | 52 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 72 insertions(+), 3 deletions(-) diff --git a/PSReadLine/Movement.vi.cs b/PSReadLine/Movement.vi.cs index 011c146d9..01bad815e 100644 --- a/PSReadLine/Movement.vi.cs +++ b/PSReadLine/Movement.vi.cs @@ -254,7 +254,28 @@ private int ViFindBrace(int i) case ')': return ViFindBackward(i, '(', withoutPassing: ')'); default: - return i; + ReadOnlySpan 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; + + Outer: + int match = _buffer[nextParen] switch + { + // if next is opening, find forward + '{' => ViFindForward(nextParen, '}', withoutPassing: '{'), + '[' => ViFindForward(nextParen, ']', withoutPassing: '['), + '(' => ViFindForward(nextParen, ')', withoutPassing: '('), + // if next is closing, find backward + '}' => ViFindBackward(nextParen, '{', withoutPassing: '}'), + ']' => ViFindBackward(nextParen, '[', withoutPassing: ']'), + ')' => ViFindBackward(nextParen, '(', withoutPassing: ')'), + _ => nextParen + }; + + return match == nextParen ? i : match; } } diff --git a/test/MovementTest.VI.cs b/test/MovementTest.VI.cs index 3c4573ebe..30d4d0e95 100644 --- a/test/MovementTest.VI.cs +++ b/test/MovementTest.VI.cs @@ -1,4 +1,5 @@ -using Xunit; +using System.Collections.Generic; +using Xunit; namespace Test { @@ -370,7 +371,7 @@ public void ViGlobMovement_EmptyBuffer_Defect1195() TestSetup(KeyMode.Vi); TestMustDing("", Keys( - _.Escape, "W" + _.Escape, "W" )); } @@ -454,6 +455,53 @@ public void ViGotoBrace() )); } + // tests when cursor not on any paren + foreach (var (opening, closing) in new[] { ('(', ')'), ('{', '}'), ('[', ']') }) + { + // closing paren with backward match + string input1 = $"0{opening}2{opening}4foo{closing}"; + Test(input1, Keys( + input1, + CheckThat(() => AssertCursorLeftIs(9)), + _.Escape, CheckThat(() => AssertCursorLeftIs(8)), + "0ff", CheckThat(() => AssertCursorLeftIs(5)), + _.Percent, CheckThat(() => AssertCursorLeftIs(3)), + _.Percent, CheckThat(() => AssertCursorLeftIs(8)) + )); + + // 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)) + )); + + // opening paren with forward match + string input3 = $"0{opening}2foo6{closing}"; + Test(input3, Keys( + input3, + CheckThat(() => AssertCursorLeftIs(8)), + _.Escape, CheckThat(() => AssertCursorLeftIs(7)), + "0ff", CheckThat(() => AssertCursorLeftIs(3)), + _.Percent, CheckThat(() => AssertCursorLeftIs(1)), + _.Percent, CheckThat(() => AssertCursorLeftIs(7)) + )); + // opening paren without forward match + string input4 = $"0)2]4foo{opening}("; + Test(input4, Keys( + input4, + CheckThat(() => AssertCursorLeftIs(10)), + _.Escape, CheckThat(() => AssertCursorLeftIs(9)), + "0ff", CheckThat(() => AssertCursorLeftIs(5)), + _.Percent, CheckThat(() => AssertCursorLeftIs(5)), // stay still + _.Percent, CheckThat(() => AssertCursorLeftIs(5)) + )); + } + // <%> with empty text buffer should work fine. Test("", Keys( _.Escape, _.Percent, From 85ec2c28afe9d40f1cf0d879e6959b3946e0e382 Mon Sep 17 00:00:00 2001 From: Andy Jordan <2226434+andyleejordan@users.noreply.github.com> Date: Tue, 31 Mar 2026 17:03:27 -0700 Subject: [PATCH 2/3] Fix `ViGotoBrace` tests 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. --- test/MovementTest.VI.cs | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/test/MovementTest.VI.cs b/test/MovementTest.VI.cs index 30d4d0e95..f41daecfe 100644 --- a/test/MovementTest.VI.cs +++ b/test/MovementTest.VI.cs @@ -424,6 +424,11 @@ public void ViCursorMovement() [SkippableFact] public void ViGotoBrace() { + // NOTE: When the input has unmatched braces, in order to avoid an + // exception caused by AcceptLineImpl waiting for incomplete input, + // the test needs to end with the Vi command "ddi" and assert that + // the result is an empty string. + TestSetup(KeyMode.Vi); Test("0[2(4{6]8)a}c", Keys( @@ -451,25 +456,26 @@ public void ViGotoBrace() CheckThat(() => AssertCursorLeftIs(4)), _.Percent, CheckThat(() => AssertCursorLeftIs(4)), - "ddi" + "ddi" // Unmatched brace )); } - // tests when cursor not on any paren + // Tests when the cursor is not on any paren foreach (var (opening, closing) in new[] { ('(', ')'), ('{', '}'), ('[', ']') }) { - // closing paren with backward match + // Closing paren with backward match string input1 = $"0{opening}2{opening}4foo{closing}"; - Test(input1, Keys( + Test("", Keys( input1, CheckThat(() => AssertCursorLeftIs(9)), _.Escape, CheckThat(() => AssertCursorLeftIs(8)), "0ff", CheckThat(() => AssertCursorLeftIs(5)), _.Percent, CheckThat(() => AssertCursorLeftIs(3)), - _.Percent, CheckThat(() => AssertCursorLeftIs(8)) + _.Percent, CheckThat(() => AssertCursorLeftIs(8)), + "ddi" // Unmatched closing brace )); - // closing paren without backward match + // Closing paren without backward match string input2 = $"0]2)4foo{closing}"; Test(input2, Keys( input2, @@ -480,7 +486,7 @@ public void ViGotoBrace() _.Percent, CheckThat(() => AssertCursorLeftIs(5)) )); - // opening paren with forward match + // Opening paren with forward match string input3 = $"0{opening}2foo6{closing}"; Test(input3, Keys( input3, @@ -490,15 +496,17 @@ public void ViGotoBrace() _.Percent, CheckThat(() => AssertCursorLeftIs(1)), _.Percent, CheckThat(() => AssertCursorLeftIs(7)) )); - // opening paren without forward match + + // Opening paren without forward match string input4 = $"0)2]4foo{opening}("; - Test(input4, Keys( + TestMustDing("", Keys( input4, CheckThat(() => AssertCursorLeftIs(10)), _.Escape, CheckThat(() => AssertCursorLeftIs(9)), "0ff", CheckThat(() => AssertCursorLeftIs(5)), _.Percent, CheckThat(() => AssertCursorLeftIs(5)), // stay still - _.Percent, CheckThat(() => AssertCursorLeftIs(5)) + _.Percent, CheckThat(() => AssertCursorLeftIs(5)), + "ddi" // Unmatched brace )); } From 7f5b4cba6ae984496f20795e2616a085f4402f53 Mon Sep 17 00:00:00 2001 From: sharpchen Date: Wed, 8 Apr 2026 09:21:15 +0800 Subject: [PATCH 3/3] Resolve `ViGotoBrace` Reviews --- PSReadLine/Movement.vi.cs | 10 +++++++--- test/MovementTest.VI.cs | 5 ++--- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/PSReadLine/Movement.vi.cs b/PSReadLine/Movement.vi.cs index 01bad815e..542a37b7d 100644 --- a/PSReadLine/Movement.vi.cs +++ b/PSReadLine/Movement.vi.cs @@ -254,12 +254,16 @@ private int ViFindBrace(int i) case ')': return ViFindBackward(i, '(', withoutPassing: ')'); default: - ReadOnlySpan parenthese = stackalloc char[] { '{', '}', '(', ')', '[', ']' }; + ReadOnlySpan parentheses = 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; + for (int idx = 0; idx < parentheses.Length; idx++) + if (parentheses[idx] == _buffer[nextParen]) goto Outer; + + // if not found, nextParen could exceed the range + if (nextParen >= _buffer.Length) + return i; Outer: int match = _buffer[nextParen] switch diff --git a/test/MovementTest.VI.cs b/test/MovementTest.VI.cs index f41daecfe..27dfe7956 100644 --- a/test/MovementTest.VI.cs +++ b/test/MovementTest.VI.cs @@ -1,5 +1,4 @@ -using System.Collections.Generic; -using Xunit; +using Xunit; namespace Test { @@ -477,7 +476,7 @@ public void ViGotoBrace() // Closing paren without backward match string input2 = $"0]2)4foo{closing}"; - Test(input2, Keys( + TestMustDing(input2, Keys( input2, CheckThat(() => AssertCursorLeftIs(9)), _.Escape, CheckThat(() => AssertCursorLeftIs(8)),