Skip to content

feat(keybindings): add editor.reloadFromDisk and a Reload from Disk menu item (#11085) - #11086

Open
RyuK-H wants to merge 1 commit into
stablyai:mainfrom
RyuK-H:feat/editor-reload-from-disk
Open

RyuK-H wants to merge 1 commit into
stablyai:mainfrom
RyuK-H:feat/editor-reload-from-disk

Conversation

@RyuK-H

@RyuK-H RyuK-H commented Jul 28, 2026 •

Copy link
Copy Markdown

Closes #11085

Summary

Reload from Disk already exists as a user action, but its only entry point is ExternalFileChangeBanner, which appears only when a tab holds unsaved edits and the file changed on disk. Dismissing it with Keep My Edits runs keepTabEditsOverExternalChange, which re-stamps lastKnownDiskSignature to the current disk content — so until the file changes on disk again, there is no way back to the disk version short of closing the tab and losing the draft. On a clean tab there is no entry point at all; the only lever is app.forceReload, which re-renders the whole app.

This exposes the existing action through the two surfaces editor features already use, the same shape as Word Wrap (#10086):

Both call the already-exported reloadTabContentFromDisk(file, reloadContent) from ExternalFileChangeBanner.tsx:23, so behavior is identical to the banner button, including the "Reloaded from disk" toast with Undo. No new reload logic — the reload pipeline itself is untouched.

Both surfaces share one predicate, canReloadTabFromDisk (file.mode === 'edit' && file.readOnly !== true):

  • Diff surfaces are excluded. reloadTabContentFromDisk already carries the note that on diff tabs the reload rotates the Monaco model and destroys its undo stack. Inside the banner that is an informed choice among Compare / Reload / Keep; from a standalone menu item it would not be, so diff tabs do not get the entry point.
  • Read-only tabs are excluded. Log live-tail tabs already follow the file.

The keyboard path is useEditorReloadFromDiskShortcut, modeled directly on the existing useMarkdownPreviewShortcut — a window keydown listener scoped by panelRef.current.contains(target) so a split's inactive editor cannot swallow the chord.

Screenshots

No visual change unless the action is available. When it is, the editor actions menu gains one item:

…  (More actions)
├─ ☑ Word Wrap
├─ ─────────
├─   Show front matter
├─ ─────────
├─   Export as PDF
├─ ─────────
└─   Reload from Disk      ← new

Settings → Shortcuts → Editors gains a Reload from Disk row with no binding assigned, rendered by the existing definition-driven list. No new components, tokens, or layout.

Testing

  • pnpm lint
  • pnpm typecheck
  • pnpm test — 38672 passed, 63 skipped, 3673 files
  • pnpm build — see note below
  • Added or updated high-quality tests that would catch regressions

Run on macOS 15 (arm64), Node 24.18.0, pnpm 10.24.0.

New tests:

  • EditorPanelMarkdownActionsMenu.test.tsx — the item renders and fires onReloadFromDisk when canReloadFromDisk is true, and is absent when it is false.
  • ExternalFileChangeBanner.test.tsx — canReloadTabFromDisk accepts editable file tabs and rejects diff surfaces and read-only tabs, so the diff undo-stack exclusion is pinned by a test rather than by a comment.

Targeted run while iterating:

pnpm exec vitest run --config config/vitest.config.ts \
  src/renderer/src/components/editor src/shared
→ 510 passed | 2 skipped (512 files), 4302 passed | 7 skipped

Build note: pnpm build is build:desktop && build:native. build:desktop passes (renderer, main, preload, and the projected web client — 761 files, 40.3 MiB). build:native fails on this machine in native/computer-use-macos, because xcode-select -p points at /Library/Developer/CommandLineTools and the Swift 6 PackageDescription library is only in a full Xcode install. That failure is a toolchain gap on my side and is unrelated to this PR, which touches no native or Swift code — but I could not verify it green, so I am flagging it rather than ticking it silently.

AI Review Report

I ran the change through an AI review pass focused on the risks specific to it, plus an independent adversarial review of the proposal itself before writing any code.

What it checked and what it changed:

  • Destructive-action risk. The first draft simply bound reloadTabContentFromDisk to a key and a menu item. The review flagged that this function unconditionally runs clearEditorDraft / markFileDirty(false), which is safe inside the banner — where the user is answering a three-way conflict prompt — but not from a context-free entry point, especially on diff tabs where the function's own comment warns the Monaco undo stack is destroyed. Result: added the canReloadTabFromDisk gate excluding diff surfaces and read-only tabs, and covered it with tests.
  • Precedent check. The review found Request for folder refresh/reload feature in file explorer #563 and File Explorer does not refresh when files are created/deleted externally (e.g. by terminal CLI tools) #387, which asked for manual refresh in the file explorer and were resolved by fixing the watcher instead. That is a real risk of this PR being read as papering over watcher bugs, so the justification was rewritten around the Keep My Edits dismissal path, which survives a perfect watcher. The linked issue states this explicitly.
  • Scope honesty. An earlier draft of the description claimed "no new logic". The review pointed out that EditorPanelMarkdownActionsMenu is a pure presentational component, so props still have to be threaded from EditorPanelShell. Corrected: the reload logic is unchanged; the exposure path is new.
  • Cross-platform compatibility was explicitly reviewed for macOS, Linux, and Windows. No metaKey is hardcoded anywhere in this change — matching goes through keybindingMatchesAction with getShortcutPlatform(), the same path every other editor shortcut uses. The action ships with platformBindings([]) on all three platforms, so no chord is claimed and no conflict can be introduced; findKeybindingConflicts has nothing to report. Shortcut labels are rendered by the existing definition-driven Settings list, so ⌘/Ctrl presentation is unchanged. No paths, shell invocations, or Electron platform APIs are touched.
  • i18n. The new string is registered under the hashed auto.* key derived by config/scripts/localize-renderer-strings.mjs and added to all five catalogs (en, ko, ja, zh, es). pnpm lint runs verify:localization-catalog and verify:localization-coverage, both green, including locale parity at 11473 keys each.

Security Audit

Low risk. No new IPC channel, no new command execution, no auth or secret handling, no dependency change.

Specifics reviewed for this change:

  • File reads. The reload path calls the existing reloadContent callback, which is the same one the banner, the load-error retry, and the live-tail hook already use. It carries the file's own worktreeId, runtimeEnvironmentId, connectionId, and expectedExternalSshTargetId through readRuntimeFileContent, so SSH and remote-runtime tabs read from their owning runtime, not the local disk. No path is constructed or joined in this PR — OpenFile.filePath is passed through unchanged.
  • Data loss, not data leak, is the real risk here. That is what the canReloadTabFromDisk gate and the existing 8-second Undo toast address. Worth a reviewer's opinion: the item is offered on dirty tabs so the post-dismissal case is recoverable, with Undo as the safety net. If you would rather it route dirty tabs into ExternalFileChangeCompareDialog instead of reloading directly, that is a small follow-up and I am happy to make it.
  • Event handling. The keydown listener is added with capture: true on window and removed in the effect cleanup, guarded by panelRef.current.contains(target) so it cannot fire for another split's editor. It bails on event.repeat and event.defaultPrevented.

Notes

  • Untitled tabs are mode: 'edit', so the action is offered on them. Reloading one that has no file on disk yet surfaces the existing FileLoadErrorView with its Retry button rather than failing silently. Happy to exclude them explicitly if you would prefer.
  • The action is deliberately unbound. Users assign it in Settings → Shortcuts.
  • Separately, I posted a repro in [Bug]: Regression - files not updating, have to close and re-open #7868 for external changes not being picked up at all in folder workspaces — fix(editor): watch folder workspaces for external file changes #11182 fixes that, and narrows the scope my report overstated. It is a watcher bug and this PR is not a fix for it. The two do not overlap: a correct watcher auto-reloads clean tabs and raises the banner on dirty ones, and neither reaches the post-Keep My Edits state described in the Summary.

X (Twitter): @rkh1206

…enu item

Reload from Disk already exists as a user action, but its only entry point is
the external-change banner, which appears only when a tab holds unsaved edits
and the file changed on disk. Dismissing it with Keep My Edits re-stamps the
disk baseline, so until the file changes again there is no way back to the disk
version short of closing the tab.

Expose the existing action the way Word Wrap is exposed (stablyai#10086): an editor
keybinding action plus an item in the editor actions menu. Both call the
existing reloadTabContentFromDisk, so behavior matches the banner exactly.

Ships unbound by default, matching sourceControl.sendReviewNotes (stablyai#10070) and
the sleeping-workspaces toggle (stablyai#6169) — Ctrl+R and Ctrl+Shift+R are already
taken on Windows/Linux.

Gated on editable file tabs: diff surfaces are excluded because the reload
rotates the Monaco model and drops its undo stack, and read-only live-tail
tabs already follow the file.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 28, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Adds reload-from-disk support for editable editor tabs. The feature introduces eligibility checks, a localized markdown actions menu item, shell and header wiring, and a platform-aware editor keyboard shortcut hook scoped to the active panel. The shortcut action is registered in the keybinding definitions without default bindings. Tests cover tab eligibility, menu visibility, callback invocation, and updated component props.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The change implements the requested editor-scoped shortcut and menu action, reuses existing reload logic, and gates diff/read-only tabs appropriately.
Out of Scope Changes check ✅ Passed The added hooks, tests, i18n strings, and keybinding/menu wiring all support the requested feature set.
Title check ✅ Passed The title clearly names the new editor.reloadFromDisk action and Reload from Disk menu item, matching the PR's main change.
Description check ✅ Passed The description includes all required sections and is detailed, with summary, screenshots, testing, AI review, security audit, and notes.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
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.

[Feature]: Add "Reload from Disk" to the editor actions menu and keybindings

2 participants