Skip to content

Maintenance: VimScript Variables - Convert to objects and improve documentation#1573

Open
claude[bot] wants to merge 3 commits intomasterfrom
maintenance/variable-objects
Open

Maintenance: VimScript Variables - Convert to objects and improve documentation#1573
claude[bot] wants to merge 3 commits intomasterfrom
maintenance/variable-objects

Conversation

@claude
Copy link

@claude claude bot commented Feb 10, 2026

Summary

  • Converted HighLightVariable and RegisterVariable from classes to object singletons
  • Added comprehensive KDoc documentation to both variables
  • Updated VimVariableServiceBase to use direct object references instead of instantiation

Area Inspected

vim-engine/src/main/kotlin/com/maddyhome/idea/vim/vimscript/model/variables/HighLightVariable.kt and related Variable implementations

Issues Found

  1. Inconsistent singleton pattern: HighLightVariable and RegisterVariable were classes but had no mutable state, yet were being instantiated on every use
  2. Missing documentation: HighLightVariable lacked KDoc comments explaining its purpose
  3. Wasteful instantiation: VimVariableServiceBase created new instances with HighLightVariable() and RegisterVariable() on every evaluation

Changes Made

1. Convert HighLightVariable to object and add documentation

  • Changed from class to internal object since it has no state
  • Added comprehensive KDoc explaining what v:hlsearch represents, its behavior, and reference to :help v:hlsearch
  • Aligns with the pattern used by KeyVariable and ValueVariable

2. Convert RegisterVariable to object and improve documentation

  • Changed from class to internal object for consistency
  • Enhanced KDoc to explicitly mention v:register and added reference to :help v:register
  • Maintains existing functionality while eliminating unnecessary instantiation

3. Update VimVariableServiceBase to use object references

  • Changed from HighLightVariable().evaluate(...) to HighLightVariable.evaluate(...)
  • Changed from RegisterVariable().evaluate(...) to RegisterVariable.evaluate(...)
  • Matches the pattern already used for KeyVariable and ValueVariable

Why These Changes Improve the Code

  1. Performance: Eliminates unnecessary object allocation on every variable evaluation
  2. Consistency: All Variable implementations now follow the same pattern - objects for stateless variables, objects with mutable state for stateful ones
  3. Maintainability: Better documentation helps future contributors understand what these variables represent
  4. Kotlin best practices: Using objects for stateless implementations is idiomatic Kotlin

Testing

Existing tests in HighLightVariableTest and RegisterVariableTest should continue to pass unchanged. The functionality remains identical - only the implementation pattern changed.

🤖 Generated with Claude Code

github-actions bot and others added 3 commits February 10, 2026 06:34
Changed HighLightVariable from a class to an object singleton since it
has no mutable state. This prevents unnecessary instantiation and aligns
with Kotlin best practices and the pattern used by other Variable
implementations like KeyVariable and ValueVariable.

Added KDoc documentation explaining what v:hlsearch represents, its
behavior, and reference to Vim help.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Changed RegisterVariable from a class to an object singleton for
consistency with other Variable implementations and to prevent
unnecessary instantiation.

Enhanced KDoc to explicitly mention v:register and added reference to
Vim help documentation.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Changed from instantiating HighLightVariable() and RegisterVariable()
to using direct object references HighLightVariable and RegisterVariable,
matching the pattern used for KeyVariable and ValueVariable.

This eliminates unnecessary object creation since these Variables are
now singletons.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
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.

0 participants