Fix VIM-2656: Incorrect shift motion behavior with tabs

[claude]

Summary

Fixes VIM-2656: >> command now correctly adds full tab indents instead of creating mixed tabs/spaces when using tab-based indentation.

Problem

When projects used tabs for indentation, the >> command would sometimes insert partial indents (mixed tabs and spaces) instead of full tab-based indents. This occurred when lines had existing misaligned whitespace.

For example, with 2 spaces of existing indent, >> would produce 1 tab + 2 spaces instead of rounding to 1 full tab.

Root Cause

The indentRange method in VimChangeGroupBase.kt calculated new indent as currentColumn + indentSize, which preserved any misalignment from existing indentation. This resulted in column positions that weren't multiples of tabSize, causing createIndentBySize to generate mixed tabs and spaces.

Solution

Changed the indentation logic to work with indent depth (levels) instead of raw column positions:

1. Calculate current indent depth: currentDepth = col / indentSize (floor division)
2. Calculate new depth after shift: newDepth = currentDepth + shiftCount
3. Create indent string: createIndentByDepth(newDepth)

This ensures indents always align to proper indent boundaries, so when using tabs, >> always adds full tab indents rather than creating "half-indented" lines.

Testing

• Added new test file ShiftRightTabsTest.kt with tests for tab-based indentation
• Tests verify that misaligned spaces get rounded to proper tab boundaries
• All existing shift tests continue to pass

Test plan

• Verify existing shift tests pass
• New test demonstrates the bug is fixed
• Test with tabs-only indentation settings
• Test with mixed starting indentation

🤖 Generated with Claude Code

[claude]

Good practice: Moving the indentSize calculation outside the loop is a nice optimization. This value is constant for all lines being shifted, so computing it once improves performance.

[claude]

Question: Integer division here will always floor the result, which is correct for the fix. However, could you add a brief comment explaining why we use integer division (floor) rather than rounding? This would help future maintainers understand that misaligned indents should round down to the previous indent level before shifting.

Example: With indentSize=4 and col=2, we get currentDepth=0 (not 0.5 rounded), which is the intended behavior.

[claude]

Excellent test: This is the key test case that demonstrates the bug fix. Starting with 2 spaces of misaligned indent and expecting it to round to 1 full tab (not 1 tab + 2 spaces) is exactly what VIM-2656 was about.

Suggestion for additional edge case: Consider adding a test where TAB_SIZE != INDENT_SIZE (e.g., TAB_SIZE=8, INDENT_SIZE=4). This is a valid configuration and would verify the fix works correctly when indent depth and tab rendering differ.

[claude]

Test coverage note: All tests use TAB_SIZE = 4 and INDENT_SIZE = 4. While this covers the common case, consider adding at least one test with different values (e.g., TAB_SIZE=8, INDENT_SIZE=4) to ensure the fix works correctly when these values differ.

This configuration is valid and used in some codebases where you want 4-space indent levels but tabs render as 8 spaces.

[claude]

PR Review Summary

This is a well-implemented fix for VIM-2656. The approach is sound, and the code changes are minimal and focused. Here's my detailed assessment:

✅ Strengths

1. Correct Root Cause Analysis: The PR correctly identifies that the bug was caused by calculating indents as currentColumn + indentSize, which preserved misalignment from existing indentation.

2. Clean Solution: The fix elegantly shifts from column-based to depth-based calculations:

   • currentDepth = col / indentSize (floor division)
   • newDepth = currentDepth + dir * count
   • createIndentByDepth(newDepth)

   This ensures indents always align to proper boundaries.

3. Comprehensive Test Coverage: The new test file covers:

   • Shifting with existing tab indent
   • Shifting from no indent
   • Multiple consecutive shifts
   • The key case: Misaligned spaces being normalized (VIM-2656)

4. Performance Optimization: Moving indentSize calculation outside the loop (vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimChangeGroupBase.kt:2001) is a nice touch.

5. Well-Documented: The PR description clearly explains the problem, root cause, and solution with examples.

🔍 Areas for Improvement

1. Missing Test Edge Case

All tests use TAB_SIZE = 4 and INDENT_SIZE = 4. Consider adding a test with different values (e.g., TAB_SIZE=8, INDENT_SIZE=4). This is a valid configuration used in some codebases and would verify the fix works when indent depth and tab rendering differ.

2. Code Comment for Integer Division

At vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimChangeGroupBase.kt:2008, the integer division behavior (floor) is critical to the fix. A brief comment would help future maintainers understand why misaligned indents round down before shifting.

3. Shift Left Coverage

The PR only adds tests for shift right (>>). While the same code path handles shift left (<<), adding at least one test for << with tabs would provide better coverage.

🛡️ Quality Assessment

• Code Quality: ✅ Excellent - minimal changes, clear logic
• Bug Fix Correctness: ✅ Correct - addresses the root cause
• Test Coverage: ⚠️ Good, but missing edge cases (different TAB_SIZE/INDENT_SIZE, shift left)
• Performance: ✅ No concerns, includes minor optimization
• Security: ✅ No security concerns
• Best Practices: ✅ Follows IdeaVim conventions

📋 Recommendation

APPROVE with minor suggestions. The core fix is correct and well-tested. The suggested improvements are nice-to-haves that would make the PR even stronger, but they don't block merging.

The fix correctly implements Vim's behavior of normalizing misaligned indents to proper boundaries when shifting, which is exactly what VIM-2656 requested.