Skip to content

Make line commands aware of folded blocks#2508

Open
bajrangCoder wants to merge 2 commits into
mainfrom
fix/fold-aware-line-commands
Open

Make line commands aware of folded blocks#2508
bajrangCoder wants to merge 2 commits into
mainfrom
fix/fold-aware-line-commands

Conversation

@bajrangCoder

@bajrangCoder bajrangCoder commented Jul 20, 2026

Copy link
Copy Markdown
Member

Closes: #2492

@greptile-apps

greptile-apps Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR introduces foldAwareLineCommands.ts, a new TypeScript module that wraps CodeMirror's five built-in line commands (copy up/down, move up/down, delete) so they treat an entire folded block as a single logical line. The module builds a FoldIndex from foldedRanges, expands each selection to cover its full fold group, performs the text change, then re-applies translated foldEffects to the result.

  • New module (foldAwareLineCommands.ts): implements getFoldIndex, expandBlockToFolds, selectedLineBlocks, copyFoldedLines, moveFoldedLines, and deleteLineFoldAware with fallback to native commands when no folds are active.
  • Registry update (commandRegistry.js): replaces the five native @codemirror/commands imports with the fold-aware equivalents; no keybinding changes.
  • Tests (editor.tests.js): five new test cases assert document content, fold count, and cursor line after each command on a shared four-line folded-block fixture.

Confidence Score: 4/5

Safe to merge for move and delete; the copy commands produce a visible UX regression (duplicate appears on the wrong side of the cursor) that was flagged in an earlier review thread and remains unaddressed.

The move and delete paths correctly swap blocks, translate fold effects to their new positions, and fall back to native commands when no folds are active. The copy path inserts the duplicate at block.from for copy-down — placing the new copy above the original — while copy-up inserts at block.to, placing the copy below. Both directions are reversed relative to native CodeMirror behavior. The cursor ends up at the right line number in the test fixture only because the two copies are textually identical, so the assertion passes without distinguishing which duplicate holds the cursor.

The copy direction logic in copyFoldedLines (lines 219–224 of foldAwareLineCommands.ts) warrants a second look before merging.

Important Files Changed

Filename Overview
src/cm/foldAwareLineCommands.ts New module providing fold-aware wrappers for 5 line commands. The move and delete paths are well-structured; the copy path has a direction reversal noted in the prior review thread that remains unresolved in this revision.
src/cm/commandRegistry.js Straightforward swap of 5 native CodeMirror commands for the new fold-aware equivalents; old imports removed cleanly.
src/test/editor.tests.js Adds 5 new tests covering fold preservation, document content, fold count, and cursor position after each fold-aware command. Move and delete assertions are unambiguous; copy-direction tests assert correct line numbers but cannot distinguish which duplicate holds the cursor because the two copies are identical text.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[User triggers line command] --> B{Active folds in selection?}
    B -- No --> C[Delegate to native copyLineDown/Up moveLineDown/Up deleteLine]
    B -- Yes --> D[getFoldIndex build FoldGroup list]
    D --> E[selectedLineBlocks expand each selection to full fold group]
    E --> F{Command type}
    F -- copy --> G[copyFoldedLines insert duplicate text re-apply foldEffects for both copies]
    F -- move --> H[moveFoldedLines swap block with neighbor re-apply translated foldEffects]
    F -- delete --> I[deleteLineFoldAware delete entire block including surrounding newline]
    G --> J[view.dispatch changes + selection + foldEffect array]
    H --> J
    I --> K[view.dispatch changes + cursor at former block start]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[User triggers line command] --> B{Active folds in selection?}
    B -- No --> C[Delegate to native copyLineDown/Up moveLineDown/Up deleteLine]
    B -- Yes --> D[getFoldIndex build FoldGroup list]
    D --> E[selectedLineBlocks expand each selection to full fold group]
    E --> F{Command type}
    F -- copy --> G[copyFoldedLines insert duplicate text re-apply foldEffects for both copies]
    F -- move --> H[moveFoldedLines swap block with neighbor re-apply translated foldEffects]
    F -- delete --> I[deleteLineFoldAware delete entire block including surrounding newline]
    G --> J[view.dispatch changes + selection + foldEffect array]
    H --> J
    I --> K[view.dispatch changes + cursor at former block start]
Loading

Reviews (2): Last reviewed commit: "Merge main and address fold-aware copy r..." | Re-trigger Greptile

Comment thread src/cm/foldAwareLineCommands.ts Outdated
Comment thread src/test/editor.tests.js
@bajrangCoder

This comment was marked as outdated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

Move/copy/delete folded blocks

1 participant