Skip to content

Fix that ViFindBrace doesn't search for brace when current char is not a brace#4862

Open
sharpchen wants to merge 1 commit intoPowerShell:masterfrom
sharpchen:vifindbrace
Open

Fix that ViFindBrace doesn't search for brace when current char is not a brace#4862
sharpchen wants to merge 1 commit intoPowerShell:masterfrom
sharpchen:vifindbrace

Conversation

@sharpchen
Copy link

@sharpchen sharpchen commented Jul 22, 2025

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):
foo

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
Copy link

Aston32 commented Mar 17, 2026

S

@andyleejordan
Copy link
Member

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
Copy link
Author

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
Copy link
Author

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.

// 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;
Copy link
Member

Choose a reason for hiding this comment

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

The rare great use of goto

@andyleejordan
Copy link
Member

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.

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

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
Copy link
Member

Looks like tests are only available on Windows.

I forgot about that but yes this is unfortunately true.

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.

3 participants