Fix(VIM-4030): Fix % motion ignoring braces inside string literals #1434
Conversation
The % motion and block text objects (va{, vi{) now correctly identify
matching braces when braces appear inside string literals. The fix
implements PSI-based string detection that works across different
languages (Java, Kotlin, Rust, etc.) by checking element type names.
Falls back to text-based detection when PSI is not available.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <[email protected]>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| !elementType.contains("INTEGER", ignoreCase = true) && | ||
| !elementType.contains("FLOAT", ignoreCase = true) && | ||
| !elementType.contains("DOUBLE", ignoreCase = true) && | ||
| !elementType.contains("CHAR_LITERAL", ignoreCase = true)) |
There was a problem hiding this comment.
Missing CHAR exclusion. CHAR_LITERAL exclusion exists but some languages use just CHAR (e.g., C++). This could incorrectly match character literals.
| return when (quoteChar) { | ||
| '"' -> text.startsWith("\"") || text.startsWith("@\"") || text.startsWith("r\"") || |
There was a problem hiding this comment.
Rust raw strings like r###"..."### aren't fully handled. The check only looks for r# prefix but Rust allows multiple # characters. findContentEnd also doesn't handle the matching ### suffix - it will only strip a single quote character.
| // Skip language-specific prefixes (r, R, f, b, @, $, etc.) | ||
| while (startIndex < text.length && text[startIndex] != '"' && text[startIndex] != '\'') { |
There was a problem hiding this comment.
Fragile prefix detection. Skips all non-quote characters but this could incorrectly skip past digits in edge cases like 123"text". Should validate that skipped characters are valid prefix characters (letters/symbols only).
| // Check for triple quotes | ||
| if (startIndex + 2 < text.length && |
There was a problem hiding this comment.
Off-by-one check. Should be startIndex + 2 <= text.length (not <). Current code requires 3 chars total but accesses text[startIndex + 2] which needs startIndex + 3 <= text.length.
| fun `test percent skips brace inside string literal`() { | ||
| configureByJavaText( | ||
| """ | ||
| void test() $c{ | ||
| String json = "{\"key\":\"value\"}"; | ||
| } | ||
| """.trimIndent() | ||
| ) | ||
| typeText("%") | ||
| assertState( | ||
| """ | ||
| void test() { | ||
| String json = "{\"key\":\"value\"}"; | ||
| $c} | ||
| """.trimIndent() | ||
| ) | ||
| } | ||
|
|
||
| @Test | ||
| @TestFor(issues = ["VIM-4030"]) | ||
| @TestWithoutNeovim(SkipNeovimReason.PSI) | ||
| fun `test percent skips brace inside string with unbalanced quotes`() { | ||
| // This test specifically targets the bug: the string contains a closing brace | ||
| // preceded by an unescaped quote which confuses text-based detection | ||
| configureByJavaText( | ||
| """ | ||
| void test() $c{ | ||
| String s = "{\"}"; | ||
| } | ||
| """.trimIndent() | ||
| ) | ||
| typeText("%") | ||
| assertState( | ||
| """ | ||
| void test() { | ||
| String s = "{\"}"; | ||
| $c} | ||
| """.trimIndent() | ||
| ) | ||
| } | ||
|
|
||
| @Test | ||
| @TestFor(issues = ["VIM-4030"]) | ||
| @TestWithoutNeovim(SkipNeovimReason.PSI) | ||
| fun `test percent skips brace inside text block`() { | ||
| // Java text blocks (triple-quoted strings) should be recognized via PSI | ||
| // The text-based detection would misinterpret the triple quotes | ||
| configureByJavaText( | ||
| """ | ||
| void test() $c{ | ||
| String json = ${"\"\"\""} | ||
| {"key":"value"} | ||
| ${"\"\"\""}; | ||
| } | ||
| """.trimIndent() | ||
| ) | ||
| typeText("%") | ||
| assertState( | ||
| """ | ||
| void test() { | ||
| String json = ${"\"\"\""} | ||
| {"key":"value"} | ||
| ${"\"\"\""}; | ||
| $c} | ||
| """.trimIndent() | ||
| ) | ||
| } | ||
|
|
There was a problem hiding this comment.
Missing test coverage:
- Escaped quotes:
"{\"}" - Nested braces:
"{{}}" - Empty strings:
"{}" - Multiple strings on same line
- Cursor positions (start/middle/end of string)
- Edge case: string at EOF without newline
Code ReviewOverall: Good solution using PSI for string detection. Fixes the core issue but has several bugs and gaps. Critical Issues
Quality Issues
Test CoverageTests only cover basic cases. Missing:
PerformancePSI tree walk on every % motion could be slow in large files. Consider caching or early termination. RecommendationFix critical bugs before merge. Test coverage acceptable for initial fix but should expand. |
1 participant
Summary
Fixes VIM-4030: The
%motion doesn't work when{}are separated by quotes.IjVimPsiServicethat correctly identifies when the cursor is inside a string literal%motion,va{, andvi{text objects to correctly ignore braces inside string literalsTest plan
%motion with braces inside Java string literals%motion with braces inside Java text blocksva{with braces inside string literalsvi{with braces inside string literalsWorkflow run
https://github.com/JetBrains/ideavim/actions/runs/20569002980
🤖 Generated with Claude Code