From e0a6c264be67ac09c3ece2c5b2e51ecfc13e11f5 Mon Sep 17 00:00:00 2001 From: Raunak Raj <71929976+bajrangCoder@users.noreply.github.com> Date: Wed, 22 Jul 2026 10:54:59 +0530 Subject: [PATCH 1/2] refactor(editor): centralize CodeMirror commands and keybindings --- src/cm/baseExtensions.ts | 4 +- src/cm/commandRegistry.js | 244 ++++++++++++++++------------ src/cm/foldingCommands.js | 60 +++++++ src/cm/keyBindingUtils.js | 108 ++++++++++++ src/cm/lsp/clientManager.ts | 21 +-- src/components/terminal/terminal.js | 9 +- src/lib/keyBindings.js | 162 +++++++++++++++--- src/main.js | 10 +- src/test/editor.tests.js | 165 ++++++++++++++++++- 9 files changed, 633 insertions(+), 150 deletions(-) create mode 100644 src/cm/foldingCommands.js create mode 100644 src/cm/keyBindingUtils.js diff --git a/src/cm/baseExtensions.ts b/src/cm/baseExtensions.ts index a0db81d4c..927c3ed94 100644 --- a/src/cm/baseExtensions.ts +++ b/src/cm/baseExtensions.ts @@ -4,7 +4,7 @@ import { closeBracketsKeymap, completionKeymap, } from "@codemirror/autocomplete"; -import { defaultKeymap, history, historyKeymap } from "@codemirror/commands"; +import { history } from "@codemirror/commands"; import { bracketMatching, defaultHighlightStyle, @@ -79,8 +79,6 @@ export default function createBaseExtensions( keymap.of([ ...(autoCloseBrackets ? closeBracketsKeymap : []), ...completionKeymap, - ...defaultKeymap, - ...historyKeymap, ]), ); extensions.push( diff --git a/src/cm/commandRegistry.js b/src/cm/commandRegistry.js index 95d36b28e..a4577f7cd 100644 --- a/src/cm/commandRegistry.js +++ b/src/cm/commandRegistry.js @@ -46,13 +46,12 @@ import { selectPageUp, simplifySelection, toggleBlockComment, + toggleLineComment, undo, } from "@codemirror/commands"; import { - foldAll, foldCode, indentUnit as indentUnitFacet, - unfoldAll, unfoldCode, } from "@codemirror/language"; import { @@ -81,6 +80,12 @@ import { moveLineDownFoldAware, moveLineUpFoldAware, } from "cm/foldAwareLineCommands"; +import { foldAllCodeBlocks, unfoldAllCodeBlocks } from "cm/foldingCommands"; +import { + canonicalizeKeyBinding, + keyBindingsConflict, + toCodeMirrorKey, +} from "cm/keyBindingUtils"; import { renameSymbol as acodeRenameSymbol, clearDiagnosticsEffect, @@ -98,7 +103,10 @@ import { showDocumentSymbols } from "components/symbolsPanel"; import toast from "components/toast"; import prompt from "dialogs/prompt"; import actions from "handlers/quickTools"; -import keyBindings from "lib/keyBindings"; +import keyBindings, { + APP_KEY_BINDING_NAMES, + CODEMIRROR_COMMAND_NAMES, +} from "lib/keyBindings"; import settings from "lib/settings"; import Url from "utils/Url"; @@ -130,47 +138,23 @@ let resolvedKeyBindings = keyBindings; /** @type {Record} */ let cachedResolvedKeyBindings = {}; +/** @type {Record} */ +let cachedEffectiveKeyBindings = {}; + +/** @type {{key: string, command: string, shadowedBy: string}[]} */ +let cachedKeyBindingConflicts = []; + let resolvedKeyBindingsVersion = 0; /** @type {import("@codemirror/view").KeyBinding[]} */ let cachedKeymap = []; -const ARROW_KEY_MAP = { - left: "ArrowLeft", - right: "ArrowRight", - up: "ArrowUp", - down: "ArrowDown", -}; - -const SPECIAL_KEY_MAP = { - esc: "Escape", - escape: "Escape", - return: "Enter", - enter: "Enter", - space: "Space", - del: "Delete", - delete: "Delete", - backspace: "Backspace", - tab: "Tab", - home: "Home", - end: "End", - pageup: "PageUp", - pagedown: "PageDown", - insert: "Insert", -}; - -const MODIFIER_MAP = { - ctrl: "Mod", - control: "Mod", - cmd: "Mod", - meta: "Mod", - shift: "Shift", - alt: "Alt", - option: "Alt", -}; +/** @type {Set} */ +const commandViews = new Set(); const CODEMIRROR_COMMAND_ENTRIES = Object.entries(cmCommands).filter( - ([, value]) => typeof value === "function", + ([name, value]) => + typeof value === "function" && CODEMIRROR_COMMAND_NAMES.has(name), ); const CODEMIRROR_COMMAND_MAP = new Map( @@ -955,7 +939,7 @@ function registerCoreCommands() { run(view) { const resolvedView = resolveView(view); if (!resolvedView) return false; - return lineComment(resolvedView); + return toggleLineComment(resolvedView); }, }); addCommand({ @@ -1055,7 +1039,7 @@ function registerCoreCommands() { run(view) { const resolvedView = resolveView(view); if (!resolvedView) return false; - return foldAll(resolvedView); + return foldAllCodeBlocks(resolvedView); }, }); addCommand({ @@ -1066,7 +1050,7 @@ function registerCoreCommands() { run(view) { const resolvedView = resolveView(view); if (!resolvedView) return false; - return unfoldAll(resolvedView); + return unfoldAllCodeBlocks(resolvedView); }, }); } @@ -1531,50 +1515,10 @@ function buildResolvedKeyBindingsSnapshot() { ); } -function toCodeMirrorKey(combo) { - if (!combo) return null; - const parts = combo.endsWith("-") - ? [...combo.slice(0, -1).split("-").filter(Boolean), "-"] - : combo - .split("-") - .map((part) => part.trim()) - .filter(Boolean); - const modifiers = []; - let key = null; - - parts.forEach((part, index) => { - const lower = part.toLowerCase(); - if (MODIFIER_MAP[lower]) { - const mod = MODIFIER_MAP[lower]; - if (!modifiers.includes(mod)) modifiers.push(mod); - return; - } - - if (ARROW_KEY_MAP[lower]) { - key = ARROW_KEY_MAP[lower]; - return; - } - - if (SPECIAL_KEY_MAP[lower]) { - key = SPECIAL_KEY_MAP[lower]; - return; - } - - if (part.length === 1 && /[a-z]/i.test(part)) { - key = part.length === 1 ? part.toLowerCase() : part; - return; - } - - key = part; - }); - - if (!key) return modifiers.join("-") || null; - return modifiers.length ? `${modifiers.join("-")}-${key}` : key; -} - function rebuildKeymap() { - const bindings = []; cachedResolvedKeyBindings = buildResolvedKeyBindingsSnapshot(); + const candidates = []; + let order = 0; commandMap.forEach((command, name) => { const bindingInfo = resolveBindingInfo(name); command.description = @@ -1588,18 +1532,91 @@ function rebuildKeymap() { combos.forEach((combo) => { const cmKey = toCodeMirrorKey(combo); if (!cmKey) return; - bindings.push({ + candidates.push({ + name, key: cmKey, - run: (view) => executeCommand(name, view), - preventDefault: true, + rawKey: combo, + order: order++, + priority: getBindingPriority(name), }); }); }); + + candidates.sort( + (left, right) => right.priority - left.priority || left.order - right.order, + ); + + const bindings = []; + const claimedKeys = new Map(); + const effectiveKeys = new Map(); + const conflicts = []; + for (const candidate of candidates) { + const canonicalKey = canonicalizeKeyBinding(candidate.key); + const claimed = Array.from(claimedKeys.entries()).find(([key]) => + keyBindingsConflict(key, canonicalKey), + ); + if (claimed) { + const [claimedKey, owner] = claimed; + const appCommandShadowsCodeMirrorDefault = + APP_KEY_BINDING_NAMES.has(owner) && + CODEMIRROR_COMMAND_NAMES.has(candidate.name) && + candidate.priority === 10; + const repeatedKeyForSameCommand = + owner === candidate.name && claimedKey === canonicalKey; + if (!repeatedKeyForSameCommand && !appCommandShadowsCodeMirrorDefault) { + conflicts.push({ + key: candidate.key, + command: candidate.name, + shadowedBy: owner, + }); + } + continue; + } + + claimedKeys.set(canonicalKey, candidate.name); + if (!effectiveKeys.has(candidate.name)) { + effectiveKeys.set(candidate.name, []); + } + effectiveKeys.get(candidate.name).push(candidate.rawKey); + bindings.push({ + key: candidate.key, + run: (view) => executeCommand(candidate.name, view), + preventDefault: true, + }); + } + + cachedEffectiveKeyBindings = Object.fromEntries( + Object.entries(cachedResolvedKeyBindings).map(([name, binding]) => [ + name, + { + ...binding, + key: effectiveKeys.get(name)?.join("|") || null, + }, + ]), + ); + cachedKeyBindingConflicts = conflicts; cachedKeymap = bindings; resolvedKeyBindingsVersion += 1; return bindings; } +function getBindingPriority(name) { + if (APP_KEY_BINDING_NAMES.has(name)) return 40; + if (!Object.prototype.hasOwnProperty.call(keyBindings, name)) return 30; + + const override = resolvedKeyBindings?.[name]; + if ( + override && + typeof override === "object" && + Object.prototype.hasOwnProperty.call(override, "key") && + override.key !== keyBindings[name]?.key + ) { + return 20; + } + + return 10; +} + function resolveCommand(name) { return commandMap.get(name) || null; } @@ -1638,6 +1655,14 @@ export function getResolvedKeyBindings() { return cachedResolvedKeyBindings; } +export function getEffectiveKeyBindings() { + return cachedEffectiveKeyBindings; +} + +export function getKeyBindingConflicts() { + return cachedKeyBindingConflicts.map((conflict) => ({ ...conflict })); +} + export function getResolvedKeyBindingsVersion() { return resolvedKeyBindingsVersion; } @@ -1650,7 +1675,15 @@ export async function setKeyBindings(view) { await loadCustomKeyBindings(); const bindings = rebuildKeymap(); const resolvedView = resolveView(view); - applyCommandKeymap(resolvedView, bindings); + if (resolvedView) commandViews.add(resolvedView); + for (const knownView of commandViews) { + if (!knownView.dom?.isConnected) { + commandViews.delete(knownView); + continue; + } + applyCommandKeymap(knownView, bindings); + } + return getKeyBindingConflicts(); } async function loadCustomKeyBindings() { @@ -1658,23 +1691,29 @@ async function loadCustomKeyBindings() { const bindingsFile = fsOperation(KEYBINDING_FILE); if (await bindingsFile.exists()) { const bindings = await bindingsFile.readFile("json"); - if (bindings && typeof bindings === "object") { - let updated = false; - Object.keys(keyBindings).forEach((key) => { - if (!(key in bindings)) { - bindings[key] = keyBindings[key]; - updated = true; - } - }); - resolvedKeyBindings = bindings; - if (updated) { - bindingsFile - .writeFile(JSON.stringify(bindings, undefined, 2)) - .catch((error) => { - window.log?.("error", "Failed to back-fill new key bindings!"); - window.log?.("error", error); - }); + if ( + !bindings || + typeof bindings !== "object" || + Array.isArray(bindings) + ) { + throw new TypeError("Key bindings must be a JSON object"); + } + + let updated = false; + Object.keys(keyBindings).forEach((key) => { + if (!(key in bindings)) { + bindings[key] = keyBindings[key]; + updated = true; } + }); + resolvedKeyBindings = bindings; + if (updated) { + bindingsFile + .writeFile(JSON.stringify(bindings, undefined, 2)) + .catch((error) => { + window.log?.("error", "Failed to back-fill new key bindings!"); + window.log?.("error", error); + }); } } else { throw new Error("Key binding file not found"); @@ -1791,6 +1830,7 @@ function normalizeExternalKey(bindKey) { function applyCommandKeymap(view, bindings = cachedKeymap) { if (!view) return; + commandViews.add(view); view.dispatch({ effects: commandKeymapCompartment.reconfigure( keymap.of(bindings ?? cachedKeymap), diff --git a/src/cm/foldingCommands.js b/src/cm/foldingCommands.js new file mode 100644 index 000000000..bb2df32ad --- /dev/null +++ b/src/cm/foldingCommands.js @@ -0,0 +1,60 @@ +import { + codeFolding, + ensureSyntaxTree, + foldable, + foldEffect, + foldedRanges, + foldState, + unfoldEffect, +} from "@codemirror/language"; +import { StateEffect } from "@codemirror/state"; + +const FULL_PARSE_BUDGET_MS = 100; + +/** + * Fold every foldable block, including nested blocks. CodeMirror's built-in + * foldAll intentionally skips nested ranges after finding a top-level fold. + */ +export function foldAllCodeBlocks(view) { + const { state } = view; + ensureSyntaxTree(state, state.doc.length, FULL_PARSE_BUDGET_MS); + + const existing = new Set(); + foldedRanges(state).between(0, state.doc.length, (from, to) => { + existing.add(`${from}:${to}`); + }); + + const effects = []; + const discovered = new Set(); + for (let lineNumber = 1; lineNumber <= state.doc.lines; lineNumber += 1) { + const line = state.doc.line(lineNumber); + const range = foldable(state, line.from, line.to); + if (!range) continue; + + const id = `${range.from}:${range.to}`; + if (existing.has(id) || discovered.has(id)) continue; + discovered.add(id); + effects.push(foldEffect.of(range)); + } + + if (!effects.length) return false; + if (!state.field(foldState, false)) { + // Install the state field first so it can consume the fold effects in + // the following transaction when folding was disabled in editor settings. + view.dispatch({ effects: StateEffect.appendConfig.of(codeFolding()) }); + } + view.dispatch({ effects }); + return true; +} + +/** Unfold every stored fold, including nested folds created above. */ +export function unfoldAllCodeBlocks(view) { + const effects = []; + foldedRanges(view.state).between(0, view.state.doc.length, (from, to) => { + effects.push(unfoldEffect.of({ from, to })); + }); + + if (!effects.length) return false; + view.dispatch({ effects }); + return true; +} diff --git a/src/cm/keyBindingUtils.js b/src/cm/keyBindingUtils.js new file mode 100644 index 000000000..69e2a3583 --- /dev/null +++ b/src/cm/keyBindingUtils.js @@ -0,0 +1,108 @@ +const ARROW_KEY_MAP = { + left: "ArrowLeft", + right: "ArrowRight", + up: "ArrowUp", + down: "ArrowDown", +}; + +const SPECIAL_KEY_MAP = { + esc: "Escape", + escape: "Escape", + return: "Enter", + enter: "Enter", + space: "Space", + del: "Delete", + delete: "Delete", + backspace: "Backspace", + tab: "Tab", + home: "Home", + end: "End", + pageup: "PageUp", + pagedown: "PageDown", + insert: "Insert", +}; + +const MODIFIER_MAP = { + mod: "Mod", + ctrl: "Mod", + control: "Mod", + cmd: "Mod", + meta: "Mod", + shift: "Shift", + alt: "Alt", + option: "Alt", +}; + +const MODIFIER_ORDER = ["Mod", "Alt", "Shift"]; + +export function toCodeMirrorKey(combo) { + if (!combo) return null; + const strokes = String(combo) + .trim() + .split(/\s+/) + .map(toCodeMirrorKeyStroke) + .filter(Boolean); + return strokes.length ? strokes.join(" ") : null; +} + +export function canonicalizeKeyBinding(combo) { + return toCodeMirrorKey(combo)?.toLowerCase() || null; +} + +/** + * CodeMirror cannot use a key as both a command and a multi-stroke prefix. + * Chords that merely share a prefix (for example Ctrl-K C and Ctrl-K U) are + * compatible with each other. + */ +export function keyBindingsConflict(left, right) { + const leftKey = canonicalizeKeyBinding(left); + const rightKey = canonicalizeKeyBinding(right); + if (!leftKey || !rightKey) return false; + return ( + leftKey === rightKey || + leftKey.startsWith(`${rightKey} `) || + rightKey.startsWith(`${leftKey} `) + ); +} + +function toCodeMirrorKeyStroke(stroke) { + const parts = stroke.endsWith("-") + ? [...stroke.slice(0, -1).split("-").filter(Boolean), "-"] + : stroke + .split("-") + .map((part) => part.trim()) + .filter(Boolean); + const modifiers = new Set(); + let key = null; + + parts.forEach((part) => { + const lower = part.toLowerCase(); + if (MODIFIER_MAP[lower]) { + modifiers.add(MODIFIER_MAP[lower]); + return; + } + + if (ARROW_KEY_MAP[lower]) { + key = ARROW_KEY_MAP[lower]; + return; + } + + if (SPECIAL_KEY_MAP[lower]) { + key = SPECIAL_KEY_MAP[lower]; + return; + } + + if (part.length === 1 && /[a-z]/i.test(part)) { + key = part.toLowerCase(); + return; + } + + key = part; + }); + + if (!key) return null; + const orderedModifiers = MODIFIER_ORDER.filter((modifier) => + modifiers.has(modifier), + ); + return orderedModifiers.length ? `${orderedModifiers.join("-")}-${key}` : key; +} diff --git a/src/cm/lsp/clientManager.ts b/src/cm/lsp/clientManager.ts index f95f539cc..e14041a85 100644 --- a/src/cm/lsp/clientManager.ts +++ b/src/cm/lsp/clientManager.ts @@ -1,16 +1,13 @@ import { getIndentUnit, indentUnit } from "@codemirror/language"; import type { LSPClientExtension } from "@codemirror/lsp-client"; import { - findReferencesKeymap, - formatKeymap, - jumpToDefinitionKeymap, LSPClient, LSPPlugin, serverCompletion, serverDiagnostics, } from "@codemirror/lsp-client"; import { EditorState, Extension, Facet, MapMode } from "@codemirror/state"; -import { EditorView, keymap } from "@codemirror/view"; +import { EditorView } from "@codemirror/view"; import lspStatusBar from "components/lspStatusBar"; import notificationManager from "lib/notificationManager"; import Uri from "utils/Uri"; @@ -19,7 +16,6 @@ import { clearDiagnosticsEffect } from "./diagnostics"; import { supportsBuiltinFormatting } from "./formattingSupport"; import { inlayHintsExtension } from "./inlayHints"; import { addLspLog } from "./logs"; -import { acodeRenameKeymap } from "./rename"; import { selectRuntimeProvider } from "./runtimeProviders"; import serverRegistry from "./serverRegistry"; import { @@ -212,10 +208,8 @@ function buildBuiltinExtensions( hover: includeHover = true, completion: includeCompletion = true, signature: includeSignature = true, - keymaps: includeKeymaps = true, diagnostics: includeDiagnostics = true, inlayHints: includeInlayHints = false, - formatting: includeFormatting = true, } = config; const extensions: Extension[] = []; @@ -223,18 +217,7 @@ function buildBuiltinExtensions( if (includeCompletion) extensions.push(serverCompletion()); if (includeHover) extensions.push(hoverTooltips()); - if (includeKeymaps) { - const bindings = [ - ...(includeFormatting ? formatKeymap : []), - ...acodeRenameKeymap, - ...jumpToDefinitionKeymap, - ...findReferencesKeymap, - ]; - if (bindings.length) { - extensions.push(keymap.of(bindings)); - } - } - if (includeSignature) extensions.push(signatureHelp()); + if (includeSignature) extensions.push(signatureHelp({ keymap: false })); if (includeDiagnostics) { const diagExt = serverDiagnostics(); diagnosticsExtension = diagExt; diff --git a/src/components/terminal/terminal.js b/src/components/terminal/terminal.js index 82100572f..a21545389 100644 --- a/src/components/terminal/terminal.js +++ b/src/components/terminal/terminal.js @@ -13,7 +13,7 @@ import { WebglAddon } from "@xterm/addon-webgl"; import { Terminal as Xterm } from "@xterm/xterm"; import { executeCommand, - getResolvedKeyBindings, + getEffectiveKeyBindings, getResolvedKeyBindingsVersion, } from "cm/commandRegistry"; import confirm from "dialogs/confirm"; @@ -375,7 +375,7 @@ export default class TerminalComponent { const parsedBindings = []; - Object.entries(getResolvedKeyBindings()).forEach(([name, binding]) => { + Object.entries(getEffectiveKeyBindings()).forEach(([name, binding]) => { if (!binding.key) return; // Skip editor-only keybindings in terminal @@ -385,6 +385,11 @@ export default class TerminalComponent { const keys = binding.key.split("|"); keys.forEach((keyCombo) => { + // CodeMirror supports multi-stroke chords, while xterm's keyboard + // callback receives one event at a time. Do not misread a chord as + // a single malformed terminal shortcut. + if (/\s/.test(keyCombo.trim())) return; + const parts = keyCombo.endsWith("-") ? [...keyCombo.slice(0, -1).split("-").filter(Boolean), "-"] : keyCombo.split("-"); diff --git a/src/lib/keyBindings.js b/src/lib/keyBindings.js index 5ac984166..b25213962 100644 --- a/src/lib/keyBindings.js +++ b/src/lib/keyBindings.js @@ -4,12 +4,19 @@ import { emacsStyleKeymap, historyKeymap, indentWithTab, - standardKeymap, } from "@codemirror/commands"; +import { + canonicalizeKeyBinding, + keyBindingsConflict, +} from "cm/keyBindingUtils"; const MODIFIER_ORDER = ["Ctrl", "Alt", "Shift", "Cmd"]; +const CODEMIRROR_NON_COMMAND_EXPORTS = new Set([ + "history", + "redoDepth", + "undoDepth", +]); const KEYMAP_SOURCES = [ - ...standardKeymap, ...defaultKeymap, ...historyKeymap, ...emacsStyleKeymap, @@ -307,7 +314,7 @@ const APP_BINDING_CONFIG = [ { name: "problems", description: "Show problems", - key: null, + key: "Ctrl-Shift-M", readOnly: true, editorOnly: true, }, @@ -367,13 +374,13 @@ const APP_BINDING_CONFIG = [ { name: "openPluginsPage", description: "Open plugins page", - key: null, + key: "Ctrl-Shift-X", readOnly: true, }, { name: "openFileExplorer", description: "Open file explorer", - key: null, + key: "Ctrl-Shift-E", readOnly: true, }, { @@ -544,6 +551,13 @@ const APP_BINDING_CONFIG = [ readOnly: false, editorOnly: true, }, + { + name: "deleteToLineEnd", + description: "Delete to line end", + key: null, + readOnly: false, + editorOnly: true, + }, { name: "togglecomment", description: "Toggle comment", @@ -568,7 +582,7 @@ const APP_BINDING_CONFIG = [ { name: "toggleBlockComment", description: "Toggle block comment", - key: "Ctrl-Shift-/", + key: "Ctrl-Shift-/|Shift-Alt-A", readOnly: false, editorOnly: true, }, @@ -589,7 +603,7 @@ const APP_BINDING_CONFIG = [ { name: "simplifySelection", description: "Simplify selection", - key: null, + key: "Escape", readOnly: true, editorOnly: true, }, @@ -630,9 +644,80 @@ const APP_BINDING_CONFIG = [ readOnly: true, editorOnly: true, }, + { + name: "deleteTrailingWhitespace", + description: "Delete trailing whitespace", + key: null, + readOnly: false, + editorOnly: true, + }, + { + name: "formatDocument", + description: "Format document (Language Server)", + key: "Alt-Shift-F", + readOnly: false, + editorOnly: true, + }, + { + name: "renameSymbol", + description: "Rename symbol (Language Server)", + key: null, + readOnly: false, + editorOnly: true, + }, + { + name: "showSignatureHelp", + description: "Show signature help", + key: "Ctrl-Shift-Space", + readOnly: true, + editorOnly: true, + }, + { + name: "prevSignature", + description: "Previous signature", + key: "Ctrl-Shift-Up", + readOnly: true, + editorOnly: true, + }, + { + name: "nextSignature", + description: "Next signature", + key: "Ctrl-Shift-Down", + readOnly: true, + editorOnly: true, + }, + { + name: "jumpToDefinition", + description: "Go to definition (Language Server)", + key: "F12", + readOnly: true, + editorOnly: true, + }, + { + name: "findReferences", + description: "Find all references (Language Server)", + key: "Shift-F12", + readOnly: true, + editorOnly: true, + }, + { + name: "nextDiagnostic", + description: "Go to next diagnostic", + key: "F8", + readOnly: true, + editorOnly: true, + }, + { + name: "previousDiagnostic", + description: "Go to previous diagnostic", + key: "Shift-F8", + readOnly: true, + editorOnly: true, + }, ]; const APP_KEY_BINDINGS = buildAppBindings(APP_BINDING_CONFIG); +export const APP_KEY_BINDING_NAMES = new Set(Object.keys(APP_KEY_BINDINGS)); const APP_CUSTOM_COMMANDS = new Set( APP_BINDING_CONFIG.filter((config) => !config.action).map( (config) => config.name, @@ -646,9 +731,13 @@ const FORCE_READ_ONLY = new Set([ const MUTATING_COMMAND_PATTERN = /^(delete|insert|indent|move|copy|split|transpose|toggle|undo|redo|line|block)/i; -const CODEMIRROR_COMMAND_NAMES = new Set( +export const CODEMIRROR_COMMAND_NAMES = new Set( Object.entries(cmCommands) - .filter(([, value]) => typeof value === "function") + .filter( + ([name, value]) => + typeof value === "function" && + !CODEMIRROR_NON_COMMAND_EXPORTS.has(name), + ) .map(([name]) => name), ); @@ -694,7 +783,8 @@ function buildAppBindings(configs) { function buildCodemirrorKeyBindings(appBindings) { const commandEntries = Object.entries(cmCommands).filter( - ([, value]) => typeof value === "function", + ([name, value]) => + CODEMIRROR_COMMAND_NAMES.has(name) && typeof value === "function", ); const commandNameByFunction = new Map( commandEntries.map(([name, fn]) => [fn, name]), @@ -704,10 +794,11 @@ function buildCodemirrorKeyBindings(appBindings) { for (const binding of KEYMAP_SOURCES) { const baseCombos = new Set(); + // Acode's portable Ctrl syntax becomes CodeMirror's Mod at runtime, so + // platform-specific mac aliases would only create duplicate bindings here. pushCommandCombo(binding.run, binding.key, "win", baseCombos); pushCommandCombo(binding.run, binding.win, "win", baseCombos); pushCommandCombo(binding.run, binding.linux, "win", baseCombos); - pushCommandCombo(binding.run, binding.mac, "mac", baseCombos); if (binding.shift) { const shiftName = commandNameByFunction.get(binding.shift); @@ -718,7 +809,6 @@ function buildCodemirrorKeyBindings(appBindings) { normalizeKey(binding.key, "win"), normalizeKey(binding.win, "win"), normalizeKey(binding.linux, "win"), - normalizeKey(binding.mac, "mac"), ].filter(Boolean); for (const combo of combos) { addCommandCombo(comboMap, shiftName, ensureModifier(combo, "Shift")); @@ -727,17 +817,47 @@ function buildCodemirrorKeyBindings(appBindings) { } } - const result = {}; + const result = Object.fromEntries( + commandEntries + .filter(([name]) => !appBindings[name]) + .map(([name]) => [ + name, + { + description: humanizeCommandName(name), + key: null, + readOnly: inferReadOnly(name), + editorOnly: true, + }, + ]), + ); + const claimedCombos = new Set(); + for (const binding of Object.values(appBindings)) { + for (const combo of String(binding?.key || "").split("|")) { + const normalized = canonicalizeKeyBinding(combo); + if (normalized) claimedCombos.add(normalized); + } + } + for (const [name, combos] of comboMap.entries()) { if (!combos.size || appBindings[name]) continue; - result[name] = { - description: humanizeCommandName(name), - key: Array.from(combos) - .sort((a, b) => a.localeCompare(b)) - .join("|"), - readOnly: inferReadOnly(name), - editorOnly: true, - }; + const availableCombos = Array.from(combos) + .filter((combo) => { + const normalized = canonicalizeKeyBinding(combo); + if (!normalized) return false; + if ( + Array.from(claimedCombos).some((claimed) => + keyBindingsConflict(claimed, normalized), + ) + ) { + return false; + } + claimedCombos.add(normalized); + return true; + }) + .sort((a, b) => a.localeCompare(b)); + result[name].key = availableCombos.length + ? availableCombos.join("|") + : null; } return result; diff --git a/src/main.js b/src/main.js index efe610526..20a1f52c5 100644 --- a/src/main.js +++ b/src/main.js @@ -637,7 +637,15 @@ async function loadApp() { if (activeFile) editorManager.editor.contentDOM.blur(); }; sdcard.watchFile(KEYBINDING_FILE, async () => { - await setKeyBindings(editorManager.editor); + const conflicts = await setKeyBindings(editorManager.editor); + if (conflicts.length) { + const conflict = conflicts[0]; + console.warn("Ignored conflicting key bindings", conflicts); + toast( + `Keybinding conflict: ${conflict.key} is already used by ${conflict.shadowedBy}`, + ); + return; + } toast(strings["key bindings updated"]); }); //#endregion diff --git a/src/test/editor.tests.js b/src/test/editor.tests.js index d614db377..a1a88f503 100644 --- a/src/test/editor.tests.js +++ b/src/test/editor.tests.js @@ -1,4 +1,12 @@ -import { history, isolateHistory, redo, undo } from "@codemirror/commands"; +import { + defaultKeymap, + history, + historyKeymap, + isolateHistory, + redo, + undo, +} from "@codemirror/commands"; +import { javascript } from "@codemirror/lang-javascript"; import { bracketMatching, defaultHighlightStyle, @@ -9,7 +17,7 @@ import { } from "@codemirror/language"; import { highlightSelectionMatches, searchKeymap } from "@codemirror/search"; import { Compartment, EditorSelection, EditorState } from "@codemirror/state"; -import { EditorView, runScopeHandlers } from "@codemirror/view"; +import { EditorView, keymap, runScopeHandlers } from "@codemirror/view"; import createBaseExtensions from "cm/baseExtensions"; import { copyLineDownFoldAware, @@ -18,12 +26,18 @@ import { moveLineDownFoldAware, moveLineUpFoldAware, } from "cm/foldAwareLineCommands"; +import { foldAllCodeBlocks, unfoldAllCodeBlocks } from "cm/foldingCommands"; import indentedLineWrapping, { DEFAULT_MAX_WRAP_INDENT_COLUMNS, getContinuationIndentColumns, getWrapIndentColumns, } from "cm/indentedLineWrapping"; import indentGuides from "cm/indentGuides"; +import { + canonicalizeKeyBinding, + keyBindingsConflict, + toCodeMirrorKey, +} from "cm/keyBindingUtils"; import { findQuickToolCommand, getShortcutAlternatives, @@ -42,6 +56,7 @@ import { addPointerSelectionRange, getEdgeScrollDirections, } from "cm/touchSelectionMenu"; +import keyBindings, { CODEMIRROR_COMMAND_NAMES } from "lib/keyBindings"; import { TestRunner } from "./tester"; export async function runCodeMirrorTests(writeOutput) { @@ -58,6 +73,7 @@ export async function runCodeMirrorTests(writeOutput) { doc, extensions: [ ...createBaseExtensions(baseExtensionOptions), + keymap.of([...defaultKeymap, ...historyKeymap]), ...extensions, ], }); @@ -618,6 +634,151 @@ export async function runCodeMirrorTests(writeOutput) { ); }); + runner.test("Every CodeMirror command can be assigned a key", (test) => { + const missingCommands = Array.from(CODEMIRROR_COMMAND_NAMES).filter( + (name) => !keyBindings[name], + ); + + test.assertEqual(missingCommands.join(","), ""); + }); + + runner.test("Default key bindings have one owner per shortcut", (test) => { + const shortcuts = []; + const conflicts = []; + for (const [name, binding] of Object.entries(keyBindings)) { + for (const shortcut of String(binding.key || "").split("|")) { + if (!shortcut) continue; + const normalized = canonicalizeKeyBinding(shortcut); + if (!normalized) continue; + const claimed = shortcuts.find(({ key }) => + keyBindingsConflict(key, normalized), + ); + if (claimed) { + const repeatedKeyForSameCommand = + claimed.name === name && claimed.key === normalized; + if (!repeatedKeyForSameCommand) { + conflicts.push(`${shortcut}: ${claimed.name}, ${name}`); + } + } else if (!claimed) { + shortcuts.push({ key: normalized, name }); + } + } + } + + test.assertEqual(conflicts.join("; "), ""); + }); + + runner.test("Ctrl-K stays available for the terminal plugin", (test) => { + const ctrlKBindings = []; + for (const [name, binding] of Object.entries(keyBindings)) { + for (const shortcut of String(binding.key || "").split("|")) { + if (keyBindingsConflict(shortcut, "Ctrl-K")) { + ctrlKBindings.push(`${name}: ${shortcut}`); + } + } + } + test.assertEqual(ctrlKBindings.join("; "), ""); + }); + + runner.test( + "CodeMirror can compile the generated default keymap", + async (test) => { + const generatedKeymap = Object.values(keyBindings).flatMap((binding) => + String(binding.key || "") + .split("|") + .filter(Boolean) + .map((shortcut) => ({ + key: toCodeMirrorKey(shortcut), + run: () => false, + })), + ); + + await withEditor( + test, + (view) => { + let error = null; + try { + runScopeHandlers( + view, + new KeyboardEvent("keydown", { key: "Enter" }), + "editor", + ); + } catch (caught) { + error = caught; + } + test.assert( + !error, + error?.message || "Generated keymap should compile", + ); + }, + "", + [keymap.of(generatedKeymap)], + ); + }, + ); + + runner.test("Normalized key bindings remain stable", (test) => { + test.assertEqual(canonicalizeKeyBinding("Ctrl-Tab"), "mod-tab"); + test.assertEqual(canonicalizeKeyBinding("Mod-Tab"), "mod-tab"); + test.assertEqual(canonicalizeKeyBinding("Ctrl-Shift-Tab"), "mod-shift-tab"); + test.assertEqual(canonicalizeKeyBinding("Mod-Shift-Tab"), "mod-shift-tab"); + test.assertEqual(canonicalizeKeyBinding("Ctrl-K S"), "mod-k s"); + test.assertEqual(canonicalizeKeyBinding("Ctrl-K Ctrl-X"), "mod-k mod-x"); + test.assert(keyBindingsConflict("Ctrl-K", "Ctrl-K S")); + test.assert(!keyBindingsConflict("Ctrl-K S", "Ctrl-K Ctrl-X")); + }); + + runner.test( + "Conventional editor shortcuts are available by default", + (test) => { + test.assertEqual(keyBindings.saveAllChanges.key, null); + test.assertEqual(keyBindings.problems.key, "Ctrl-Shift-M"); + test.assertEqual(keyBindings.formatDocument.key, "Alt-Shift-F"); + test.assertEqual(keyBindings.jumpToDefinition.key, "F12"); + test.assertEqual(keyBindings.findReferences.key, "Shift-F12"); + test.assertEqual(keyBindings.nextDiagnostic.key, "F8"); + test.assertEqual(keyBindings.previousDiagnostic.key, "Shift-F8"); + test.assertEqual(keyBindings.simplifySelection.key, "Escape"); + test.assertEqual(keyBindings.deleteToLineEnd.key, null); + test.assertEqual(keyBindings.deleteTrailingWhitespace.key, null); + test.assertEqual(keyBindings.renameSymbol.key, null); + test.assertEqual( + keyBindings.toggleBlockComment.key, + "Ctrl-Shift-/|Shift-Alt-A", + ); + }, + ); + + runner.test( + "Pane focus shortcuts override conflicting editor defaults", + (test) => { + test.assertEqual(keyBindings.focusPaneUp.key, "Ctrl-Alt-Up"); + test.assertEqual(keyBindings.focusPaneDown.key, "Ctrl-Alt-Down"); + test.assertEqual(keyBindings.addCursorAbove.key, null); + test.assertEqual(keyBindings.addCursorBelow.key, null); + }, + ); + + runner.test( + "Fold all includes nested blocks and unfold all clears them", + async (test) => { + await withEditor( + test, + async (view) => { + test.assert(foldAllCodeBlocks(view), "Nested blocks should fold"); + test.assert( + countFolds(view) >= 2, + "Fold all should retain both outer and nested folds", + ); + test.assert(unfoldAllCodeBlocks(view), "All folds should unfold"); + test.assertEqual(countFolds(view), 0); + }, + "function outer() {\n if (true) {\n console.log('nested');\n }\n}", + [javascript()], + ); + }, + ); + // ========================================= // FOLD-AWARE LINE COMMAND TESTS // ========================================= From cf2acc596d07af6b29cb4c496262882acef0fca1 Mon Sep 17 00:00:00 2001 From: Raunak Raj <71929976+bajrangCoder@users.noreply.github.com> Date: Wed, 22 Jul 2026 11:39:58 +0530 Subject: [PATCH 2/2] fix fold all command same line thing --- src/cm/foldingCommands.js | 63 ++++++++++++++++++++++++++++++++++----- src/test/editor.tests.js | 9 ++---- 2 files changed, 58 insertions(+), 14 deletions(-) diff --git a/src/cm/foldingCommands.js b/src/cm/foldingCommands.js index bb2df32ad..c3eef1334 100644 --- a/src/cm/foldingCommands.js +++ b/src/cm/foldingCommands.js @@ -1,15 +1,57 @@ import { codeFolding, ensureSyntaxTree, - foldable, foldEffect, foldedRanges, + foldNodeProp, + foldService, foldState, + syntaxTree, unfoldEffect, } from "@codemirror/language"; import { StateEffect } from "@codemirror/state"; const FULL_PARSE_BUDGET_MS = 100; +const INCOMPLETE_PARSE_MARGIN = 50; + +function findServiceFold(state, line) { + for (const service of state.facet(foldService)) { + const range = service(state, line.from, line.to); + if (range) return range; + } + return null; +} + +function addSyntaxFolds(state, tree, line, addRange) { + if (!tree || tree.length < line.to) return; + + for (let iter = tree.resolveStack(line.to, 1); iter; iter = iter.next) { + const node = iter.node; + if (node.to <= line.to || node.from > line.to) continue; + const lastChild = node.lastChild; + if ( + tree.length !== state.doc.length && + node.to >= tree.length - INCOMPLETE_PARSE_MARGIN && + lastChild?.to === node.to && + lastChild.type.isError + ) { + continue; + } + + const fold = node.type.prop(foldNodeProp); + if (!fold) continue; + + const range = fold(node, state); + if ( + range && + range.from >= line.from && + range.from <= line.to && + range.to > line.to + ) { + addRange(range); + } + } +} /** * Fold every foldable block, including nested blocks. CodeMirror's built-in @@ -17,7 +59,9 @@ const FULL_PARSE_BUDGET_MS = 100; */ export function foldAllCodeBlocks(view) { const { state } = view; - ensureSyntaxTree(state, state.doc.length, FULL_PARSE_BUDGET_MS); + const tree = + ensureSyntaxTree(state, state.doc.length, FULL_PARSE_BUDGET_MS) ?? + syntaxTree(state); const existing = new Set(); foldedRanges(state).between(0, state.doc.length, (from, to) => { @@ -26,15 +70,18 @@ export function foldAllCodeBlocks(view) { const effects = []; const discovered = new Set(); - for (let lineNumber = 1; lineNumber <= state.doc.lines; lineNumber += 1) { - const line = state.doc.line(lineNumber); - const range = foldable(state, line.from, line.to); - if (!range) continue; - + const addRange = (range) => { const id = `${range.from}:${range.to}`; - if (existing.has(id) || discovered.has(id)) continue; + if (existing.has(id) || discovered.has(id)) return; discovered.add(id); effects.push(foldEffect.of(range)); + }; + + for (let lineNumber = 1; lineNumber <= state.doc.lines; lineNumber += 1) { + const line = state.doc.line(lineNumber); + const serviceRange = findServiceFold(state, line); + if (serviceRange) addRange(serviceRange); + else addSyntaxFolds(state, tree, line, addRange); } if (!effects.length) return false; diff --git a/src/test/editor.tests.js b/src/test/editor.tests.js index a1a88f503..854136268 100644 --- a/src/test/editor.tests.js +++ b/src/test/editor.tests.js @@ -760,20 +760,17 @@ export async function runCodeMirrorTests(writeOutput) { ); runner.test( - "Fold all includes nested blocks and unfold all clears them", + "Fold all includes same-line nested blocks and unfold all clears them", async (test) => { await withEditor( test, async (view) => { test.assert(foldAllCodeBlocks(view), "Nested blocks should fold"); - test.assert( - countFolds(view) >= 2, - "Fold all should retain both outer and nested folds", - ); + test.assertEqual(countFolds(view), 2); test.assert(unfoldAllCodeBlocks(view), "All folds should unfold"); test.assertEqual(countFolds(view), 0); }, - "function outer() {\n if (true) {\n console.log('nested');\n }\n}", + "function outer() { if (true) {\n console.log('nested');\n} }", [javascript()], ); },