From e2afd0a0fe7f0f414a348d3ee590ce5d82895834 Mon Sep 17 00:00:00 2001 From: Ajit Kumar Date: Mon, 14 Sep 2026 07:20:27 +0530 Subject: [PATCH 1/6] feat: plugin quicktool adapter - allow plugin to adapt quicktool - allow plugin to hook save, save event - fix icon size in file explorer --- CONTRIBUTING.md | 2 + docs/file-save-events.md | 29 ++++ docs/quicktools-adapter.md | 30 ++++ src/components/tooltip/index.js | 50 +++++- src/components/tooltip/longPress.js | 125 +++++++++++++++ src/components/tooltip/style.scss | 11 +- src/handlers/quickTools.js | 112 ++++++++++++- src/handlers/quickToolsInit.js | 55 ++++++- src/lib/acode.js | 6 + src/lib/commands.js | 29 ++-- src/lib/editorFile.js | 76 ++++++++- src/lib/quickToolsAdapter.js | 191 ++++++++++++++++++++++ src/lib/quickToolsOverlays.js | 53 ++++++ src/main.js | 2 + src/pages/fileBrowser/fileBrowser.scss | 13 ++ tests/helpers/loadSourceModule.js | 24 +++ tests/unit/fileSaveEvents.test.js | 122 ++++++++++++++ tests/unit/iconTooltips.test.js | 141 ++++++++++++++++ tests/unit/quickToolsAdapter.test.js | 107 +++++++++++++ tests/unit/quickToolsAdapterUi.test.js | 213 +++++++++++++++++++++++++ tests/unit/quickToolsOverlays.test.js | 94 +++++++++++ tests/unit/saveCommands.test.js | 68 ++++++++ 22 files changed, 1523 insertions(+), 30 deletions(-) create mode 100644 docs/file-save-events.md create mode 100644 docs/quicktools-adapter.md create mode 100644 src/components/tooltip/longPress.js create mode 100644 src/lib/quickToolsAdapter.js create mode 100644 src/lib/quickToolsOverlays.js create mode 100644 tests/helpers/loadSourceModule.js create mode 100644 tests/unit/fileSaveEvents.test.js create mode 100644 tests/unit/iconTooltips.test.js create mode 100644 tests/unit/quickToolsAdapter.test.js create mode 100644 tests/unit/quickToolsAdapterUi.test.js create mode 100644 tests/unit/quickToolsOverlays.test.js create mode 100644 tests/unit/saveCommands.test.js diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2457415d2e..aeb58e1073 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -208,3 +208,5 @@ Many font editing software and web-based tools exist for this purpose. Some of t To create plugins for Acode: - [Plugin Starter Repository](https://github.com/Acode-Foundation/acode-plugin) - [Plugin Documentation](https://docs.acode.app/) +- [Custom editor quicktools API](docs/quicktools-adapter.md) +- [Custom-tab Save and Save As](docs/file-save-events.md) diff --git a/docs/file-save-events.md b/docs/file-save-events.md new file mode 100644 index 0000000000..3244e6b226 --- /dev/null +++ b/docs/file-save-events.md @@ -0,0 +1,29 @@ +# Custom-tab Save and Save As + +Custom tabs use Acode's standard Save controls through the `EditorFile` save event. The plugin owns serialization, destination dialogs, writing and dirty tracking. + +```js +if (typeof tab.canSave === "boolean") { + const onSave = (event) => { + event.respondWith(saveDocument(event.saveAs)); + }; + tab.on("save", onSave); + // On plugin disposal: + // tab.off("save", onSave); +} +``` + +`tab.canSave` is read-only: open text-editor tabs qualify automatically; custom tabs need a `save` listener or `onsave` handler. Removing the handler or closing the custom tab disables saving. The handler must implement saving and guard loading, protection and processing. + +The event retains `target`, `preventDefault()` and `stopPropagation()`. It additionally exposes: + +- `saveAs`: `false` for `tab.save()`, `true` for `tab.saveAs()`. +- `respondWith(Promise)`: call synchronously, once, during dispatch to suppress default saving. Resolve `true` after a successful write, `false` for cancellation or no save, and reject for failure. Calling after an `await` or responding twice throws. + +Other observers can still see the event. Existing `preventDefault()` cancellation remains supported. Unhandled text saves use the normal text writer; unhandled custom-tab saves return `false`. + +Concurrent requests on one custom tab share its pending promise; the first request determines Save versus Save As. Different tabs are independent. Invoke the plugin's writer directly to avoid recursive save dispatch. Remove the listener on unload and cancel pending work on tab close; closed tabs cannot publish completion notifications. + +Before resolving success, update the destination and acknowledge only the written revision; newer edits must remain dirty. Acode then emits `save-file` and update notifications. Save All runs sequentially; save-and-close stops after cancellation, failure or remaining edits. Text encoding, format-on-save and autosave retain their existing behavior. + +Feature-detect `tab.canSave`. Older hosts require the plugin's own fallback and commands; their built-in Save menu does not support custom tabs. diff --git a/docs/quicktools-adapter.md b/docs/quicktools-adapter.md new file mode 100644 index 0000000000..3edb45b559 --- /dev/null +++ b/docs/quicktools-adapter.md @@ -0,0 +1,30 @@ +# Custom editor quicktools API + +`acode.registerQuickToolsAdapter(tab, adapter)` connects stock quicktools to one custom `EditorFile`. Register when the editor is ready. Feature-detect the API and use `hideQuickTools: true` while loading, unsupported or on older hosts. Registration allows one adapter per tab and returns an idempotent cleanup function; call it on plugin unload. Tab closure also unregisters the adapter. + +```js +const unregister = acode.registerQuickToolsAdapter?.(tab, { + getState: () => ({ enabled: editor.writable, busy: editor.saving }), + canHandle: action => editor.supports(action), + subscribe: listener => editor.subscribe(listener), // returns unsubscribe + captureSelection: () => editor.captureSelection(), + restoreSelection: selection => editor.restoreSelection(selection), + execute: (action, { signal }) => editor.execute(action, signal), + focus: () => editor.focus(), + cancel: () => editor.cancelPendingInput(), // optional + onError: error => reportError(error), // optional +}); +``` + +The example `editor` is plugin-owned. `getState`, `canHandle`, `subscribe` and `execute` are required; the other callbacks are optional. State and support queries must be synchronous, cheap and side-effect free. Execution, selection and focus callbacks may return promises. Notify subscribers when writability, processing, history or supported operations change; report plugin-owned dialogs as busy. + +- `enabled` controls visibility while respecting the user's quicktools preferences. `busy` blocks editing without changing the layout; the command palette stays available. +- `canHandle(action)` guards execution and Undo/Redo availability. Return `false` for unsupported operations; they never fall through to another editor. +- `execute` receives `{ type: 'text', text }`, `{ type: 'key', key, ctrlKey, shiftKey, altKey, metaKey }`, or `{ type: 'command', command }`. Keys use `KeyboardEvent.key` names, but editors must implement browser-default movement and deletion through their own APIs. +- Command names include `undo`, `redo`, `saveFile`, `find`, `selectall`, `paste`, `movelinesup`, `movelinesdown`, `copylinesup` and `copylinesdown`. +- Modifier button availability is queried as `{ type: 'modifier', key: 'ctrl' | 'shift' | 'alt' | 'meta' }`. Acode combines captured keyboard input with modifiers; adapters do not execute modifier toggles. +- For supported paste operations, Acode reads native clipboard text and delivers a text action. + +Acode captures selection before quicktools take focus and serializes input. Restore snapshots only for the current document revision. Restore focus only for explicit editing input; registration, selection restoration and dialogs must not force the keyboard open. + +Tab changes, native overlays, disabled/busy state and disposal abort the execution signal and cancel modifiers, repeats and queued input. Check the signal and your session/revision after asynchronous work. `cancel` must not synchronously emit another state update. Leave Back handling and the action stack to Acode; keep stock quicktools items, gestures and preferences intact. diff --git a/src/components/tooltip/index.js b/src/components/tooltip/index.js index 9366e6b6ae..a7bd558bfb 100644 --- a/src/components/tooltip/index.js +++ b/src/components/tooltip/index.js @@ -1,25 +1,45 @@ import "./style.scss"; import { animate } from "motion"; +import { installIconTooltips } from "./longPress"; let tooltip; let rafId = null; +let animation; +let icons; + +export function initIconTooltips() { + icons ||= installIconTooltips(document, showTooltip, hideTooltip); + return icons.dismiss; +} function createTooltip() { if (tooltip) return tooltip; tooltip = document.createElement("div"); tooltip.className = "acode-tooltip"; + tooltip.setAttribute("role", "tooltip"); document.body.appendChild(tooltip); return tooltip; } -export function showTooltip(target, text) { +export function showTooltip(target, text, description) { if (!target || !text) return; const $tooltip = createTooltip(); - $tooltip.textContent = text; + animation?.stop(); + $tooltip.replaceChildren(); + const label = document.createElement("div"); + label.textContent = text; + $tooltip.append(label); + if (description) { + const detail = document.createElement("div"); + detail.className = "acode-tooltip-description"; + detail.textContent = description; + $tooltip.append(detail); + } + $tooltip.removeAttribute("aria-hidden"); const rect = target.getBoundingClientRect(); @@ -27,18 +47,32 @@ export function showTooltip(target, text) { cancelAnimationFrame(rafId); } rafId = requestAnimationFrame(() => { + const viewport = window.visualViewport; + const x = viewport?.offsetLeft || 0, + y = viewport?.offsetTop || 0; + const visibleWidth = viewport?.width || window.innerWidth; + const visibleHeight = viewport?.height || window.innerHeight; + $tooltip.style.maxWidth = `${Math.max(0, visibleWidth - 16)}px`; + $tooltip.style.maxHeight = `${Math.max(0, visibleHeight - 16)}px`; const width = $tooltip.offsetWidth; const height = $tooltip.offsetHeight; const left = Math.max( - 8, + x + 8, Math.min( - window.innerWidth - width - 8, + x + visibleWidth - width - 8, rect.left + rect.width / 2 - width / 2, ), ); - const top = Math.max(8, rect.top - height - 10); + const above = rect.top - height - 10; + const top = Math.max( + y + 8, + Math.min( + y + visibleHeight - height - 8, + above >= y + 8 ? above : rect.bottom + 10, + ), + ); $tooltip.style.left = `${left}px`; $tooltip.style.top = `${top}px`; @@ -48,7 +82,7 @@ export function showTooltip(target, text) { rafId = null; return; } - animate( + animation = animate( $tooltip, { opacity: 1, @@ -64,6 +98,8 @@ export function showTooltip(target, text) { export function hideTooltip() { if (!tooltip) return; + animation?.stop(); + tooltip.setAttribute("aria-hidden", "true"); if (rafId !== null) { cancelAnimationFrame(rafId); @@ -74,7 +110,7 @@ export function hideTooltip() { tooltip.style.transform = "translateY(5px)"; return; } - animate( + animation = animate( tooltip, { opacity: 0, diff --git a/src/components/tooltip/longPress.js b/src/components/tooltip/longPress.js new file mode 100644 index 0000000000..419837bd06 --- /dev/null +++ b/src/components/tooltip/longPress.js @@ -0,0 +1,125 @@ +/** One delegated listener set, including icon controls inside open ShadowRoots. */ +export function installIconTooltips(root, show, hide) { + let press, timer, suppressed, observer; + let shadowRoots = []; + const pointers = new Set(); + const labelOf = (node) => + ["data-label", "aria-label", "title"] + .map((name) => node.getAttribute(name)?.trim()) + .find(Boolean); + const targetOf = (event) => { + const path = event.composedPath(); + // Quicktools already own their tooltip, repeat and modifier gestures. + if (path.some((node) => node?.id === "quick-tools")) return; + return path.find( + (node) => + node?.matches?.("button, [role='button'], .icon") && labelOf(node), + ); + }; + function dismiss() { + clearTimeout(timer); + press = undefined; + observer?.disconnect(); + observer = undefined; + for (const shadow of shadowRoots) + shadow.removeEventListener("scroll", dismiss, true); + shadowRoots = []; + hide(); + } + function down(event) { + pointers.add(event.pointerId); + dismiss(); + suppressed = undefined; + if (pointers.size !== 1 || event.button !== 0) return; + const target = targetOf(event); + if (!target) return; + // Element scroll events do not cross a shadow boundary. + shadowRoots = event + .composedPath() + .filter((node) => node?.nodeType === 11 && node.host); + for (const shadow of shadowRoots) + shadow.addEventListener("scroll", dismiss, true); + press = { target, x: event.clientX, y: event.clientY, held: false }; + timer = setTimeout(() => { + if (!press || !target.isConnected) return; + press.held = true; + show(target, labelOf(target), target.dataset.description); + observer = new MutationObserver(() => { + if (!target.isConnected) dismiss(); + }); + // Also observe removal of the tab's host, including nested shadow hosts. + for (const boundary of [root, ...shadowRoots]) + observer.observe(boundary, { childList: true, subtree: true }); + }, 500); + } + function move(event) { + if ( + press && + Math.hypot(event.clientX - press.x, event.clientY - press.y) > 10 + ) + dismiss(); + } + function up(event) { + pointers.delete(event.pointerId); + clearTimeout(timer); + if (press?.held) + suppressed = { target: press.target, until: Date.now() + 750 }; + press = undefined; + } + function cancel(event) { + pointers.delete(event.pointerId); + dismiss(); + } + function click(event) { + if ( + event.detail && + suppressed?.target === targetOf(event) && + Date.now() < suppressed.until + ) { + event.preventDefault(); + event.stopImmediatePropagation(); + } + suppressed = undefined; + } + function contextmenu(event) { + // A control's own menu wins over the shared tooltip. + if (event.defaultPrevented) { + dismiss(); + return; + } + if (press?.held && press.target === targetOf(event)) event.preventDefault(); + } + function reset() { + pointers.clear(); + suppressed = undefined; + dismiss(); + } + const events = { + pointerdown: down, + pointermove: move, + pointerup: up, + pointercancel: cancel, + click, + scroll: dismiss, + keydown: dismiss, + }; + for (const [name, handler] of Object.entries(events)) + root.addEventListener(name, handler, true); + root.addEventListener("contextmenu", contextmenu); + const viewport = root.defaultView?.visualViewport; + root.defaultView?.addEventListener("blur", reset); + viewport?.addEventListener("resize", dismiss); + viewport?.addEventListener("scroll", dismiss); + return { + dismiss: reset, + dispose() { + reset(); + for (const [name, handler] of Object.entries(events)) + root.removeEventListener(name, handler, true); + root.removeEventListener("contextmenu", contextmenu); + root.defaultView?.removeEventListener("blur", reset); + viewport?.removeEventListener("resize", dismiss); + viewport?.removeEventListener("scroll", dismiss); + }, + }; +} diff --git a/src/components/tooltip/style.scss b/src/components/tooltip/style.scss index 2436406dae..fbd3c47c85 100644 --- a/src/components/tooltip/style.scss +++ b/src/components/tooltip/style.scss @@ -11,7 +11,11 @@ font-size: 12px; - white-space: nowrap; + white-space: normal; + overflow-wrap: anywhere; + overflow: hidden; + box-sizing: border-box; + line-height: 1.4; pointer-events: none; @@ -21,4 +25,7 @@ z-index: 999999; } - +.acode-tooltip-description { + margin-top: 3px; + opacity: 0.8; +} diff --git a/src/handlers/quickTools.js b/src/handlers/quickTools.js index 00bcad9aa4..1db59d956c 100644 --- a/src/handlers/quickTools.js +++ b/src/handlers/quickTools.js @@ -24,6 +24,7 @@ import { import { runQuickToolKey } from "cm/quickToolsNavigation"; import quickTools from "components/quickTools"; import actionStack from "lib/actionStack"; +import quickToolsAdapters from "lib/quickToolsAdapter"; import searchHistory from "lib/searchHistory"; import appSettings from "lib/settings"; import searchSettings from "settings/searchSettings"; @@ -50,6 +51,74 @@ let activeSearchState = null; let searchCloseVisibilityObserver = null; /** @type {import("./readOnlyQuickToolsCapture").ReadOnlyQuickToolsCaptureSession | null} */ let readOnlyCaptureSession = null; +let adapterCapture = null; + +function adapterAction(action, value) { + if (action === "insert") return { type: "text", text: String(value ?? "") }; + if (action === "key") { + const event = KeyboardEvent("keydown", getKeys({ keyCode: Number(value) })); + return { type: "key", key: event.key, ...getQuickToolsModifierSnapshot() }; + } + if (action === "command" || action === "search") + return { + type: "command", + command: action === "search" ? "find" : String(value), + }; + if (["shift", "ctrl", "alt", "meta"].includes(action)) + return { type: "modifier", key: action }; + return null; +} + +function handleAdapterCapture(event) { + if (!adapterCapture) return false; + if ( + adapterCapture.target !== editorManager.activeFile || + !quickToolsAdapters.has() + ) { + adapterCapture = null; + return true; + } + const result = captureReadOnlyQuickToolsKey(adapterCapture, { + type: event.type, + key: event.key, + data: event.data, + value: quickTools.$input.value, + inputType: event.inputType, + isComposing: event.isComposing, + }); + adapterCapture = result.session; + if (result.outcome.kind === "pass") { + if (event.type !== "keydown") return false; + if (["Shift", "Control", "Alt", "Meta", "Process"].includes(event.key)) + return true; + quickToolsAdapters.dispatch({ + type: "key", + key: event.key, + ...adapterCapture.modifiers, + }); + } else if (result.outcome.kind === "key") { + const modifiers = adapterCapture.modifiers; + const shiftOnly = + modifiers.shiftKey && + !modifiers.ctrlKey && + !modifiers.altKey && + !modifiers.metaKey; + quickToolsAdapters.dispatch({ + type: "key", + key: shiftOnly + ? mapQuickToolShiftText(result.outcome.key) + : result.outcome.key, + ...modifiers, + }); + } else { + if (result.outcome.kind === "duplicate") event.preventDefault(); + return true; + } + if (!event.isComposing) quickTools.$input.value = ""; + event.preventDefault(); + clearQuickToolsModifierState(); + return true; +} const state = { shift: false, @@ -73,14 +142,17 @@ setQuickToolsModifierInputHandler(handleCodeMirrorQuickToolsTextInput); */ quickTools.$input.addEventListener("beforeinput", (event) => { + if (handleAdapterCapture(event)) return; handleReadOnlyQuickToolsCaptureEvent(event); }); quickTools.$input.addEventListener("compositionend", (event) => { + if (handleAdapterCapture(event)) return; handleReadOnlyQuickToolsCaptureEvent(event); }); quickTools.$input.addEventListener("input", (e) => { + if (handleAdapterCapture(e)) return; if (handleReadOnlyQuickToolsCaptureEvent(e)) return; const key = e.target.value.toUpperCase(); quickTools.$input.value = ""; @@ -113,6 +185,7 @@ quickTools.$input.addEventListener("input", (e) => { }); quickTools.$input.addEventListener("keydown", (e) => { + if (handleAdapterCapture(e)) return; if (handleReadOnlyQuickToolsCaptureEvent(e)) return; const { keyCode, key, which } = e; const keyCombination = getKeys({ keyCode, key, which }); @@ -269,6 +342,7 @@ export function clearQuickToolsModifierState({ restoreFocus = false } = {}) { } export function cancelQuickToolsModifierInput() { + adapterCapture = null; clearReadOnlyCaptureSession(); const changed = clearQuickToolsModifierState(); quickTools.$input.value = ""; @@ -290,6 +364,34 @@ export default function actions(action, value) { const { editor } = editorManager; const { $input, $replaceInput } = quickTools; + const routed = quickToolsAdapters.has() && adapterAction(action, value); + if (routed) { + if (routed.type === "command" && routed.command === "openCommandPalette") + return executeCommand("openCommandPalette", editor); + if (!quickToolsAdapters.available(routed)) return false; + if (routed.type === "modifier") { + quickToolsAdapters.capture(); + state[action] = !state[action]; + events[action].forEach((cb) => cb(state[action])); + if (Object.values(state).some(Boolean)) { + adapterCapture = { + target: editorManager.activeFile, + modifiers: getQuickToolsModifierSnapshot(), + consumed: false, + }; + $input.value = ""; + $input.focus(); + } else { + cancelQuickToolsModifierInput(); + quickToolsAdapters.focus(); + } + return state[action]; + } + const handled = quickToolsAdapters.dispatch(routed); + if (routed.type !== "key" || !routed.key.startsWith("Arrow")) + clearQuickToolsModifierState(); + return handled; + } if (Object.keys(state).includes(action)) { setInput(); @@ -736,7 +838,7 @@ function setHeight(height = 1, save = true) { const { editor, activeFile } = editorManager; // If active file has hideQuickTools, force height to 0 and don't save - if (activeFile?.hideQuickTools) { + if (!quickToolsAdapters.visible(activeFile)) { height = 0; save = false; } @@ -937,6 +1039,10 @@ function getFooterHeight() { } function focusEditor() { + if (quickToolsAdapters.has()) { + quickToolsAdapters.focus(); + return; + } const { editor, activeFile } = editorManager; if (!activeFile?.focused) { return; @@ -1047,6 +1153,10 @@ function dismissReadOnlyQuickToolsInput(view) { } function restoreQuickToolsTargetFocus() { + if (quickToolsAdapters.has()) { + quickToolsAdapters.focus(); + return; + } const codeMirrorView = getCodeMirrorInputView(input); if (codeMirrorView) { if (dismissReadOnlyQuickToolsInput(codeMirrorView)) return; diff --git a/src/handlers/quickToolsInit.js b/src/handlers/quickToolsInit.js index 330cd7d197..2410cd08a2 100644 --- a/src/handlers/quickToolsInit.js +++ b/src/handlers/quickToolsInit.js @@ -6,6 +6,9 @@ import { hideTooltip, showTooltip } from "components/tooltip"; import config from "lib/config"; import appSettings from "lib/settings"; import actions, { cancelQuickToolsModifierInput, key } from "./quickTools"; +import quickToolsAdapters from "lib/quickToolsAdapter"; +import { watchQuickToolsOverlays } from "lib/quickToolsOverlays"; +import { syncQuickToolsVisibility } from "lib/editorFile"; const CONTEXT_MENU_TIMEOUT = 500; const MOVE_X_THRESHOLD = 50; @@ -56,6 +59,41 @@ function clearTouchFeedback() { */ export default function init() { const { $footer, $toggler, $input } = quickTools; + let adapterWasActive = false, + visible; + const refreshAdapter = () => { + const hasAdapter = quickToolsAdapters.has(); + if (!hasAdapter && !adapterWasActive) { + visible = undefined; + return; + } + adapterWasActive = hasAdapter; + const nextVisible = quickToolsAdapters.visible(); + if (visible !== nextVisible) { + visible = nextVisible; + syncQuickToolsVisibility(editorManager.activeFile); + } + updateHistoryButtons(); + }; + const cancelAdapterInput = () => { + clearTimeout(timeout); + touchcancel(); + reset(); + cancelQuickToolsModifierInput(); + quickToolsAdapters.cancel(); + }; + quickToolsAdapters.subscribe((change) => { + if (change?.cancelled && change.tab === editorManager.activeFile) + cancelAdapterInput(); + refreshAdapter(); + }); + watchQuickToolsOverlays(quickToolsAdapters, cancelAdapterInput); + $footer.addEventListener( + "pointerdown", + () => quickToolsAdapters.capture(), + true, + ); + $footer.addEventListener("keydown", () => quickToolsAdapters.capture(), true); $toggler.addEventListener("click", (e) => { e.preventDefault(); @@ -102,7 +140,11 @@ export default function init() { }); editorManager.on("editor-state-changed", updateHistoryButtons); - editorManager.on("switch-file", cancelQuickToolsModifierInput); + editorManager.on("switch-file", () => { + if (adapterWasActive || quickToolsAdapters.has()) cancelAdapterInput(); + else cancelQuickToolsModifierInput(); + quickToolsAdapters.sync(); + }); appSettings.on("update:quicktoolsItems:after", () => { setTimeout(updateHistoryButtons, 100); @@ -361,7 +403,7 @@ function oncontextmenu(e) { timeout = setTimeout(dispatchEventWithTimeout, time); }; - if (activeFile.focused) { + if (activeFile.focused && !quickToolsAdapters.has()) { focusEditorIfEditable(editor); } dispatchEventWithTimeout(); @@ -413,6 +455,15 @@ function updateQuickToolsState() { } function updateHistoryButtons() { + if (quickToolsAdapters.has()) { + for (const command of ["undo", "redo"]) { + updateHistoryButton( + command, + !quickToolsAdapters.available({ type: "command", command }), + ); + } + return; + } const { editor, activeFile } = editorManager; const disabled = !editor || activeFile?.type !== "editor"; diff --git a/src/lib/acode.js b/src/lib/acode.js index 668a5356db..50212a4010 100644 --- a/src/lib/acode.js +++ b/src/lib/acode.js @@ -84,6 +84,7 @@ import helpers from "utils/helpers"; import KeyboardEvent from "utils/keyboardEvent"; import Url from "utils/Url"; import config from "./config"; +import quickToolsAdapters from "./quickToolsAdapter"; import webview from "./webview"; class Acode { @@ -1032,6 +1033,11 @@ class Acode { return command; } + /** Register input, selection and availability handlers for one custom tab. */ + registerQuickToolsAdapter(tab, adapter) { + return quickToolsAdapters.register(tab, adapter); + } + removeCommand(name) { if (!name) return; removeExternalCommand(name); diff --git a/src/lib/commands.js b/src/lib/commands.js index 7d7082d042..ff35a4df65 100644 --- a/src/lib/commands.js +++ b/src/lib/commands.js @@ -78,7 +78,7 @@ function resolveExactFile(referenceFile) { export function canSaveFile(file = editorManager.activeFile) { return ( - file?.type === "editor" && + (file?.type === "editor" || file?.canSave === true) && typeof file.save === "function" && typeof file.saveAs === "function" ); @@ -139,8 +139,11 @@ async function closeTabs(files, options = {}) { } for (const file of [...closableFiles]) { - if (save) { - await file.save(); + if (save && file.isUnsaved) { + if (!canSaveFile(file)) return false; + const saved = await file.save(); + if (saved === false || file.hasUnsavedChanges?.() || file.isUnsaved) + return false; } await file.remove(true, { silentPinned: true }); @@ -206,10 +209,14 @@ export default { strings["save all changes warning"], ); if (!doSave) return; - editorManager.files.forEach((file) => { - file.save(); - file.isUnsaved = false; - }); + for (const file of [...editorManager.files]) { + if (!file.isUnsaved) continue; + if (!canSaveFile(file)) return false; + const saved = await file.save(); + if (saved === false || file.hasUnsavedChanges?.() || file.isUnsaved) + return false; + } + return true; }, "close-current-tab"() { editorManager.activeFile?.remove(); @@ -493,8 +500,8 @@ export default { try { const { activeFile } = editorManager; if (!canSaveFile(activeFile)) return; - await activeFile.save(); - if (showToast) { + const saved = await activeFile.save(); + if (showToast && (activeFile.type === "editor" || saved === true)) { toast(strings["file saved"]); } } catch (error) { @@ -505,8 +512,8 @@ export default { try { const { activeFile } = editorManager; if (!canSaveFile(activeFile)) return; - await activeFile.saveAs(); - if (showToast) { + const saved = await activeFile.saveAs(); + if (showToast && (activeFile.type === "editor" || saved === true)) { toast(strings["file saved"]); } } catch (error) { diff --git a/src/lib/editorFile.js b/src/lib/editorFile.js index f5074fa687..6a06481349 100644 --- a/src/lib/editorFile.js +++ b/src/lib/editorFile.js @@ -15,6 +15,7 @@ import confirm from "dialogs/confirm"; import DOMPurify from "dompurify"; import startDrag from "handlers/editorFileTab"; import actions from "handlers/quickTools"; +import quickToolsAdapters from "lib/quickToolsAdapter"; import { openTabContextMenuOnRelease } from "handlers/tabContextMenu"; import tag from "html-tag-js"; import mimeTypes from "mime-types"; @@ -61,9 +62,9 @@ function getMainCSSStyleSheet() { return null; } -function syncQuickToolsVisibility(file) { +export function syncQuickToolsVisibility(file) { const { $toggler } = quickTools; - const hideForFile = !!file?.hideQuickTools; + const hideForFile = !quickToolsAdapters.visible(file); clearTimeout($toggler._hideTimeout); if (hideForFile || !appSettings.value.floatingButton) { @@ -461,6 +462,7 @@ export default class EditorFile { */ #loadOptions; #loadPromise = null; + #pendingSave = null; /** * Weather file is changed and needs to be saved * @type {boolean} @@ -1441,7 +1443,6 @@ export default class EditorFile { * @returns {Promise} true if file is saved, false if not. */ save() { - if (this.type !== "editor") return Promise.resolve(false); return this.#save(false); } @@ -1450,10 +1451,19 @@ export default class EditorFile { * @returns {Promise} true if file is saved, false if not. */ saveAs() { - if (this.type !== "editor") return Promise.resolve(false); return this.#save(true); } + /** Custom tabs opt into the standard save controls through their save event. */ + get canSave() { + return ( + !!this.#tab && + (this.type === "editor" || + typeof this.onsave === "function" || + this.#events.save.length > 0) + ); + } + setReadOnly(value) { const readOnly = !!value; this.readOnly = readOnly; @@ -1964,10 +1974,39 @@ export default class EditorFile { // } #save(as) { - const event = createFileEvent(this); - this.#emit("save", event); + if (!this.canSave) return Promise.resolve(false); + if (this.type === "editor") return this.#dispatchSave(as); + if (this.#pendingSave) return this.#pendingSave; + // Set the promise before dispatch so re-entrant requests also share it. + this.#pendingSave = Promise.resolve() + .then(() => (this.canSave ? this.#dispatchSave(as) : false)) + .finally(() => { + this.#pendingSave = null; + }); + return this.#pendingSave; + } - if (event.defaultPrevented) return Promise.resolve(false); + #dispatchSave(as) { + const event = new SaveFileEvent(this, as); + try { + this.#emit("save", event); + } catch (error) { + // A later observer can fail after a handler has supplied a promise. + event.response?.catch(() => {}); + return Promise.reject(error); + } finally { + event.finishDispatch(); + } + if (event.response) + return event.response.then((saved) => { + if (!saved || !this.#tab) return false; + editorManager.onupdate("save-file"); + editorManager.emit("update", "save-file"); + editorManager.emit("save-file", this); + return true; + }); + if (event.defaultPrevented || this.type !== "editor") + return Promise.resolve(false); return Promise.all([this.flushCacheWrite(), saveFile(this, as)]); } @@ -2128,3 +2167,26 @@ class FileEvent { return this.#defaultPrevented; } } + +class SaveFileEvent extends FileEvent { + #dispatching = true; + #response; + saveAs; + constructor(file, saveAs) { + super(file); + this.saveAs = saveAs; + } + /** Claim this save synchronously; resolve true only after a successful write. */ + respondWith(result) { + if (!this.#dispatching || this.#response) + throw new Error("respondWith must be called once during the save event."); + this.preventDefault(); + this.#response = Promise.resolve(result).then((saved) => saved === true); + } + get response() { + return this.#response; + } + finishDispatch() { + this.#dispatching = false; + } +} diff --git a/src/lib/quickToolsAdapter.js b/src/lib/quickToolsAdapter.js new file mode 100644 index 0000000000..15415988e2 --- /dev/null +++ b/src/lib/quickToolsAdapter.js @@ -0,0 +1,191 @@ +import { hasQuickToolsOverlay } from "./quickToolsOverlays"; + +/** + * Per-tab input routing for custom editors. No editor engine or UI dependencies. + * An adapter owns all editing actions on its tab: unsupported actions never fall + * through to the code editor, including while an asynchronous action is pending. + */ +export function createQuickToolsAdapterRegistry( + getActiveTab, + resolveAction = async (action) => action, + isBlocked = () => false, +) { + const entries = new Map(); + const listeners = new Set(); + let activeTab, + blocked = false; + const notify = (change) => listeners.forEach((listener) => listener(change)); + const current = () => entries.get(getActiveTab()); + function cancel(entry) { + if (!entry) return; + entry.controller.abort(); + entry.controller = new AbortController(); + entry.queue = Promise.resolve(); + entry.pending = 0; + entry.selection = undefined; + entry.adapter.cancel?.(); + } + const valid = (entry, signal) => + !signal.aborted && + !blocked && + !isBlocked() && + current() === entry && + entry.adapter.getState().enabled && + !entry.adapter.getState().busy; + return { + subscribe(listener) { + listeners.add(listener); + return () => listeners.delete(listener); + }, + register(tab, adapter) { + if ( + !tab || + !adapter?.getState || + !adapter?.canHandle || + !adapter?.execute || + !adapter?.subscribe + ) + throw new TypeError( + "A quicktools adapter needs a tab, state, availability and execution handlers.", + ); + if (entries.has(tab)) + throw new Error("This tab already has a quicktools adapter."); + const entry = { + tab, + adapter, + controller: new AbortController(), + queue: Promise.resolve(), + pending: 0, + selection: undefined, + }; + entries.set(tab, entry); + const unsubscribe = adapter.subscribe(() => { + const state = adapter.getState(); + const cancelled = !state.enabled || state.busy; + if (cancelled) cancel(entry); + notify({ tab, cancelled }); + }); + const dispose = () => { + if (entries.get(tab) !== entry) return; + entries.delete(tab); + unsubscribe?.(); + tab.off?.("close", dispose); + cancel(entry); + notify({ tab, cancelled: true }); + }; + tab.on?.("close", dispose); + notify(); + return dispose; + }, + sync() { + const next = getActiveTab(); + if (next !== activeTab) { + cancel(entries.get(activeTab)); + activeTab = next; + } + notify(); + }, + setBlocked(value) { + if (blocked === value) return; + blocked = value; + if (value) cancel(current()); + notify(); + }, + has(tab = getActiveTab()) { + return entries.has(tab); + }, + visible(tab = getActiveTab()) { + const entry = entries.get(tab); + return entry ? !!entry.adapter.getState().enabled : !tab?.hideQuickTools; + }, + available(action) { + const entry = current(); + return ( + !!entry && + !blocked && + !isBlocked() && + entry.adapter.getState().enabled && + !entry.adapter.getState().busy && + entry.adapter.canHandle(action) + ); + }, + capture() { + const entry = current(); + if (!entry || blocked || isBlocked() || entry.selection) return; + // Repeated input must capture the selection produced by the preceding + // action, rather than restoring an older caret when the queue catches up. + entry.selection = entry.pending + ? entry.queue.then(() => entry.adapter.captureSelection?.()) + : Promise.resolve(entry.adapter.captureSelection?.()); + // Capture failures are reported by dispatch, without an unhandled rejection. + entry.selection.catch(() => {}); + }, + dispatch(action) { + const entry = current(); + if (!entry) return false; + if (!this.available(action)) return true; + const signal = entry.controller.signal, + selection = entry.selection; + entry.selection = undefined; + entry.pending++; + entry.queue = entry.queue + .then(async () => { + if (!valid(entry, signal) || !this.available(action)) return; + if (selection) { + const snapshot = await selection; + if (!valid(entry, signal)) return; + await entry.adapter.restoreSelection?.(snapshot); + } + if (!valid(entry, signal) || !this.available(action)) return; + const resolved = await resolveAction(action); + if (valid(entry, signal) && this.available(action)) + await entry.adapter.execute(resolved, { signal }); + }) + .catch((error) => { + if (valid(entry, signal)) entry.adapter.onError?.(error); + }) + .finally(() => { + if (signal === entry.controller.signal) entry.pending--; + }); + return true; + }, + focus() { + const entry = current(); + if (entry && !blocked && entry.adapter.getState().enabled) { + const signal = entry.controller.signal; + Promise.resolve() + .then(() => { + if (valid(entry, signal)) return entry.adapter.focus?.(); + }) + .catch((error) => { + if (valid(entry, signal)) entry.adapter.onError?.(error); + }); + } + }, + cancel() { + cancel(current()); + }, + }; +} + +export default createQuickToolsAdapterRegistry( + () => globalThis.editorManager?.activeFile, + async (action) => { + const paste = + (action.type === "command" && action.command === "paste") || + (action.type === "key" && + (action.ctrlKey || action.metaKey) && + action.key.toLowerCase() === "v"); + if (!paste) return action; + const text = await new Promise((resolve, reject) => { + const clipboard = globalThis.cordova?.plugins?.clipboard; + if (!clipboard) { + reject(new Error("Clipboard is unavailable.")); + return; + } + clipboard.paste(resolve, reject); + }); + return { type: "text", text: String(text || "") }; + }, + hasQuickToolsOverlay, +); diff --git a/src/lib/quickToolsOverlays.js b/src/lib/quickToolsOverlays.js new file mode 100644 index 0000000000..ac812a35c9 --- /dev/null +++ b/src/lib/quickToolsOverlays.js @@ -0,0 +1,53 @@ +// Native dialogs and menus are direct children of body or the app container. +// Never observe editor content (including an adapter's ShadowRoot/iframe). +const containers = () => + [globalThis.document?.body, globalThis.app].filter( + (node, index, nodes) => + node?.nodeType === 1 && nodes.indexOf(node) === index, + ); +export function hasQuickToolsOverlay() { + return containers().some((root) => + [...root.children].some((node) => + node.matches(".prompt, #palette, .context-menu, .mask"), + ), + ); +} + +export function watchQuickToolsOverlays(registry, cancelInput) { + let watching = false, + wasBlocked = false; + const update = () => { + const blocked = hasQuickToolsOverlay(); + if (blocked && !wasBlocked) cancelInput(); + wasBlocked = blocked; + registry.setBlocked(blocked); + }; + const observer = new MutationObserver(update); + const sync = () => { + const active = registry.has(); + if (active === watching) return; + watching = active; + if (active) { + for (const node of containers()) + observer.observe(node, { childList: true }); + document.addEventListener("focusin", update, true); + document.addEventListener("pointerdown", update, true); + update(); + } else { + wasBlocked = false; + observer.disconnect(); + document.removeEventListener("focusin", update, true); + document.removeEventListener("pointerdown", update, true); + registry.setBlocked(false); + } + }; + const unsubscribe = registry.subscribe(sync); + sync(); + return () => { + unsubscribe(); + observer.disconnect(); + document.removeEventListener("focusin", update, true); + document.removeEventListener("pointerdown", update, true); + registry.setBlocked(false); + }; +} diff --git a/src/main.js b/src/main.js index 87b6f9a445..f6d4cd0f05 100644 --- a/src/main.js +++ b/src/main.js @@ -28,6 +28,7 @@ import Contextmenu from "components/contextmenu"; import Sidebar from "components/sidebar"; import tile from "components/tile"; import toast from "components/toast"; +import { initIconTooltips } from "components/tooltip"; import alert from "dialogs/alert"; import confirm from "dialogs/confirm"; import intentHandler, { processPendingIntents } from "handlers/intent"; @@ -704,6 +705,7 @@ async function loadApp() { //#region Add event listeners initModes(); quickToolsInit(); + editorManager.on("switch-file", initIconTooltips()); sidebarApps.init($sidebar); await sidebarApps.loadApps(); editorManager.onupdate = onEditorUpdate; diff --git a/src/pages/fileBrowser/fileBrowser.scss b/src/pages/fileBrowser/fileBrowser.scss index 769090e05d..8d55d69766 100644 --- a/src/pages/fileBrowser/fileBrowser.scss +++ b/src/pages/fileBrowser/fileBrowser.scss @@ -4,6 +4,19 @@ } .tile { + > .icon:first-child { + --file-icon-size: 24px; + font-size: var(--file-icon-size); + background-size: var(--file-icon-size); + background-position: center; + + > svg, > img { + width: var(--file-icon-size); + height: var(--file-icon-size); + object-fit: contain; + } + } + &[data-storage-type="notification"] { background-color: rgb(153, 153, 255); background-color: var(--primary-color); diff --git a/tests/helpers/loadSourceModule.js b/tests/helpers/loadSourceModule.js new file mode 100644 index 0000000000..a64a8e3122 --- /dev/null +++ b/tests/helpers/loadSourceModule.js @@ -0,0 +1,24 @@ +import { readFileSync } from "node:fs"; +import path from "node:path"; +import vm from "node:vm"; +import { transformSync } from "@babel/core"; + +const compiled = new Map(); +// Compile the actual source with Acode's JSX transform; replace only its environment. +export function loadSourceModule(relative, dependencies, globals = {}) { + const filename = path.resolve(relative); + if (!compiled.has(filename)) compiled.set(filename, transformSync(readFileSync(filename, "utf8"), { + filename, babelrc: false, configFile: false, + presets: [["@babel/preset-env", { targets: { node: "current" }, modules: "commonjs" }]], + plugins: ["html-tag-js/jsx/syntax-parser.js", "html-tag-js/jsx/jsx-to-tag.js"], + }).code); + const exports = {}; + vm.runInNewContext(compiled.get(filename), { + exports, Promise, console, setTimeout, clearTimeout, + require(id) { + if (!(id in dependencies)) throw Error(`Unexpected import: ${id}`); + return dependencies[id]; + }, ...globals, + }, { filename }); + return exports; +} diff --git a/tests/unit/fileSaveEvents.test.js b/tests/unit/fileSaveEvents.test.js new file mode 100644 index 0000000000..8374b7368f --- /dev/null +++ b/tests/unit/fileSaveEvents.test.js @@ -0,0 +1,122 @@ +// @vitest-environment happy-dom +import { afterEach, expect, it, vi } from "vitest"; +import { EditorState, EditorSelection } from "@codemirror/state"; +import tag from "html-tag-js"; +import { loadSourceModule } from "../helpers/loadSourceModule"; + +const deferred = () => { let resolve, reject; const promise = new Promise((a, b) => { resolve = a; reject = b; }); return { promise, resolve, reject }; }; +function setup() { + const write = vi.fn(async () => true); + const manager = { files: [], getFile: () => null, addFile: file => manager.files.push(file), emit: vi.fn(), onupdate: vi.fn(), activeFile: null }; + const defaults = Object.fromEntries([ + "fileSystem", "components/quickTools", "components/sidebar", "components/toast", "dialogs/confirm", "handlers/editorFileTab", "handlers/quickTools", "lib/quickToolsAdapter", "handlers/tabContextMenu", "dompurify", "mime-types", "utils/codeHighlight", "utils/Path", "utils/remoteFilePreview", "utils/Url", "./loadPlugins", "./openFolder", "./run", "cm/editorReadOnly", + ].map(id => [id, {}])); + const { default: EditorFile } = loadSourceModule("src/lib/editorFile.js", { + ...defaults, + "components/sidebar": { hide: vi.fn() }, + "@codemirror/state": { EditorState, EditorSelection }, + "cm/editorUtils": { getDocText: doc => doc.toString() }, + "cm/modelist": { getModeForPath: () => "text", getMode: () => ({ name: "text", extensions: [], getExtension: () => [] }) }, + "html-tag-js": tag, + "components/tile": options => { + const element = document.createElement("li"); + element.innerHTML = ''; + element.tail = vi.fn(); + element.text = options.text; + return element; + }, + "utils/helpers": { normalizeMtime: () => null, getIconForFile: () => "file" }, + "./config": { DEFAULT_FILE_SESSION: "default" }, + "./settings": { value: {}, on: vi.fn(), off: vi.fn() }, + "./saveFile": write, + }, { document, window, tag, editorManager: manager }); + EditorFile.prototype.setMode = vi.fn(); // Language setup is unrelated to save routing. + const file = (type = "docs") => new EditorFile("test.txt", { + id: String(manager.files.length), type, content: document.createElement("div"), render: false, text: "original", + }); + return { file, write, manager }; +} +afterEach(() => document.body.replaceChildren()); + +it("enables only custom tabs with listeners or onsave, and never uses the text writer", async () => { + const f = setup(), doc = f.file(), terminal = f.file("terminal"); + expect(doc.canSave).toBe(false); + expect(await doc.save()).toBe(false); + expect(await terminal.saveAs()).toBe(false); + const handler = e => e.respondWith(Promise.resolve(true)); + doc.on("save", handler); + expect(doc.canSave).toBe(true); + expect(await doc.save()).toBe(true); + doc.off("save", handler); + expect(doc.canSave).toBe(false); + doc.onsave = handler; + expect(await doc.saveAs()).toBe(true); + doc.onsave = undefined; + expect(doc.canSave).toBe(false); + expect(f.write).not.toHaveBeenCalled(); +}); +it("awaits one response, distinguishes Save As and shares repeated requests per tab", async () => { + const f = setup(), one = f.file(), two = f.file(), hold = deferred(); + const events = [], handler = vi.fn(e => { events.push(e); e.respondWith(hold.promise); }); + one.on("save", handler); + two.on("save", e => e.respondWith(Promise.resolve(false))); + const saving = one.saveAs(); + expect(one.save()).toBe(saving); + await Promise.resolve(); + expect(handler).toHaveBeenCalledOnce(); + expect(events[0].saveAs).toBe(true); + expect(events[0].target).toBe(one); + expect(await two.save()).toBe(false); + hold.resolve(true); + expect(await saving).toBe(true); + await one.save(); + expect(events[1].saveAs).toBe(false); +}); +it("rejects failures, duplicate and late responses, and releases pending requests", async () => { + const f = setup(), file = f.file(); + let event; + const fail = e => { event = e; e.respondWith(Promise.reject(Error("disk full"))); }; + file.on("save", fail); + await expect(file.save()).rejects.toThrow("disk full"); + expect(() => event.respondWith(Promise.resolve(true))).toThrow(/once/); + file.off("save", fail); + const duplicate = e => { e.respondWith(Promise.resolve(true)); e.respondWith(Promise.resolve(true)); }; + file.on("save", duplicate); + await expect(file.save()).rejects.toThrow(/once/); + file.off("save", duplicate); + file.on("save", e => { event = e; }); + expect(await file.save()).toBe(false); + expect(() => event.respondWith(Promise.resolve(true))).toThrow(/during/); +}); +it("preserves text fallback and existing cancellation/listener ordering", async () => { + const f = setup(), file = f.file("editor"); + file.flushCacheWrite = vi.fn(async () => {}); + expect(file.canSave).toBe(true); + await file.saveAs(); + expect(f.write).toHaveBeenCalledWith(file, true); + const cancel = e => e.preventDefault(), observe = vi.fn(); + file.onsave = cancel; + file.on("save", observe); + expect(await file.save()).toBe(false); + expect(observe).toHaveBeenCalledOnce(); + expect(f.write).toHaveBeenCalledOnce(); + file.onsave = e => { e.preventDefault(); e.stopPropagation(); }; + await file.save(); + expect(observe).toHaveBeenCalledOnce(); + file.onsave = e => e.respondWith(Promise.resolve(true)); + expect(await file.save()).toBe(true); + expect(f.write).toHaveBeenCalledOnce(); +}); + + it("drops completion notifications after closing the captured custom tab", async () => { + const f = setup(), file = f.file(), hold = deferred(); + f.file(); + file.on("save", e => e.respondWith(hold.promise)); + const saving = file.save(); + await Promise.resolve(); + await file.remove(true); + expect(file.canSave).toBe(false); + hold.resolve(true); + expect(await saving).toBe(false); + expect(f.manager.emit.mock.calls.some(([name]) => name === "save-file")).toBe(false); +}); diff --git a/tests/unit/iconTooltips.test.js b/tests/unit/iconTooltips.test.js new file mode 100644 index 0000000000..0423eaa948 --- /dev/null +++ b/tests/unit/iconTooltips.test.js @@ -0,0 +1,141 @@ +// @vitest-environment happy-dom +import { afterEach, expect, it, vi } from "vitest"; +import { installIconTooltips } from "../../src/components/tooltip/longPress"; +let controller; +afterEach(() => { + controller?.dispose(); + vi.useRealTimers(); + document.body.replaceChildren(); +}); +function fixture() { + vi.useFakeTimers(); + const owner = document.createElement("div"); + document.body.append(owner); + const root = owner.attachShadow({ mode: "open" }); + root.innerHTML = ''; + const button = root.querySelector("button"), + icon = root.querySelector("svg"); + const show = vi.fn(), + hide = vi.fn(); + controller = installIconTooltips(document, show, hide); + const send = (name, options = {}) => + icon.dispatchEvent( + new PointerEvent(name, { + bubbles: true, + composed: true, + cancelable: true, + pointerId: 1, + pointerType: "touch", + button: 0, + clientX: 20, + clientY: 20, + ...options, + }), + ); + return { root, owner, button, icon, show, hide, send }; +} +it("reads metadata through Shadow DOM and consumes the release click without activating the icon", async () => { + const f = fixture(), + clicked = vi.fn(); + f.button.onclick = clicked; + f.button.dataset.description = "Existing description"; + f.send("pointerdown"); + await vi.advanceTimersByTimeAsync(499); + expect(f.show).not.toHaveBeenCalled(); + await vi.advanceTimersByTimeAsync(1); + expect(f.show).toHaveBeenCalledWith( + f.button, + "Format", + "Existing description", + ); + f.send("pointerup"); + f.icon.dispatchEvent( + new MouseEvent("click", { + bubbles: true, + composed: true, + cancelable: true, + detail: 1, + }), + ); + expect(clicked).not.toHaveBeenCalled(); + f.send("pointerdown"); + f.send("pointerup"); + f.icon.dispatchEvent( + new MouseEvent("click", { + bubbles: true, + composed: true, + cancelable: true, + detail: 1, + }), + ); + expect(clicked).toHaveBeenCalledOnce(); +}); +it("cancels scrolling, movement, extra touches and removal; leaves stock quicktools alone", async () => { + const f = fixture(); + for (const cancel of [ + () => f.send("pointermove", { clientX: 40 }), + () => document.dispatchEvent(new Event("scroll")), + () => f.root.dispatchEvent(new Event("scroll")), + () => f.send("pointercancel"), + ]) { + f.send("pointerdown"); + cancel(); + f.send("pointerup"); + await vi.advanceTimersByTimeAsync(600); + } + expect(f.show).not.toHaveBeenCalled(); + f.send("pointerdown"); + f.send("pointerdown", { pointerId: 2 }); + await vi.advanceTimersByTimeAsync(600); + expect(f.show).not.toHaveBeenCalled(); + f.send("pointerup", { pointerId: 2 }); + f.send("pointerup"); + f.send("pointerdown"); + await vi.advanceTimersByTimeAsync(500); + expect(f.show).toHaveBeenCalledWith(f.button, "Format", undefined); + f.hide.mockClear(); + f.owner.remove(); + await vi.advanceTimersByTimeAsync(0); + expect(f.hide).toHaveBeenCalled(); + document.body.append(f.owner); + f.owner.id = "quick-tools"; + f.send("pointerup"); + f.show.mockClear(); + f.send("pointerdown"); + await vi.advanceTimersByTimeAsync(600); + expect(f.show).not.toHaveBeenCalled(); +}); +it("uses existing global labels in precedence order and lets an existing context menu win", async () => { + const f = fixture(); + f.button.setAttribute("aria-label", "Accessible label"); + f.button.title = "Title label"; + for (const [remove, label] of [ + [null, "Format"], + ["data-label", "Accessible label"], + ["aria-label", "Title label"], + ]) { + if (remove) f.button.removeAttribute(remove); + f.send("pointerdown"); + await vi.advanceTimersByTimeAsync(500); + expect(f.show).toHaveBeenLastCalledWith(f.button, label, undefined); + f.send("pointerup"); + } + f.button.addEventListener("contextmenu", (event) => event.preventDefault()); + f.send("pointerdown"); + f.hide.mockClear(); + f.icon.dispatchEvent( + new Event("contextmenu", { + bubbles: true, + composed: true, + cancelable: true, + }), + ); + await vi.advanceTimersByTimeAsync(600); + expect(f.hide).toHaveBeenCalled(); + f.send("pointerup"); + f.button.removeAttribute("title"); + f.show.mockClear(); + f.send("pointerdown"); + await vi.advanceTimersByTimeAsync(600); + expect(f.show).not.toHaveBeenCalled(); +}); diff --git a/tests/unit/quickToolsAdapter.test.js b/tests/unit/quickToolsAdapter.test.js new file mode 100644 index 0000000000..3da3ed38fb --- /dev/null +++ b/tests/unit/quickToolsAdapter.test.js @@ -0,0 +1,107 @@ +import { describe, it, expect, vi } from "vitest"; +import { createQuickToolsAdapterRegistry } from "../../src/lib/quickToolsAdapter"; + +function setup(resolve) { + let tab = { hideQuickTools: true }; + const original = tab; + const registry = createQuickToolsAdapterRegistry(() => tab, resolve); + let state = { enabled: true, busy: false }, notify; + const adapter = { + getState: () => state, + canHandle: (action) => action.type !== "command" || action.command === "undo", + subscribe: (fn) => { notify = fn; return vi.fn(); }, + execute: vi.fn(async () => {}), + captureSelection: vi.fn(async () => ({ anchor: 3 })), + restoreSelection: vi.fn(async () => {}), + focus: vi.fn(), onError: vi.fn(), cancel: vi.fn(), + }; + const dispose = registry.register(tab, adapter); + registry.sync(); + return { registry, adapter, original, dispose, + change: (value) => { tab = value; registry.sync(); }, + state: (value) => { state = { ...state, ...value }; notify(); }, + }; +} +const insert = { type: "text", text: "x" }; +const settle = () => new Promise((resolve) => setTimeout(resolve, 0)); + +describe("custom-tab quicktools", () => { + it("uses editor availability, preserves configured opt-out for unadapted tabs, and never falls through unsupported actions", async () => { + const f = setup(); + expect(f.registry.visible()).toBe(true); + expect(f.registry.dispatch({ type: "command", command: "movelinesup" })).toBe(true); + f.state({ busy: true }); + expect(f.registry.visible()).toBe(true); + expect(f.registry.available(insert)).toBe(false); + f.registry.dispatch(insert); + await settle(); + expect(f.adapter.execute).not.toHaveBeenCalled(); + f.state({ enabled: false }); + expect(f.registry.visible()).toBe(false); + f.change({ type: "editor", hideQuickTools: false }); + expect(f.registry.visible()).toBe(true); + expect(f.registry.dispatch(insert)).toBe(false); + f.change({ type: "terminal", hideQuickTools: false }); + expect(f.registry.dispatch(insert)).toBe(false); + }); + it("captures once before focus moves and preserves sequential repeated input", async () => { + const f = setup(); + f.registry.capture(); f.registry.capture(); + for (let i = 0; i < 3; i++) f.registry.dispatch({ type: "text", text: String(i) }); + await settle(); + expect(f.adapter.captureSelection).toHaveBeenCalledOnce(); + expect(f.adapter.restoreSelection).toHaveBeenCalledOnce(); + expect(f.adapter.execute.mock.calls.map(([action]) => action.text)).toEqual(["0", "1", "2"]); + expect(f.adapter.focus).not.toHaveBeenCalled(); + }); + it("drops clipboard work and queued repeats after switching away and back", async () => { + let finish; + const f = setup(() => new Promise((resolve) => { finish = resolve; })); + f.registry.dispatch(insert); f.registry.dispatch(insert); + await settle(); + f.change({ type: "editor" }); f.change(f.original); + finish(insert); + await settle(); + expect(f.adapter.execute).not.toHaveBeenCalled(); + }); + it("cancels pending selection restoration when a dialog opens or the adapter closes", async () => { + for (const close of [false, true]) { + const f = setup(); let finish; + f.adapter.captureSelection.mockImplementation(() => new Promise((resolve) => { finish = resolve; })); + f.registry.capture(); f.registry.dispatch(insert); + await settle(); + if (close) f.dispose(); else f.registry.setBlocked(true); + finish({ anchor: 3 }); await settle(); + expect(f.adapter.restoreSelection).not.toHaveBeenCalled(); + expect(f.adapter.execute).not.toHaveBeenCalled(); + } + }); + it("isolates two custom tabs and makes stale cleanup harmless", async () => { + const f = setup(); const second = { hideQuickTools: true }, execute = vi.fn(); + f.registry.register(second, { ...f.adapter, execute }); + f.change(second); f.registry.dispatch(insert); await settle(); + expect(execute).toHaveBeenCalledOnce(); + expect(f.adapter.execute).not.toHaveBeenCalled(); + f.dispose(); f.dispose(); + expect(f.registry.has(second)).toBe(true); + }); + it("starts a fresh queue after cancellation even if the old clipboard never resolves", async () => { + let first = true; + const f = setup(action => { if (first) { first = false; return new Promise(() => {}); } return action; }); + f.registry.dispatch(insert); await settle(); + f.change({ type: "editor" }); f.change(f.original); + f.registry.dispatch(insert); await settle(); + expect(f.adapter.execute).toHaveBeenCalledOnce(); + }); + it("captures the updated caret for rapid taps and cancels deferred focus", async () => { + const f = setup(); let caret = 0; + f.adapter.captureSelection.mockImplementation(async () => caret); + f.adapter.restoreSelection.mockImplementation(async (value) => { caret = value; }); + f.adapter.execute.mockImplementation(async () => { caret++; }); + for (let i = 0; i < 4; i++) { f.registry.capture(); f.registry.dispatch(insert); } + await settle(); expect(caret).toBe(4); + f.registry.focus(); f.registry.cancel(); await settle(); + expect(f.adapter.focus).not.toHaveBeenCalled(); + }); + +}); diff --git a/tests/unit/quickToolsAdapterUi.test.js b/tests/unit/quickToolsAdapterUi.test.js new file mode 100644 index 0000000000..749ce5bbc2 --- /dev/null +++ b/tests/unit/quickToolsAdapterUi.test.js @@ -0,0 +1,213 @@ +// @vitest-environment happy-dom +import { afterEach, expect, it, vi } from "vitest"; + +vi.mock("components/quickTools", () => { + const $footer = document.createElement("footer"); + $footer.innerHTML = `
`; + return { + default: { + $footer, + $row1: $footer.children[0], + $row2: $footer.children[1], + $input: document.createElement("input"), + $toggler: document.createElement("button"), + }, + }; +}); +vi.mock("cm/commandRegistry", () => ({ + executeCommand: vi.fn(), + getRegisteredCommands: () => [], +})); +vi.mock("settings/searchSettings", () => ({ default: vi.fn() })); +vi.mock("dialogs/confirm", () => ({ default: vi.fn() })); +// Happy DOM lacks the legacy initKeyboardEvent API used by the app's polyfill. +vi.mock("utils/keyboardEvent", () => ({ + default: (type, init) => new KeyboardEvent(type, init), +})); +vi.mock("lib/settings", () => ({ + default: { + value: Object.freeze({ + quickTools: 2, + quicktoolsItems: Object.freeze([5, 20, 3, 4]), + floatingButton: false, + quickToolsTriggerMode: "click", + }), + QUICKTOOLS_TRIGGER_MODE_CLICK: "click", + on: vi.fn(), + }, +})); +vi.mock("lib/editorFile", () => ({ syncQuickToolsVisibility: vi.fn() })); +vi.mock("components/quickTools/items", () => ({ description: {} })); +vi.mock("components/tooltip", () => ({ + hideTooltip: vi.fn(), + showTooltip: vi.fn(), +})); +vi.mock("lib/config", () => ({ default: {} })); +vi.mock("cm/editorReadOnly", () => ({ focusEditorIfEditable: vi.fn() })); +vi.mock("@codemirror/commands", async (importOriginal) => ({ + ...(await importOriginal()), + undoDepth: () => 1, + redoDepth: () => 0, +})); + +afterEach(() => { + vi.useRealTimers(); + vi.unstubAllGlobals(); + vi.resetModules(); + vi.clearAllMocks(); + document.body.replaceChildren(); +}); + +async function setup() { + vi.useFakeTimers(); + const listeners = new Map(); + const manager = { + activeFile: { type: "editor" }, + editor: { state: {} }, + on(events, listener) { + for (const event of [events].flat()) + listeners.set(event, [...(listeners.get(event) || []), listener]); + }, + }; + vi.stubGlobal("editorManager", manager); + vi.stubGlobal("root", document.body); + const { default: init } = await import("handlers/quickToolsInit"); + const { default: tools } = await import("components/quickTools"); + const { default: registry } = await import("lib/quickToolsAdapter"); + const { default: settings } = await import("lib/settings"); + const { syncQuickToolsVisibility } = await import("lib/editorFile"); + const { default: actions, key } = await import("handlers/quickTools"); + const { default: stack } = await import("lib/actionStack"); + const switchTab = (tab) => { + manager.activeFile = tab; + listeners.get("switch-file").forEach((fn) => fn()); + }; + init(); + vi.runOnlyPendingTimers(); + return { + tools, + registry, + settings, + syncQuickToolsVisibility, + switchTab, + actions, + key, + stack, + }; +} + +it("keeps stock items and preferences intact through adapter state and tab changes", async () => { + const { tools, registry, settings, syncQuickToolsVisibility, switchTab } = + await setup(); + const stock = () => + [...tools.$footer.querySelectorAll("[data-action]")].map( + (button) => button.outerHTML, + ); + const before = stock(), + preferences = JSON.stringify(settings.value); + let state = { enabled: true, busy: false }, + notify; + const word = { hideQuickTools: true }; + switchTab(word); + const dispose = registry.register(word, { + getState: () => state, + canHandle: (action) => action.command === "redo", + execute: vi.fn(), + subscribe: (fn) => { + notify = fn; + return () => {}; + }, + }); + expect(syncQuickToolsVisibility).toHaveBeenLastCalledWith(word); + expect(tools.$footer.querySelector('[data-id="undo"]').disabled).toBe(true); + expect(tools.$footer.querySelector('[data-id="redo"]').disabled).toBe(false); + state = { ...state, busy: true }; + notify(); + expect(tools.$footer.querySelector('[data-id="redo"]').disabled).toBe(true); + expect(stock()).toEqual(before); + switchTab({ type: "editor" }); + vi.runOnlyPendingTimers(); + expect(tools.$footer.querySelector('[data-id="undo"]').disabled).toBe(false); + expect(tools.$footer.querySelector('[data-id="redo"]').disabled).toBe(true); + // Loading another tab must not reuse the previous document's visibility cache. + const second = { hideQuickTools: true }; + switchTab(second); + const disposeSecond = registry.register(second, { + getState: () => ({ enabled: true }), + canHandle: () => false, + execute: vi.fn(), + subscribe: () => () => {}, + }); + expect(syncQuickToolsVisibility).toHaveBeenLastCalledWith(second); + dispose(); + expect(stock()).toEqual(before); + expect(JSON.stringify(settings.value)).toBe(preferences); + disposeSecond(); +}); + +it("routes modifier input and cancels capture on state, overlay, tab and disposal changes without touching the Back stack", async () => { + const { tools, registry, switchTab, actions, key, stack } = await setup(); + const entry = { id: "navigation", action: vi.fn() }; + stack.push(entry); + const mutations = ["push", "remove", "pop"].map((method) => + vi.spyOn(stack, method), + ); + const word = {}; + switchTab(word); + const execute = vi.fn(); + let busy = false, + notify; + const dispose = registry.register(word, { + getState: () => ({ enabled: true, busy }), + canHandle: () => true, + subscribe: (listener) => { + notify = listener; + return () => {}; + }, + execute, + }); + expect(actions("ctrl")).toBe(true); + tools.$input.dispatchEvent( + new InputEvent("beforeinput", { + inputType: "insertText", + data: "c", + cancelable: true, + }), + ); + await vi.advanceTimersByTimeAsync(0); + expect(execute).toHaveBeenCalledWith( + expect.objectContaining({ type: "key", key: "c", ctrlKey: true }), + expect.any(Object), + ); + expect(key.ctrl).toBe(false); + + const checkCancellation = (transition) => { + expect(actions("ctrl")).toBe(true); + tools.$input.value = "pending"; + transition(); + expect(key.ctrl).toBe(false); + expect(tools.$input.value).toBe(""); + }; + checkCancellation(() => { + busy = true; + notify(); + }); + busy = false; + notify(); + const overlay = document.createElement("div"); + overlay.className = "prompt"; + checkCancellation(() => { + document.body.append(overlay); + overlay.dispatchEvent(new PointerEvent("pointerdown", { bubbles: true })); + }); + overlay.remove(); + document.body.dispatchEvent( + new PointerEvent("pointerdown", { bubbles: true }), + ); + checkCancellation(() => switchTab({ type: "editor" })); + switchTab(word); + checkCancellation(dispose); + expect(stack.length).toBe(1); + expect(stack.get(entry.id)).toBe(entry); + mutations.forEach((method) => expect(method).not.toHaveBeenCalled()); +}); diff --git a/tests/unit/quickToolsOverlays.test.js b/tests/unit/quickToolsOverlays.test.js new file mode 100644 index 0000000000..99898b88f9 --- /dev/null +++ b/tests/unit/quickToolsOverlays.test.js @@ -0,0 +1,94 @@ +// @vitest-environment happy-dom +import { afterEach, expect, it, vi } from "vitest"; +import { createQuickToolsAdapterRegistry } from "../../src/lib/quickToolsAdapter"; +import { + hasQuickToolsOverlay, + watchQuickToolsOverlays, +} from "../../src/lib/quickToolsOverlays"; +let dispose; +afterEach(() => { + dispose?.(); + document.body.replaceChildren(); +}); +const settle = () => new Promise((resolve) => setTimeout(resolve, 0)); +it("cancels clipboard and repeats on native overlays without observing editor content or changing the Back stack", async () => { + let tab = {}, + finish; + const registry = createQuickToolsAdapterRegistry( + () => tab, + (action) => + new Promise((resolve) => { + finish = () => resolve(action); + }), + hasQuickToolsOverlay, + ); + const execute = vi.fn(), + cancel = vi.fn(); + const unregister = registry.register(tab, { + getState: () => ({ enabled: true }), + canHandle: () => true, + subscribe: () => () => {}, + execute, + }); + dispose = watchQuickToolsOverlays(registry, cancel); + registry.dispatch({ type: "text", text: "pending" }); + await settle(); + const prompt = document.createElement("div"); + prompt.className = "prompt"; + document.body.append(prompt); + await settle(); + expect(cancel).toHaveBeenCalledOnce(); + expect(registry.available({ type: "text" })).toBe(false); + prompt.remove(); + await settle(); + finish(); + await settle(); + expect(execute).not.toHaveBeenCalled(); + const content = document.createElement("div"); + document.body.append(content); + await settle(); + content.append(document.createElement("div")); + await settle(); + expect(cancel).toHaveBeenCalledOnce(); + for (const type of ["editor", "terminal"]) { + tab = { type }; + registry.sync(); + document.body.append(prompt); + await settle(); + expect(registry.dispatch({ type: "text" })).toBe(false); + prompt.remove(); + } + unregister(); + expect(cancel).toHaveBeenCalledOnce(); +}); +it("checks an overlay synchronously before dispatch and does not refocus into a busy adapter", async () => { + const tab = {}, + execute = vi.fn(), + focus = vi.fn(); + let busy = false; + const registry = createQuickToolsAdapterRegistry( + () => tab, + undefined, + hasQuickToolsOverlay, + ); + registry.register(tab, { + getState: () => ({ enabled: true, busy }), + canHandle: () => true, + subscribe: () => () => {}, + execute, + focus, + }); + const palette = document.createElement("div"); + palette.id = "palette"; + document.body.append(palette); + registry.dispatch({ type: "text", text: "x" }); + registry.focus(); + await settle(); + expect(execute).not.toHaveBeenCalled(); + expect(focus).not.toHaveBeenCalled(); + palette.remove(); + busy = true; + registry.focus(); + await settle(); + expect(focus).not.toHaveBeenCalled(); +}); diff --git a/tests/unit/saveCommands.test.js b/tests/unit/saveCommands.test.js new file mode 100644 index 0000000000..eb811fe24b --- /dev/null +++ b/tests/unit/saveCommands.test.js @@ -0,0 +1,68 @@ +import { expect, it, vi } from "vitest"; +import { loadSourceModule } from "../helpers/loadSourceModule"; + +function setup(files) { + const manager = { files, activeFile: files[0], getFile: id => files.find(file => file.id === id) }; + const toast = vi.fn(), error = vi.fn(); + const dependencies = Object.fromEntries([ + "fileSystem", "@codemirror/commands", "cm/editorReadOnly", "components/sidebar", "dialogs/prompt", "handlers/quickTools", "lib/recents", "utils/color/regex", "utils/Url", "./checkFiles", "./config", "./editorFile", "./lazyImports", "./openFile", "./openFolder", "./run", "./saveState", "./settings", "./showFileInfo", + ].map(id => [id, {}])); + const module = loadSourceModule("src/lib/commands.js", { + ...dependencies, "dialogs/confirm": async () => true, "dialogs/select": async () => "save", "utils/helpers": { error }, + }, { editorManager: manager, strings: {}, toast }); + return { ...module, manager, toast, error }; +} +function tab(id, type = "docs") { + const file = { id, type, canSave: type === "docs", isUnsaved: true, remove: vi.fn(async () => true) }; + file.save = vi.fn(async () => { file.isUnsaved = false; return type === "editor" ? [undefined, undefined] : true; }); + file.saveAs = vi.fn(async () => true); + file.hasUnsavedChanges = () => file.isUnsaved; + return file; +} +it("routes standard Save and Save As to the captured tab and leaves terminals ineligible", async () => { + const one = tab("one"), two = tab("two"), code = tab("code", "editor"), terminal = tab("terminal", "terminal"); + const f = setup([one, two, code, terminal]); + for (const file of [one, two, code]) { + f.manager.activeFile = file; + expect(f.canSaveFile(file)).toBe(true); + await f.default.save(); await f.default["save-as"](); + expect(file.save).toHaveBeenCalledOnce(); expect(file.saveAs).toHaveBeenCalledOnce(); + } + f.manager.activeFile = terminal; + await f.default.save(); await f.default["save-as"](); + expect(terminal.save).not.toHaveBeenCalled(); +}); +it("does not report custom-tab success on cancellation or failure", async () => { + const file = tab("one"), f = setup([file]); + file.save.mockResolvedValueOnce(false).mockRejectedValueOnce(Error("full")); + await f.default.save(true); await f.default.save(true); + expect(f.toast).not.toHaveBeenCalled(); expect(f.error).toHaveBeenCalledOnce(); + await f.default.save(true); expect(f.toast).toHaveBeenCalledOnce(); +}); +it("saves sequentially without clearing flags and stops at cancellation or newer edits", async () => { + const one = tab("one"), two = tab("two"), code = tab("code", "editor"); + const f = setup([one, two, code]); + let release; + one.save.mockImplementationOnce(() => new Promise(resolve => { release = () => { one.isUnsaved = false; resolve(true); }; })); + const saving = f.default["save-all-changes"](); + await vi.waitFor(() => expect(one.save).toHaveBeenCalledOnce()); + expect(one.isUnsaved).toBe(true); expect(two.save).not.toHaveBeenCalled(); + two.save.mockResolvedValueOnce(false); + release(); expect(await saving).toBe(false); + expect(two.isUnsaved).toBe(true); expect(code.save).not.toHaveBeenCalled(); + two.save.mockResolvedValueOnce(true); // A newer edit remains dirty after the write. + expect(await f.default["save-all-changes"]()).toBe(false); + expect(code.save).not.toHaveBeenCalled(); + expect(await f.default["save-all-changes"]()).toBe(true); + expect(code.isUnsaved).toBe(false); +}); +it.each([false, true, "failure"])("save-and-close keeps edits after an unsuccessful save (%s)", async outcome => { + const file = tab("one"), f = setup([file]); + if (outcome === "failure") file.save.mockRejectedValueOnce(Error("disk full")); + else file.save.mockResolvedValueOnce(outcome); + const result = f.default["close-all-tabs"](); + if (outcome === "failure") await expect(result).rejects.toThrow("disk full"); + else await result; + expect(file.remove).not.toHaveBeenCalled(); + expect(file.isUnsaved).toBe(true); +}); From 3334063ea1f92ba0bbc038d24a7cf1d7aee6cdc5 Mon Sep 17 00:00:00 2001 From: Ajit Kumar Date: Mon, 14 Sep 2026 08:32:08 +0530 Subject: [PATCH 2/6] fix: Capture Swallows Later Input --- src/handlers/quickTools.js | 31 ++- src/handlers/quickToolsInit.js | 68 ++++-- src/lib/editorFile.js | 2 +- src/lib/quickToolsAdapter.js | 10 +- src/pages/fileBrowser/fileBrowser.scss | 24 +-- tests/unit/quickToolsAdapter.test.js | 18 ++ tests/unit/quickToolsAdapterUi.test.js | 277 +++++++++++++++++++++++-- 7 files changed, 376 insertions(+), 54 deletions(-) diff --git a/src/handlers/quickTools.js b/src/handlers/quickTools.js index 1db59d956c..eee1ddfe81 100644 --- a/src/handlers/quickTools.js +++ b/src/handlers/quickTools.js @@ -69,6 +69,20 @@ function adapterAction(action, value) { return null; } +function finishAdapterCapture() { + const restoreFocus = + (adapterCapture && !adapterCapture.consumed) || + document.activeElement === quickTools.$input; + // Blurring may emit composition events synchronously. Keep an inert guard + // for those events without leaving the hidden input as the typing target. + if (adapterCapture) adapterCapture.consumed = true; + clearQuickToolsModifierState(); + quickToolsAdapters.discardCapture(); + quickTools.$input.value = ""; + quickTools.$input.blur(); + if (restoreFocus) quickToolsAdapters.focus(); +} + function handleAdapterCapture(event) { if (!adapterCapture) return false; if ( @@ -111,12 +125,14 @@ function handleAdapterCapture(event) { ...modifiers, }); } else { - if (result.outcome.kind === "duplicate") event.preventDefault(); + if (result.outcome.kind === "duplicate") { + quickTools.$input.value = ""; + event.preventDefault(); + } return true; } - if (!event.isComposing) quickTools.$input.value = ""; event.preventDefault(); - clearQuickToolsModifierState(); + finishAdapterCapture(); return true; } @@ -343,6 +359,7 @@ export function clearQuickToolsModifierState({ restoreFocus = false } = {}) { export function cancelQuickToolsModifierInput() { adapterCapture = null; + quickToolsAdapters.discardCapture(); clearReadOnlyCaptureSession(); const changed = clearQuickToolsModifierState(); quickTools.$input.value = ""; @@ -368,7 +385,11 @@ export default function actions(action, value) { if (routed) { if (routed.type === "command" && routed.command === "openCommandPalette") return executeCommand("openCommandPalette", editor); - if (!quickToolsAdapters.available(routed)) return false; + if (!quickToolsAdapters.available(routed)) { + if (!Object.values(state).some(Boolean)) + quickToolsAdapters.discardCapture(); + return false; + } if (routed.type === "modifier") { quickToolsAdapters.capture(); state[action] = !state[action]; @@ -389,7 +410,7 @@ export default function actions(action, value) { } const handled = quickToolsAdapters.dispatch(routed); if (routed.type !== "key" || !routed.key.startsWith("Arrow")) - clearQuickToolsModifierState(); + finishAdapterCapture(); return handled; } diff --git a/src/handlers/quickToolsInit.js b/src/handlers/quickToolsInit.js index 2410cd08a2..ac4f07fb4c 100644 --- a/src/handlers/quickToolsInit.js +++ b/src/handlers/quickToolsInit.js @@ -4,11 +4,11 @@ import quickTools from "components/quickTools"; import { description } from "components/quickTools/items"; import { hideTooltip, showTooltip } from "components/tooltip"; import config from "lib/config"; -import appSettings from "lib/settings"; -import actions, { cancelQuickToolsModifierInput, key } from "./quickTools"; +import { syncQuickToolsVisibility } from "lib/editorFile"; import quickToolsAdapters from "lib/quickToolsAdapter"; import { watchQuickToolsOverlays } from "lib/quickToolsOverlays"; -import { syncQuickToolsVisibility } from "lib/editorFile"; +import appSettings from "lib/settings"; +import actions, { cancelQuickToolsModifierInput, key } from "./quickTools"; const CONTEXT_MENU_TIMEOUT = 500; const MOVE_X_THRESHOLD = 50; @@ -53,6 +53,12 @@ function clearTouchFeedback() { } } +function discardAdapterCapture() { + // Modifier sequences keep their selection until a key is dispatched. + if (!key.shift && !key.ctrl && !key.alt && !key.meta) + quickToolsAdapters.discardCapture(); +} + /** * Initialize quick tools * @param {HTMLElement} $footer @@ -88,12 +94,24 @@ export default function init() { refreshAdapter(); }); watchQuickToolsOverlays(quickToolsAdapters, cancelAdapterInput); - $footer.addEventListener( - "pointerdown", - () => quickToolsAdapters.capture(), - true, - ); - $footer.addEventListener("keydown", () => quickToolsAdapters.capture(), true); + const capture = () => { + discardAdapterCapture(); + quickToolsAdapters.capture(); + }; + $footer.addEventListener("pointerdown", capture, true); + $footer.addEventListener("keydown", capture, true); + $footer.addEventListener("pointercancel", discardAdapterCapture, true); + $footer.addEventListener("scroll", discardAdapterCapture, true); + const leaveQuickTools = (event) => { + if (!quickToolsAdapters.has()) return; + const path = event.composedPath(); + if (path.includes($footer) || path.includes($input)) return; + quickToolsAdapters.discardCapture(); + if (key.shift || key.ctrl || key.alt || key.meta) + cancelQuickToolsModifierInput(); + }; + document.addEventListener("pointerdown", leaveQuickTools, true); + document.addEventListener("focusin", leaveQuickTools, true); $toggler.addEventListener("click", (e) => { e.preventDefault(); @@ -189,6 +207,7 @@ export default function init() { function onwheel(e) { e.preventDefault(); + discardAdapterCapture(); const $el = e.target; const { $row1, $row2 } = quickTools; let $row; @@ -208,6 +227,7 @@ function onclick(e) { reset(); if (e.target.disabled) { + discardAdapterCapture(); e.preventDefault(); e.stopPropagation(); return; @@ -344,11 +364,10 @@ function touchend(e) { } $row.scrollLeft = scroll; - touchcancel(e); - - if ($el === $touchstart && performance.now() - startTime < 100) { - click($el); - } + const shouldClick = + $el === $touchstart && performance.now() - startTime < 100; + touchcancel(e, { preserveCapture: shouldClick }); + if (shouldClick) click($el); return; } @@ -357,7 +376,7 @@ function touchend(e) { return; } - touchcancel(e); + touchcancel(e, { preserveCapture: true }); click($el); } @@ -365,7 +384,7 @@ function touchend(e) { * * @param {TouchEvent} e */ -function touchcancel(e) { +function touchcancel(e, { preserveCapture = false } = {}) { document.removeEventListener("keyup", touchcancel); document.removeEventListener("touchend", touchend); document.removeEventListener("touchcancel", touchcancel); @@ -374,6 +393,7 @@ function touchcancel(e) { clearTimeout(contextmenuTimeout); clearTouchFeedback(); hideTooltip(); + if (!preserveCapture) discardAdapterCapture(); } /** @@ -414,7 +434,10 @@ function oncontextmenu(e) { * @param {HTMLElement} $el */ function click($el) { - if ($el.disabled) return; + if ($el.disabled) { + discardAdapterCapture(); + return; + } $el.classList.add("click"); clearTimeout($el.dataset.timeout); @@ -427,7 +450,10 @@ function click($el) { } const { action } = $el.dataset; - if (!action) return; + if (!action) { + discardAdapterCapture(); + return; + } let { value } = $el.dataset; @@ -435,7 +461,11 @@ function click($el) { value = $el.value; } - actions(action, value); + try { + actions(action, value); + } finally { + discardAdapterCapture(); + } } function scheduleUpdateQuickToolsState() { diff --git a/src/lib/editorFile.js b/src/lib/editorFile.js index 6a06481349..12878dbc44 100644 --- a/src/lib/editorFile.js +++ b/src/lib/editorFile.js @@ -15,9 +15,9 @@ import confirm from "dialogs/confirm"; import DOMPurify from "dompurify"; import startDrag from "handlers/editorFileTab"; import actions from "handlers/quickTools"; -import quickToolsAdapters from "lib/quickToolsAdapter"; import { openTabContextMenuOnRelease } from "handlers/tabContextMenu"; import tag from "html-tag-js"; +import quickToolsAdapters from "lib/quickToolsAdapter"; import mimeTypes from "mime-types"; import { applyHighlightStyles } from "utils/codeHighlight"; import helpers from "utils/helpers"; diff --git a/src/lib/quickToolsAdapter.js b/src/lib/quickToolsAdapter.js index 15415988e2..b1067f893c 100644 --- a/src/lib/quickToolsAdapter.js +++ b/src/lib/quickToolsAdapter.js @@ -120,10 +120,18 @@ export function createQuickToolsAdapterRegistry( // Capture failures are reported by dispatch, without an unhandled rejection. entry.selection.catch(() => {}); }, + discardCapture() { + // Dispatched actions already own their snapshots. Leave their queue intact. + const entry = current(); + if (entry) entry.selection = undefined; + }, dispatch(action) { const entry = current(); if (!entry) return false; - if (!this.available(action)) return true; + if (!this.available(action)) { + this.discardCapture(); + return true; + } const signal = entry.controller.signal, selection = entry.selection; entry.selection = undefined; diff --git a/src/pages/fileBrowser/fileBrowser.scss b/src/pages/fileBrowser/fileBrowser.scss index 8d55d69766..0a262aaefb 100644 --- a/src/pages/fileBrowser/fileBrowser.scss +++ b/src/pages/fileBrowser/fileBrowser.scss @@ -3,20 +3,20 @@ display: none; } - .tile { - > .icon:first-child { - --file-icon-size: 24px; - font-size: var(--file-icon-size); - background-size: var(--file-icon-size); - background-position: center; - - > svg, > img { - width: var(--file-icon-size); - height: var(--file-icon-size); - object-fit: contain; - } + #list .tile > .icon:first-child { + --file-icon-size: 24px; + font-size: var(--file-icon-size); + background-size: var(--file-icon-size); + background-position: center; + + > svg, > img { + width: var(--file-icon-size); + height: var(--file-icon-size); + object-fit: contain; } + } + .tile { &[data-storage-type="notification"] { background-color: rgb(153, 153, 255); background-color: var(--primary-color); diff --git a/tests/unit/quickToolsAdapter.test.js b/tests/unit/quickToolsAdapter.test.js index 3da3ed38fb..48bf27ab4b 100644 --- a/tests/unit/quickToolsAdapter.test.js +++ b/tests/unit/quickToolsAdapter.test.js @@ -103,5 +103,23 @@ describe("custom-tab quicktools", () => { f.registry.focus(); f.registry.cancel(); await settle(); expect(f.adapter.focus).not.toHaveBeenCalled(); }); + it("discards unused captures without cancelling an already queued edit", async () => { + const f = setup(); let finish, caret = 3; + f.adapter.captureSelection.mockImplementation(() => caret); + f.adapter.restoreSelection.mockImplementation(value => { caret = value; }); + f.adapter.execute.mockImplementationOnce(() => new Promise(resolve => { finish = resolve; })); + f.registry.capture(); f.registry.dispatch(insert); + await settle(); + const signal = f.adapter.execute.mock.calls[0][1].signal; + f.registry.capture(); f.registry.discardCapture(); + finish(); await settle(); + caret = 20; + f.registry.capture(); f.registry.dispatch(insert); + await settle(); + expect(f.adapter.restoreSelection.mock.calls).toEqual([[3], [20]]); + expect(f.adapter.execute).toHaveBeenCalledTimes(2); + expect(signal.aborted).toBe(false); + expect(f.adapter.cancel).not.toHaveBeenCalled(); + }); }); diff --git a/tests/unit/quickToolsAdapterUi.test.js b/tests/unit/quickToolsAdapterUi.test.js index 749ce5bbc2..60ec2af82d 100644 --- a/tests/unit/quickToolsAdapterUi.test.js +++ b/tests/unit/quickToolsAdapterUi.test.js @@ -1,19 +1,21 @@ // @vitest-environment happy-dom import { afterEach, expect, it, vi } from "vitest"; -vi.mock("components/quickTools", () => { - const $footer = document.createElement("footer"); - $footer.innerHTML = `
`; - return { - default: { - $footer, - $row1: $footer.children[0], - $row2: $footer.children[1], - $input: document.createElement("input"), - $toggler: document.createElement("button"), - }, - }; -}); +function mockQuickTools() { + vi.doMock("components/quickTools", () => { + const $footer = document.createElement("footer"); + $footer.innerHTML = `
`; + return { + default: { + $footer, + $row1: $footer.children[0], + $row2: $footer.children[1], + $input: document.createElement("input"), + $toggler: document.createElement("button"), + }, + }; + }); +} vi.mock("cm/commandRegistry", () => ({ executeCommand: vi.fn(), getRegisteredCommands: () => [], @@ -22,7 +24,11 @@ vi.mock("settings/searchSettings", () => ({ default: vi.fn() })); vi.mock("dialogs/confirm", () => ({ default: vi.fn() })); // Happy DOM lacks the legacy initKeyboardEvent API used by the app's polyfill. vi.mock("utils/keyboardEvent", () => ({ - default: (type, init) => new KeyboardEvent(type, init), + default: (type, init) => + new KeyboardEvent(type, { + ...init, + key: init.key || { 37: "ArrowLeft", 39: "ArrowRight" }[init.keyCode], + }), })); vi.mock("lib/settings", () => ({ default: { @@ -50,7 +56,13 @@ vi.mock("@codemirror/commands", async (importOriginal) => ({ redoDepth: () => 0, })); -afterEach(() => { +const cleanups = []; +afterEach(async () => { + cleanups.splice(0).forEach((dispose) => dispose()); + document.body.replaceChildren(); + await vi.advanceTimersByTimeAsync(0); + vi.clearAllTimers(); + vi.restoreAllMocks(); vi.useRealTimers(); vi.unstubAllGlobals(); vi.resetModules(); @@ -58,8 +70,9 @@ afterEach(() => { document.body.replaceChildren(); }); -async function setup() { +async function setup(triggerMode = "click") { vi.useFakeTimers(); + mockQuickTools(); const listeners = new Map(); const manager = { activeFile: { type: "editor" }, @@ -78,6 +91,7 @@ async function setup() { const { syncQuickToolsVisibility } = await import("lib/editorFile"); const { default: actions, key } = await import("handlers/quickTools"); const { default: stack } = await import("lib/actionStack"); + settings.value = { ...settings.value, quickToolsTriggerMode: triggerMode }; const switchTab = (tab) => { manager.activeFile = tab; listeners.get("switch-file").forEach((fn) => fn()); @@ -145,6 +159,237 @@ it("keeps stock items and preferences intact through adapter state and tab chang disposeSecond(); }); +async function adaptedSetup(triggerMode) { + const f = await setup(triggerMode); + const tab = { type: "docs" }; + f.switchTab(tab); + const editor = document.createElement("textarea"); + editor.value = "0123456789"; + document.body.append(editor); + editor.focus(); + editor.setSelectionRange(3, 3); + let state = { enabled: true }, + notify; + const adapter = { + getState: () => state, + canHandle: (action) => action.command !== "unsupported", + subscribe: (listener) => { + notify = listener; + return () => {}; + }, + captureSelection: vi.fn(() => editor.selectionStart), + restoreSelection: vi.fn((position) => + editor.setSelectionRange(position, position), + ), + execute: vi.fn((action) => { + if (action.type === "text") editor.setRangeText(action.text); + }), + focus: vi.fn(() => editor.focus()), + cancel: vi.fn(), + }; + const dispose = f.registry.register(tab, adapter); + cleanups.push(dispose); + const button = f.tools.$footer.querySelector('[data-id="move"]'); + button.dataset.action = "insert"; + button.dataset.value = "x"; + const pointer = (type, target = button) => + target.dispatchEvent(new PointerEvent(type, { bubbles: true })); + const click = () => + button.dispatchEvent( + new MouseEvent("click", { bubbles: true, cancelable: true }), + ); + return { + ...f, + editor, + adapter, + button, + pointer, + click, + dispose, + setState(value) { + state = { ...state, ...value }; + notify(); + }, + }; +} + +it.each([ + "keydown", + "beforeinput", + "input", + "compositionend", + "composition", +])("finishes %s shortcut capture, absorbs trailing events, and resumes normal typing", async (type) => { + const f = await adaptedSetup(); + f.actions("ctrl"); + const input = f.tools.$input; + if (type === "composition") { + input.dispatchEvent( + new KeyboardEvent("keydown", { key: "Process", isComposing: true }), + ); + } + input.value = "c"; + const event = + type === "keydown" + ? new KeyboardEvent(type, { key: "c", cancelable: true }) + : new InputEvent(type === "composition" ? "beforeinput" : type, { + data: "c", + inputType: + type === "composition" ? "insertCompositionText" : "insertText", + isComposing: type === "composition", + cancelable: true, + }); + input.dispatchEvent(event); + await vi.advanceTimersByTimeAsync(0); + expect(f.adapter.execute).toHaveBeenCalledExactlyOnceWith( + expect.objectContaining({ type: "key", key: "c", ctrlKey: true }), + expect.any(Object), + ); + expect(f.key.ctrl).toBe(false); + expect(document.activeElement).toBe(f.editor); + for (const type of ["beforeinput", "input", "compositionend"]) { + input.value = "c"; + input.dispatchEvent(new InputEvent(type, { data: "c", cancelable: true })); + } + for (const text of ["x", "y"]) { + const target = document.activeElement; + if ( + target.dispatchEvent( + new InputEvent("beforeinput", { + data: text, + inputType: "insertText", + cancelable: true, + }), + ) + ) { + const caret = target.selectionStart + text.length; + target.setRangeText(text); + target.setSelectionRange(caret, caret); + } + } + await vi.advanceTimersByTimeAsync(0); + expect(f.editor.value).toBe("012xy3456789"); + expect(input.value).toBe(""); + expect(f.adapter.execute).toHaveBeenCalledOnce(); + expect(f.adapter.cancel).not.toHaveBeenCalled(); +}); + +it("completes special keys and toolbar input while preserving modifier arrow repeats", async () => { + const f = await adaptedSetup(); + for (const key of ["ArrowLeft", "Backspace"]) { + f.actions("ctrl"); + f.tools.$input.dispatchEvent( + new KeyboardEvent("keydown", { key, cancelable: true }), + ); + await vi.advanceTimersByTimeAsync(0); + expect(f.key.ctrl).toBe(false); + expect(document.activeElement).toBe(f.editor); + } + f.actions("ctrl"); + f.actions("shift"); + f.actions("key", 39); + f.actions("key", 39); + await vi.advanceTimersByTimeAsync(0); + expect( + f.adapter.execute.mock.calls.slice(-2).map(([action]) => action), + ).toEqual([ + expect.objectContaining({ + key: "ArrowRight", + ctrlKey: true, + shiftKey: true, + }), + expect.objectContaining({ + key: "ArrowRight", + ctrlKey: true, + shiftKey: true, + }), + ]); + expect(f.key.ctrl && f.key.shift).toBe(true); + f.actions("insert", "x"); + await vi.advanceTimersByTimeAsync(0); + expect(f.key.ctrl || f.key.shift).toBe(false); + expect(document.activeElement).toBe(f.editor); + expect(f.adapter.cancel).not.toHaveBeenCalled(); +}); + +it.each([ + "no focus callback", + "tab switch", + "overlay", + "busy", + "disabled", + "disposal", +])("releases shortcut input without stale focus after %s", async (transition) => { + const f = await adaptedSetup(); + f.actions("ctrl"); + f.tools.$input.dispatchEvent( + new InputEvent("beforeinput", { data: "c", cancelable: true }), + ); + if (transition === "no focus callback") delete f.adapter.focus; + if (transition === "tab switch") f.switchTab({ type: "editor" }); + if (transition === "overlay") { + const overlay = document.createElement("div"); + overlay.className = "prompt"; + document.body.append(overlay); + } + if (transition === "busy") f.setState({ busy: true }); + if (transition === "disabled") f.setState({ enabled: false }); + if (transition === "disposal") f.dispose(); + await vi.advanceTimersByTimeAsync(0); + expect(document.activeElement).not.toBe(f.tools.$input); + if (f.adapter.focus) expect(f.adapter.focus).not.toHaveBeenCalled(); +}); + +it.each([ + "pointercancel", + "scroll", + "outside pointer", + "outside focus", + "unsupported", + "unused gesture", +])("drops selection after %s without disturbing queued edits", async (transition) => { + const f = await adaptedSetup(); + f.pointer("pointerdown"); + if (transition === "pointercancel") f.pointer("pointercancel"); + if (transition === "scroll") f.tools.$row1.dispatchEvent(new Event("scroll")); + if (transition === "outside pointer") f.pointer("pointerdown", f.editor); + if (transition === "outside focus") { + f.button.focus(); + f.editor.focus(); + } + if (transition === "unsupported") f.actions("command", "unsupported"); + f.editor.setSelectionRange(8, 8); + f.pointer("pointerdown"); + f.click(); + await vi.advanceTimersByTimeAsync(0); + expect(f.editor.value).toBe("01234567x89"); + expect(f.adapter.cancel).not.toHaveBeenCalled(); +}); + +it("retains touch selection until dispatch and drops a cancelled touch", async () => { + const f = await adaptedSetup("touch"); + vi.spyOn(document, "elementFromPoint").mockReturnValue(f.button); + const touch = (type) => + f.button.dispatchEvent( + new TouchEvent(type, { + bubbles: true, + cancelable: true, + changedTouches: [{ clientX: 0, clientY: 0 }], + }), + ); + f.pointer("pointerdown"); + touch("touchstart"); + touch("touchcancel"); + f.editor.setSelectionRange(8, 8); + f.pointer("pointerdown"); + touch("touchstart"); + // Simulate selection being lost when the toolbar takes focus. + f.editor.setSelectionRange(0, 0); + touch("touchend"); + await vi.advanceTimersByTimeAsync(0); + expect(f.editor.value).toBe("01234567x89"); +}); + it("routes modifier input and cancels capture on state, overlay, tab and disposal changes without touching the Back stack", async () => { const { tools, registry, switchTab, actions, key, stack } = await setup(); const entry = { id: "navigation", action: vi.fn() }; From c3abc3746e2ac4655cda262beef32d438f047e1d Mon Sep 17 00:00:00 2001 From: Ajit Kumar Date: Mon, 14 Sep 2026 18:02:43 +0530 Subject: [PATCH 3/6] fix: issues --- CONTRIBUTING.md | 2 - docs/file-save-events.md | 29 ---- docs/quicktools-adapter.md | 30 ----- src/handlers/quickTools.js | 27 +++- src/handlers/quickToolsInit.js | 16 ++- src/lib/commands.js | 24 ++-- src/lib/quickToolsAdapter.js | 20 ++- tests/unit/quickToolsAdapter.test.js | 23 ++++ tests/unit/quickToolsAdapterUi.test.js | 180 ++++++++++++++++++++++++- tests/unit/saveCommands.test.js | 43 ++++++ 10 files changed, 302 insertions(+), 92 deletions(-) delete mode 100644 docs/file-save-events.md delete mode 100644 docs/quicktools-adapter.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index aeb58e1073..2457415d2e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -208,5 +208,3 @@ Many font editing software and web-based tools exist for this purpose. Some of t To create plugins for Acode: - [Plugin Starter Repository](https://github.com/Acode-Foundation/acode-plugin) - [Plugin Documentation](https://docs.acode.app/) -- [Custom editor quicktools API](docs/quicktools-adapter.md) -- [Custom-tab Save and Save As](docs/file-save-events.md) diff --git a/docs/file-save-events.md b/docs/file-save-events.md deleted file mode 100644 index 3244e6b226..0000000000 --- a/docs/file-save-events.md +++ /dev/null @@ -1,29 +0,0 @@ -# Custom-tab Save and Save As - -Custom tabs use Acode's standard Save controls through the `EditorFile` save event. The plugin owns serialization, destination dialogs, writing and dirty tracking. - -```js -if (typeof tab.canSave === "boolean") { - const onSave = (event) => { - event.respondWith(saveDocument(event.saveAs)); - }; - tab.on("save", onSave); - // On plugin disposal: - // tab.off("save", onSave); -} -``` - -`tab.canSave` is read-only: open text-editor tabs qualify automatically; custom tabs need a `save` listener or `onsave` handler. Removing the handler or closing the custom tab disables saving. The handler must implement saving and guard loading, protection and processing. - -The event retains `target`, `preventDefault()` and `stopPropagation()`. It additionally exposes: - -- `saveAs`: `false` for `tab.save()`, `true` for `tab.saveAs()`. -- `respondWith(Promise)`: call synchronously, once, during dispatch to suppress default saving. Resolve `true` after a successful write, `false` for cancellation or no save, and reject for failure. Calling after an `await` or responding twice throws. - -Other observers can still see the event. Existing `preventDefault()` cancellation remains supported. Unhandled text saves use the normal text writer; unhandled custom-tab saves return `false`. - -Concurrent requests on one custom tab share its pending promise; the first request determines Save versus Save As. Different tabs are independent. Invoke the plugin's writer directly to avoid recursive save dispatch. Remove the listener on unload and cancel pending work on tab close; closed tabs cannot publish completion notifications. - -Before resolving success, update the destination and acknowledge only the written revision; newer edits must remain dirty. Acode then emits `save-file` and update notifications. Save All runs sequentially; save-and-close stops after cancellation, failure or remaining edits. Text encoding, format-on-save and autosave retain their existing behavior. - -Feature-detect `tab.canSave`. Older hosts require the plugin's own fallback and commands; their built-in Save menu does not support custom tabs. diff --git a/docs/quicktools-adapter.md b/docs/quicktools-adapter.md deleted file mode 100644 index 3edb45b559..0000000000 --- a/docs/quicktools-adapter.md +++ /dev/null @@ -1,30 +0,0 @@ -# Custom editor quicktools API - -`acode.registerQuickToolsAdapter(tab, adapter)` connects stock quicktools to one custom `EditorFile`. Register when the editor is ready. Feature-detect the API and use `hideQuickTools: true` while loading, unsupported or on older hosts. Registration allows one adapter per tab and returns an idempotent cleanup function; call it on plugin unload. Tab closure also unregisters the adapter. - -```js -const unregister = acode.registerQuickToolsAdapter?.(tab, { - getState: () => ({ enabled: editor.writable, busy: editor.saving }), - canHandle: action => editor.supports(action), - subscribe: listener => editor.subscribe(listener), // returns unsubscribe - captureSelection: () => editor.captureSelection(), - restoreSelection: selection => editor.restoreSelection(selection), - execute: (action, { signal }) => editor.execute(action, signal), - focus: () => editor.focus(), - cancel: () => editor.cancelPendingInput(), // optional - onError: error => reportError(error), // optional -}); -``` - -The example `editor` is plugin-owned. `getState`, `canHandle`, `subscribe` and `execute` are required; the other callbacks are optional. State and support queries must be synchronous, cheap and side-effect free. Execution, selection and focus callbacks may return promises. Notify subscribers when writability, processing, history or supported operations change; report plugin-owned dialogs as busy. - -- `enabled` controls visibility while respecting the user's quicktools preferences. `busy` blocks editing without changing the layout; the command palette stays available. -- `canHandle(action)` guards execution and Undo/Redo availability. Return `false` for unsupported operations; they never fall through to another editor. -- `execute` receives `{ type: 'text', text }`, `{ type: 'key', key, ctrlKey, shiftKey, altKey, metaKey }`, or `{ type: 'command', command }`. Keys use `KeyboardEvent.key` names, but editors must implement browser-default movement and deletion through their own APIs. -- Command names include `undo`, `redo`, `saveFile`, `find`, `selectall`, `paste`, `movelinesup`, `movelinesdown`, `copylinesup` and `copylinesdown`. -- Modifier button availability is queried as `{ type: 'modifier', key: 'ctrl' | 'shift' | 'alt' | 'meta' }`. Acode combines captured keyboard input with modifiers; adapters do not execute modifier toggles. -- For supported paste operations, Acode reads native clipboard text and delivers a text action. - -Acode captures selection before quicktools take focus and serializes input. Restore snapshots only for the current document revision. Restore focus only for explicit editing input; registration, selection restoration and dialogs must not force the keyboard open. - -Tab changes, native overlays, disabled/busy state and disposal abort the execution signal and cancel modifiers, repeats and queued input. Check the signal and your session/revision after asynchronous work. `cancel` must not synchronously emit another state update. Leave Back handling and the action stack to Acode; keep stock quicktools items, gestures and preferences intact. diff --git a/src/handlers/quickTools.js b/src/handlers/quickTools.js index eee1ddfe81..724ccbefa8 100644 --- a/src/handlers/quickTools.js +++ b/src/handlers/quickTools.js @@ -69,10 +69,11 @@ function adapterAction(action, value) { return null; } -function finishAdapterCapture() { +function finishAdapterCapture({ focus = true } = {}) { const restoreFocus = - (adapterCapture && !adapterCapture.consumed) || - document.activeElement === quickTools.$input; + focus && + ((adapterCapture && !adapterCapture.consumed) || + document.activeElement === quickTools.$input); // Blurring may emit composition events synchronously. Keep an inert guard // for those events without leaving the hidden input as the typing target. if (adapterCapture) adapterCapture.consumed = true; @@ -357,9 +358,11 @@ export function clearQuickToolsModifierState({ restoreFocus = false } = {}) { return changed; } -export function cancelQuickToolsModifierInput() { +export function cancelQuickToolsModifierInput({ + preserveCapture = false, +} = {}) { adapterCapture = null; - quickToolsAdapters.discardCapture(); + if (!preserveCapture) quickToolsAdapters.discardCapture(); clearReadOnlyCaptureSession(); const changed = clearQuickToolsModifierState(); quickTools.$input.value = ""; @@ -383,8 +386,18 @@ export default function actions(action, value) { const { $input, $replaceInput } = quickTools; const routed = quickToolsAdapters.has() && adapterAction(action, value); if (routed) { - if (routed.type === "command" && routed.command === "openCommandPalette") - return executeCommand("openCommandPalette", editor); + if ( + routed.type === "command" && + [ + "saveFile", + "saveFileAs", + "saveAllChanges", + "openCommandPalette", + ].includes(routed.command) + ) { + finishAdapterCapture({ focus: false }); + return executeCommand(routed.command, editor); + } if (!quickToolsAdapters.available(routed)) { if (!Object.values(state).some(Boolean)) quickToolsAdapters.discardCapture(); diff --git a/src/handlers/quickToolsInit.js b/src/handlers/quickToolsInit.js index ac4f07fb4c..1df5f75512 100644 --- a/src/handlers/quickToolsInit.js +++ b/src/handlers/quickToolsInit.js @@ -81,19 +81,18 @@ export default function init() { } updateHistoryButtons(); }; - const cancelAdapterInput = () => { + const clearAdapterInput = ({ preserveCapture = false } = {}) => { clearTimeout(timeout); - touchcancel(); + touchcancel(undefined, { preserveCapture }); reset(); - cancelQuickToolsModifierInput(); - quickToolsAdapters.cancel(); + cancelQuickToolsModifierInput({ preserveCapture }); }; quickToolsAdapters.subscribe((change) => { if (change?.cancelled && change.tab === editorManager.activeFile) - cancelAdapterInput(); + clearAdapterInput(); refreshAdapter(); }); - watchQuickToolsOverlays(quickToolsAdapters, cancelAdapterInput); + watchQuickToolsOverlays(quickToolsAdapters, clearAdapterInput); const capture = () => { discardAdapterCapture(); quickToolsAdapters.capture(); @@ -159,7 +158,10 @@ export default function init() { editorManager.on("editor-state-changed", updateHistoryButtons); editorManager.on("switch-file", () => { - if (adapterWasActive || quickToolsAdapters.has()) cancelAdapterInput(); + // activeFile already points to the incoming tab. Only sync cancels the + // outgoing adapter; UI cleanup must preserve the incoming selection. + if (adapterWasActive || quickToolsAdapters.has()) + clearAdapterInput({ preserveCapture: true }); else cancelQuickToolsModifierInput(); quickToolsAdapters.sync(); }); diff --git a/src/lib/commands.js b/src/lib/commands.js index ff35a4df65..918ffef25a 100644 --- a/src/lib/commands.js +++ b/src/lib/commands.js @@ -138,9 +138,13 @@ async function closeTabs(files, options = {}) { } } + let complete = true; for (const file of [...closableFiles]) { if (save && file.isUnsaved) { - if (!canSaveFile(file)) return false; + if (!canSaveFile(file)) { + complete = false; + continue; + } const saved = await file.save(); if (saved === false || file.hasUnsavedChanges?.() || file.isUnsaved) return false; @@ -149,7 +153,7 @@ async function closeTabs(files, options = {}) { await file.remove(true, { silentPinned: true }); } - return true; + return complete; } export default { @@ -160,7 +164,7 @@ export default { await runAllTests(); }, async "close-all-tabs"() { - await closeTabs(editorManager.files); + return closeTabs(editorManager.files); }, /** * Close every tab shown in the same tab group (pane tab bar) as the @@ -186,19 +190,19 @@ export default { return closeTabs(files); }, async "close-tabs-to-left"(referenceFile) { - await closeTabs( + return closeTabs( getTabsRelativeToFile("left", referenceFile), getTabCloseSelectionOptions(), ); }, async "close-tabs-to-right"(referenceFile) { - await closeTabs( + return closeTabs( getTabsRelativeToFile("right", referenceFile), getTabCloseSelectionOptions(), ); }, async "close-other-tabs"(referenceFile) { - await closeTabs( + return closeTabs( getTabsRelativeToFile("others", referenceFile), getTabCloseSelectionOptions(), ); @@ -209,14 +213,18 @@ export default { strings["save all changes warning"], ); if (!doSave) return; + let complete = true; for (const file of [...editorManager.files]) { if (!file.isUnsaved) continue; - if (!canSaveFile(file)) return false; + if (!canSaveFile(file)) { + complete = false; + continue; + } const saved = await file.save(); if (saved === false || file.hasUnsavedChanges?.() || file.isUnsaved) return false; } - return true; + return complete; }, "close-current-tab"() { editorManager.activeFile?.remove(); diff --git a/src/lib/quickToolsAdapter.js b/src/lib/quickToolsAdapter.js index b1067f893c..03c6bf7b08 100644 --- a/src/lib/quickToolsAdapter.js +++ b/src/lib/quickToolsAdapter.js @@ -17,13 +17,18 @@ export function createQuickToolsAdapterRegistry( const notify = (change) => listeners.forEach((listener) => listener(change)); const current = () => entries.get(getActiveTab()); function cancel(entry) { - if (!entry) return; - entry.controller.abort(); - entry.controller = new AbortController(); - entry.queue = Promise.resolve(); - entry.pending = 0; - entry.selection = undefined; - entry.adapter.cancel?.(); + if (!entry || entry.cancelling) return; + entry.cancelling = true; + try { + entry.controller.abort(); + entry.controller = new AbortController(); + entry.queue = Promise.resolve(); + entry.pending = 0; + entry.selection = undefined; + entry.adapter.cancel?.(); + } finally { + entry.cancelling = false; + } } const valid = (entry, signal) => !signal.aborted && @@ -57,6 +62,7 @@ export function createQuickToolsAdapterRegistry( queue: Promise.resolve(), pending: 0, selection: undefined, + cancelling: false, }; entries.set(tab, entry); const unsubscribe = adapter.subscribe(() => { diff --git a/tests/unit/quickToolsAdapter.test.js b/tests/unit/quickToolsAdapter.test.js index 48bf27ab4b..a8822177fc 100644 --- a/tests/unit/quickToolsAdapter.test.js +++ b/tests/unit/quickToolsAdapter.test.js @@ -121,5 +121,28 @@ describe("custom-tab quicktools", () => { expect(signal.aborted).toBe(false); expect(f.adapter.cancel).not.toHaveBeenCalled(); }); + it.each([{ busy: true }, { enabled: false }])("guards cancellation that publishes state %o and permits later cancellation", async (state) => { + const f = setup(); + f.registry.dispatch(insert); + await settle(); + const signal = f.adapter.execute.mock.calls[0][1].signal; + f.adapter.cancel.mockImplementation(() => f.state(state)); + expect(() => f.registry.cancel()).not.toThrow(); + expect(signal.aborted).toBe(true); + expect(f.adapter.cancel).toHaveBeenCalledOnce(); + f.state({ enabled: true, busy: false }); + f.registry.dispatch(insert); + await settle(); + expect(f.adapter.execute).toHaveBeenCalledTimes(2); + f.registry.cancel(); + expect(f.adapter.cancel).toHaveBeenCalledTimes(2); + }); + it("releases the cancellation guard if the plugin throws", () => { + const f = setup(); + f.adapter.cancel.mockImplementationOnce(() => { throw Error("plugin failure"); }); + expect(() => f.registry.cancel()).toThrow("plugin failure"); + expect(() => f.registry.cancel()).not.toThrow(); + expect(f.adapter.cancel).toHaveBeenCalledTimes(2); + }); }); diff --git a/tests/unit/quickToolsAdapterUi.test.js b/tests/unit/quickToolsAdapterUi.test.js index 60ec2af82d..6a2d1450ca 100644 --- a/tests/unit/quickToolsAdapterUi.test.js +++ b/tests/unit/quickToolsAdapterUi.test.js @@ -1,10 +1,18 @@ // @vitest-environment happy-dom import { afterEach, expect, it, vi } from "vitest"; +import { loadSourceModule } from "../helpers/loadSourceModule"; function mockQuickTools() { - vi.doMock("components/quickTools", () => { + vi.doMock("components/quickTools", async () => { const $footer = document.createElement("footer"); $footer.innerHTML = `
`; + const { default: items } = await vi.importActual( + "components/quickTools/items", + ); + const { id, action, value } = items.find((item) => item.id === "save"); + const save = document.createElement("button"); + Object.assign(save.dataset, { id, action, value }); + $footer.children[0].append(save); return { default: { $footer, @@ -92,13 +100,15 @@ async function setup(triggerMode = "click") { const { default: actions, key } = await import("handlers/quickTools"); const { default: stack } = await import("lib/actionStack"); settings.value = { ...settings.value, quickToolsTriggerMode: triggerMode }; - const switchTab = (tab) => { + const switchTab = (tab, beforeNotify = () => {}) => { manager.activeFile = tab; + beforeNotify(); listeners.get("switch-file").forEach((fn) => fn()); }; init(); vi.runOnlyPendingTimers(); return { + manager, tools, registry, settings, @@ -110,6 +120,172 @@ async function setup(triggerMode = "click") { }; } +// Run the real host command mapping, replacing unrelated editor integrations. +function hostCommands(manager, exec) { + const dependencies = Object.fromEntries( + [ + "fileSystem", + "@codemirror/commands", + "@codemirror/language", + "@codemirror/lint", + "@codemirror/lsp-client", + "@codemirror/view", + "cm/editorReadOnly", + "cm/foldAwareLineCommands", + "cm/foldingCommands", + "cm/lsp", + "cm/lsp/references", + "components/symbolsPanel", + "components/toast", + "dialogs/prompt", + "handlers/quickTools", + "lib/settings", + "utils/Url", + ].map((id) => [id, {}]), + ); + return loadSourceModule( + "src/cm/commandRegistry.js", + { + ...dependencies, + "@codemirror/state": { Compartment: class {} }, + "cm/keyBindingUtils": { toCodeMirrorKey: () => null }, + "lib/keyBindings": { + __esModule: true, + default: {}, + APP_KEY_BINDING_NAMES: new Set(), + CODEMIRROR_COMMAND_NAMES: new Set(), + }, + }, + { editorManager: manager, acode: { exec } }, + ); +} + +it.each([ + true, + false, +])("routes the stock Save button through the host with canHandle=%s", async (supported) => { + const f = await adaptedSetup(); + f.adapter.canHandle = () => supported; + const save = vi.fn(); + f.manager.activeFile.save = save; + const exec = vi.fn(() => f.manager.activeFile.save()); + const { executeCommand } = await import("cm/commandRegistry"); + executeCommand.mockImplementation( + hostCommands(f.manager, exec).executeCommand, + ); + f.tools.$footer.querySelector('[data-id="save"]').click(); + await vi.advanceTimersByTimeAsync(0); + expect(exec).toHaveBeenCalledExactlyOnceWith("save"); + expect(save).toHaveBeenCalledOnce(); + expect(f.adapter.execute).not.toHaveBeenCalled(); +}); + +it.each([ + ["saveFileAs", "save-as"], + ["saveAllChanges", "save-all-changes"], + ["openCommandPalette", "command-palette"], +])("routes %s outside adapter availability", async (command, host) => { + const f = await adaptedSetup(); + f.setState({ busy: true }); + f.adapter.canHandle = () => false; + const exec = vi.fn(); + const { executeCommand } = await import("cm/commandRegistry"); + executeCommand.mockImplementation( + hostCommands(f.manager, exec).executeCommand, + ); + expect(f.actions("command", command)).toBe(true); + expect(exec).toHaveBeenCalledExactlyOnceWith(host); + expect(f.adapter.execute).not.toHaveBeenCalled(); +}); + +it("finishes capture before host Save without cancelling queued edits or refocusing over a dialog", async () => { + const f = await adaptedSetup(); + f.actions("ctrl"); + f.actions("key", 39); + const { executeCommand } = await import("cm/commandRegistry"); + const dialogInput = document.createElement("input"); + executeCommand.mockImplementation(() => { + expect(f.key.ctrl).toBe(false); + expect(document.activeElement).not.toBe(f.tools.$input); + document.body.append(dialogInput); + dialogInput.focus(); + return true; + }); + f.tools.$footer.querySelector('[data-id="save"]').click(); + f.tools.$input.dispatchEvent( + new InputEvent("beforeinput", { data: "s", cancelable: true }), + ); + await vi.advanceTimersByTimeAsync(0); + expect(f.adapter.execute).toHaveBeenCalledExactlyOnceWith( + expect.objectContaining({ key: "ArrowRight", ctrlKey: true }), + expect.any(Object), + ); + expect(f.adapter.cancel).not.toHaveBeenCalled(); + expect(f.adapter.focus).not.toHaveBeenCalled(); + expect(document.activeElement).toBe(dialogInput); +}); + +it("cancels only outgoing tabs through the switch handler and preserves incoming work and capture", async () => { + const f = await adaptedSetup(); + const first = f.manager.activeFile, + second = { type: "docs" }, + code = { type: "editor" }; + const next = { + ...f.adapter, + cancel: vi.fn(), + execute: vi.fn(), + captureSelection: () => 20, + restoreSelection: vi.fn(), + }; + cleanups.push(f.registry.register(second, next)); + f.switchTab(code); + expect(f.adapter.cancel).toHaveBeenCalledOnce(); + f.switchTab(first); + expect(f.adapter.cancel).toHaveBeenCalledOnce(); + f.actions("ctrl"); + f.switchTab(second, () => { + // Earlier switch listeners can enqueue work and capture before quicktools cleanup. + f.registry.dispatch({ type: "text", text: "a" }); + f.registry.capture(); + }); + f.registry.dispatch({ type: "text", text: "b" }); + await vi.advanceTimersByTimeAsync(0); + expect(f.key.ctrl).toBe(false); + expect(f.adapter.cancel).toHaveBeenCalledTimes(2); + expect(next.cancel).not.toHaveBeenCalled(); + expect(next.execute.mock.calls.map(([action]) => action.text)).toEqual([ + "a", + "b", + ]); + expect(next.restoreSelection).toHaveBeenCalledExactlyOnceWith(20); + expect( + next.execute.mock.calls.every(([, { signal }]) => !signal.aborted), + ).toBe(true); + f.switchTab(code); + expect(next.cancel).toHaveBeenCalledOnce(); +}); + +it.each([ + "busy", + "disabled", + "overlay", + "disposal", +])("does not repeat registry cancellation during %s UI cleanup", async (transition) => { + const f = await adaptedSetup(); + f.actions("ctrl"); + if (transition === "busy") f.setState({ busy: true }); + if (transition === "disabled") f.setState({ enabled: false }); + if (transition === "disposal") f.dispose(); + if (transition === "overlay") { + const overlay = document.createElement("div"); + overlay.className = "prompt"; + document.body.append(overlay); + } + await vi.advanceTimersByTimeAsync(0); + expect(f.adapter.cancel).toHaveBeenCalledOnce(); + expect(f.key.ctrl).toBe(false); +}); + it("keeps stock items and preferences intact through adapter state and tab changes", async () => { const { tools, registry, settings, syncQuickToolsVisibility, switchTab } = await setup(); diff --git a/tests/unit/saveCommands.test.js b/tests/unit/saveCommands.test.js index eb811fe24b..55bcdcb9a7 100644 --- a/tests/unit/saveCommands.test.js +++ b/tests/unit/saveCommands.test.js @@ -66,3 +66,46 @@ it.each([false, true, "failure"])("save-and-close keeps edits after an unsuccess expect(file.remove).not.toHaveBeenCalled(); expect(file.isUnsaved).toBe(true); }); + +it.each([0, 1, 2])("skips an unsavable dirty tab at position %s while saving eligible tabs", async position => { + for (const command of ["save-all-changes", "close-all-tabs"]) { + const unsupported = tab("unsupported", "terminal"), one = tab("one"), two = tab("two", "editor"); + const files = [one, two]; + files.splice(position, 0, unsupported); + const f = setup(files); + expect(await f.default[command]()).toBe(false); + expect(unsupported.save).not.toHaveBeenCalled(); + expect(unsupported.remove).not.toHaveBeenCalled(); + expect(unsupported.isUnsaved).toBe(true); + for (const file of [one, two]) { + expect(file.save).toHaveBeenCalledOnce(); + expect(file.isUnsaved).toBe(false); + expect(file.remove).toHaveBeenCalledTimes(command === "close-all-tabs" ? 1 : 0); + } + } +}); + +it.each([false, true, "failure"])("still stops after a savable write fails or remains dirty (%s)", async outcome => { + for (const command of ["save-all-changes", "close-all-tabs"]) { + const unsupported = tab("unsupported", "terminal"), failed = tab("failed"), later = tab("later", "editor"); + const f = setup([unsupported, failed, later]); + if (outcome === "failure") failed.save.mockRejectedValueOnce(Error("disk full")); + else failed.save.mockResolvedValueOnce(outcome); + const saving = f.default[command](); + if (outcome === "failure") await expect(saving).rejects.toThrow("disk full"); + else expect(await saving).toBe(false); + expect(failed.remove).not.toHaveBeenCalled(); + expect(later.save).not.toHaveBeenCalled(); + expect(later.remove).not.toHaveBeenCalled(); + } +}); + +it("keeps pinned tabs untouched by save-and-close and reports complete success for eligible tabs", async () => { + const pinned = tab("pinned"), file = tab("file"); + pinned.pinned = true; + const f = setup([pinned, file]); + expect(await f.default["close-all-tabs"]()).toBe(true); + expect(pinned.save).not.toHaveBeenCalled(); + expect(pinned.remove).not.toHaveBeenCalled(); + expect(file.remove).toHaveBeenCalledOnce(); +}); From 0bf84b6f110cdda0a2440665ec532cb5b0aa815c Mon Sep 17 00:00:00 2001 From: Ajit Kumar Date: Mon, 14 Sep 2026 20:45:38 +0530 Subject: [PATCH 4/6] fix: plugin based file system intialization - improve tooltip bug and visibility - UI issues --- src/components/tabView.js | 2 +- src/components/tooltip/longPress.js | 3 +- src/components/tooltip/style.scss | 13 +- src/fileSystem/index.js | 3 + src/lib/editorFile.js | 23 +- src/lib/editorManager.js | 46 ++-- src/lib/loadPlugins.js | 13 +- src/lib/restoreFiles.js | 8 +- src/main.js | 5 +- tests/unit/iconTooltips.test.js | 103 ++++++-- tests/unit/pluginFileRestoration.test.js | 293 +++++++++++++++++++++++ tests/unit/restoreFiles.test.js | 6 +- 12 files changed, 462 insertions(+), 56 deletions(-) create mode 100644 tests/unit/pluginFileRestoration.test.js diff --git a/src/components/tabView.js b/src/components/tabView.js index 86131a2ee7..d8910990b7 100644 --- a/src/components/tabView.js +++ b/src/components/tabView.js @@ -35,7 +35,7 @@ export default function TabView({ id, disableSwipe = false }, children) { if (!activeRect.width) return; const targetLeft = activeRect.left - optionsRect.left; const targetWidth = activeRect.width; - const targetTransform = `translate3d(${targetLeft}px, 0, 0)`; + const targetTransform = `translateX(${targetLeft}px)`; $indicator.style.width = `${targetWidth}px`; if (document.body.classList.contains("no-animation")) { $indicator.style.transform = targetTransform; diff --git a/src/components/tooltip/longPress.js b/src/components/tooltip/longPress.js index 419837bd06..805f221326 100644 --- a/src/components/tooltip/longPress.js +++ b/src/components/tooltip/longPress.js @@ -73,7 +73,8 @@ export function installIconTooltips(root, show, hide) { function click(event) { if ( event.detail && - suppressed?.target === targetOf(event) && + suppressed && + suppressed.target === targetOf(event) && Date.now() < suppressed.until ) { event.preventDefault(); diff --git a/src/components/tooltip/style.scss b/src/components/tooltip/style.scss index fbd3c47c85..ffea0f7c1e 100644 --- a/src/components/tooltip/style.scss +++ b/src/components/tooltip/style.scss @@ -1,15 +1,17 @@ .acode-tooltip { position: fixed; - padding: 6px 10px; + padding: 8px 12px; background: var(--popup-background-color); color: var(--popup-text-color); - border: 1px solid var(--popup-border-color); - box-shadow: 0 0 4px var(--box-shadow-color); + border: 1px solid + color-mix(in srgb, var(--popup-text-color) 30%, var(--popup-background-color)); + box-shadow: 0 4px 12px var(--box-shadow-color); border-radius: var(--popup-border-radius); - font-size: 12px; + font-size: 13px; + font-weight: 500; white-space: normal; overflow-wrap: anywhere; @@ -27,5 +29,6 @@ } .acode-tooltip-description { margin-top: 3px; - opacity: 0.8; + font-size: 12px; + font-weight: 400; } diff --git a/src/fileSystem/index.js b/src/fileSystem/index.js index 7169afdec0..c1ed9db8e0 100644 --- a/src/fileSystem/index.js +++ b/src/fileSystem/index.js @@ -61,6 +61,9 @@ export default function fsOperation(...url) { return fsList.find((fs) => fs.test(url))?.fs(url); } +// Check registration without constructing a transport or opening a file. +fsOperation.hasProvider = (url) => fsList.some((fs) => fs.test(url)); + fsOperation.extend = (test, fs) => { fsList.push({ test, fs }); }; diff --git a/src/lib/editorFile.js b/src/lib/editorFile.js index 12878dbc44..dd9e73ed49 100644 --- a/src/lib/editorFile.js +++ b/src/lib/editorFile.js @@ -25,7 +25,10 @@ import Path from "utils/Path"; import { readRemoteFilePreview } from "utils/remoteFilePreview"; import Url from "utils/Url"; import config from "./config"; -import { isInitialPluginLoadComplete } from "./loadPlugins"; +import { + isInitialPluginLoadComplete, + waitForInitialPluginLoad, +} from "./loadPlugins"; import openFolder from "./openFolder"; import run from "./run"; import saveFile from "./saveFile"; @@ -1199,6 +1202,8 @@ export default class EditorFile { } async writeToCache() { + // A tab switch can flush a restored tab before its document is ready. + if (!this.loaded || this.loading || !this.#tab) return; const writeVersion = this.docVersion; const text = getDocText(this.session.doc); const fs = fsOperation(this.cacheFile); @@ -1839,7 +1844,8 @@ export default class EditorFile { if (this.#type !== "editor") return; let value = ""; const protocol = this.uri ? Url.getProtocol(this.uri) : ""; - const isRemoteFile = protocol === "ftp:" || protocol === "sftp:"; + const isTransportFile = protocol === "ftp:" || protocol === "sftp:"; + const isRemoteFile = isTransportFile || protocol === "gh:"; const { cursorPos, editable } = this.#loadOptions; @@ -1861,7 +1867,7 @@ export default class EditorFile { let savedDoc = null; if (isRemoteFile) { - file = fsOperation(this.uri); + file = isTransportFile ? fsOperation(this.uri) : null; let transportCache = null; try { const localName = file?.localName; @@ -1877,11 +1883,11 @@ export default class EditorFile { transportCache, encoding: this.encoding, }); + if (!this.#tab) return; cacheExists = preview.editorCacheExists; if (cacheExists) value = preview.text; if (preview.text !== null) { - this.session = EditorState.create({ doc: preview.text }); editorManager.emit("file-loading-preview", this, preview.text); } } else { @@ -1892,6 +1898,13 @@ export default class EditorFile { } if (this.uri) { + if ( + !fsOperation.hasProvider(this.uri) && + !isInitialPluginLoadComplete() + ) { + await waitForInitialPluginLoad(); + } + if (!this.#tab) return; file ||= fsOperation(this.uri); const fileExists = await file.exists(); if (!fileExists && cacheExists) { @@ -1911,6 +1924,7 @@ export default class EditorFile { } } + if (!this.#tab) return; const isUnsaved = this.isUnsaved; this.markChanged = false; this.session = restoreSessionSelection( @@ -1938,6 +1952,7 @@ export default class EditorFile { this.#emit("load", createFileEvent(this)); }, 0); } catch (error) { + if (!this.#tab) return; this.#emit("loaderror", createFileEvent(this)); this.remove(false, { ignorePinned: true }); toast(`Unable to load: ${this.filename}`); diff --git a/src/lib/editorManager.js b/src/lib/editorManager.js index 1bf1b1b789..99aa681535 100644 --- a/src/lib/editorManager.js +++ b/src/lib/editorManager.js @@ -141,6 +141,8 @@ async function EditorManager($header, $body) { const PANE_SPLIT_VERTICAL = "vertical"; const docSyncTimers = new WeakMap(); + // Preview text belongs to the loading view, not the file's saved session. + const loadingPreviews = new WeakMap(); let touchSelectionController = null; let touchSelectionSyncRaf = 0; let nativeContextMenuDisabled = null; @@ -1403,6 +1405,7 @@ async function EditorManager($header, $body) { async function configureLspForFile(file) { const pane = getFileLspPane(file); if (!pane?.editor || pane.activeFile?.id !== file?.id) return; + if (!file.loaded || file.loading) return; const targetEditor = pane.editor; const metadata = buildLspMetadata(file, targetEditor); const token = ++pane.lspRequestToken; @@ -2824,16 +2827,18 @@ async function EditorManager($header, $body) { } } - function showLoadingEditor(file, text = "") { + function showLoadingEditor(file, text = loadingPreviews.get(file)) { const loadingState = EditorState.create({ - doc: text, + doc: text ?? "", extensions: [ themeCompartment.of(getConfiguredThemeExtension()), ...getBaseExtensionsFromOptions(), languageCompartment.of([]), lspCompartment.of([]), readOnlyCompartment.of(createEditorReadOnlyExtension(true)), - placeholder(`Loading ${file.filename || "file"}...`), + ...(text === undefined + ? [placeholder(`Loading ${file.filename || "file"}...`)] + : []), ], }); editor.setState(loadingState); @@ -2890,6 +2895,11 @@ async function EditorManager($header, $body) { // Helper: apply a file's content and language to the editor view function applyFileToEditor(file, options = {}) { if (!file || file.type !== "editor") return; + if (!file.loaded || file.loading) { + showLoadingEditor(file); + return; + } + loadingPreviews.delete(file); const { forceRecreate = false, restoreScroll = true, @@ -3289,7 +3299,7 @@ async function EditorManager($header, $body) { diagnosticsButtonSyncRaf = requestAnimationFrame(() => { diagnosticsButtonSyncRaf = 0; const active = manager.activeFile; - if (active?.type === "editor") { + if (active?.type === "editor" && active.loaded && !active.loading) { active.session = editor.state; } toggleProblemButton(); @@ -3441,7 +3451,7 @@ async function EditorManager($header, $body) { function recreateActiveEditorState() { const file = manager.activeFile; - if (file?.type !== "editor") return; + if (file?.type !== "editor" || !file.loaded || file.loading) return; file.session = editor.state; file.lastScrollTop = editor.scrollDOM?.scrollTop ?? 0; @@ -3722,7 +3732,8 @@ async function EditorManager($header, $body) { }); manager.on(["file-loading-preview"], (file, text) => { - if (!file || file.type !== "editor" || !file.loading) return; + if (!file || file.type !== "editor" || !file.loading || !file.tab) return; + loadingPreviews.set(file, text); const pane = getFilePane(file); if (!pane?.editor || pane.activeFile?.id !== file.id) return; @@ -3757,6 +3768,7 @@ async function EditorManager($header, $body) { }); manager.on(["remove-file"], (file) => { + loadingPreviews.delete(file); removeFileFromHistory(file); clearDocSyncTimers(file); detachLspForFile(file); @@ -4139,7 +4151,12 @@ async function EditorManager($header, $body) { return true; } - if (sourcePane?.activeFile?.id === file.id && file.type === "editor") { + if ( + sourcePane?.activeFile?.id === file.id && + file.type === "editor" && + file.loaded && + !file.loading + ) { const sourceEditor = sourcePane.editor; file.session = getRawEditorState(sourceEditor?.state); file.lastScrollTop = sourceEditor?.scrollDOM?.scrollTop || 0; @@ -4975,11 +4992,7 @@ async function EditorManager($header, $body) { file.tab?.classList.add("active"); updateHeaderForFile(file); if (file.type === "editor") { - if (!file.loaded && !file.loading) { - showLoadingEditor(file); - } else { - applyFileToPaneEditor(file, pane); - } + applyFileToPaneEditor(file, pane); pane.editorContainer.style.display = "block"; $hScrollbar.hideImmediately(); @@ -5008,7 +5021,7 @@ async function EditorManager($header, $body) { // Persist the previous editor's state before switching away const prev = paneActiveFile; - if (prev?.type === "editor") { + if (prev?.type === "editor" && prev.loaded && !prev.loading) { prev.session = getRawEditorState(pane.editor.state); prev.lastScrollTop = pane.editor.scrollDOM?.scrollTop || 0; prev.lastScrollLeft = pane.editor.scrollDOM?.scrollLeft || 0; @@ -5033,12 +5046,7 @@ async function EditorManager($header, $body) { if (file.type === "editor") { pane.touchSelectionController?.setEnabled(true); - if (!file.loaded && !file.loading) { - showLoadingEditor(file); - } else { - // Apply active file content and language to CodeMirror - applyFileToEditor(file); - } + applyFileToEditor(file); pane.editorContainer.style.display = "block"; $hScrollbar.hideImmediately(); diff --git a/src/lib/loadPlugins.js b/src/lib/loadPlugins.js index c3475fdaba..bfde1d50f9 100644 --- a/src/lib/loadPlugins.js +++ b/src/lib/loadPlugins.js @@ -37,6 +37,10 @@ const PLUGIN_LOAD_TIMEOUT = 15000; const PLUGIN_DISABLE_TIMEOUT = 60000; let pluginDisabledUpdateQueue = Promise.resolve(); let initialPluginLoadComplete = false; +let resolveInitialPluginLoad; +const initialPluginLoad = new Promise((resolve) => { + resolveInitialPluginLoad = resolve; +}); class PluginLoadTimeoutError extends Error { constructor() { @@ -46,10 +50,6 @@ class PluginLoadTimeoutError extends Error { } export default async function loadPlugins(loadOnlyTheme = false) { - if (!loadOnlyTheme) { - initialPluginLoadComplete = false; - } - try { const plugins = await fsOperation(PLUGIN_DIR).lsDir(); const results = []; @@ -118,6 +118,7 @@ export default async function loadPlugins(loadOnlyTheme = false) { } finally { if (!loadOnlyTheme) { initialPluginLoadComplete = true; + resolveInitialPluginLoad(); } } } @@ -126,6 +127,10 @@ export function isInitialPluginLoadComplete() { return initialPluginLoadComplete; } +export function waitForInitialPluginLoad() { + return initialPluginLoad; +} + export async function loadPluginWithTimeout(pluginId, justInstalled = false) { const pluginState = { settled: false }; const pluginLoadPromise = loadPlugin(pluginId, justInstalled) diff --git a/src/lib/restoreFiles.js b/src/lib/restoreFiles.js index 4f844ac678..980394f8ae 100644 --- a/src/lib/restoreFiles.js +++ b/src/lib/restoreFiles.js @@ -1,3 +1,4 @@ +import fsOperation from "fileSystem"; import EditorFile from "./editorFile"; /** @@ -9,6 +10,7 @@ export default async function restoreFiles(files) { const localLoads = []; files.forEach((file, index) => { + const waitsForProvider = file.uri && !fsOperation.hasProvider(file.uri); const render = file.render || (!hasRenderedFile && index === files.length - 1); const options = { @@ -19,7 +21,7 @@ export default async function restoreFiles(files) { const restoredFile = new EditorFile(file.filename, options); const load = Promise.resolve(restoredFile.load?.()); - if (isRemoteUri(file.uri)) { + if (isRemoteUri(file.uri) || waitsForProvider) { void load.catch((error) => { console.warn(`Failed to preload restored file: ${file.uri}`, error); }); @@ -32,10 +34,10 @@ export default async function restoreFiles(files) { // Finish restoring local documents before startup persistence is enabled. // Otherwise the temporary empty sessions can overwrite saved cursor state, // and the first visit to an inactive local tab visibly flashes a loading editor. - // Remote tabs keep preloading without blocking the rest of app startup. + // Remote and plugin tabs must not block the plugin startup they depend on. await Promise.all(localLoads); } function isRemoteUri(uri) { - return /^(?:https?|s?ftp):/i.test(uri || ""); + return /^(?:https?|s?ftp|gh):/i.test(uri || ""); } diff --git a/src/main.js b/src/main.js index f6d4cd0f05..404fa70de4 100644 --- a/src/main.js +++ b/src/main.js @@ -372,8 +372,9 @@ async function onDeviceReady() { } editorManager.reapplyActiveFile(); if (activeFile?.uri) { - // Re-emit file-loaded event - editorManager.emit("file-loaded", activeFile); + if (activeFile.loaded && !activeFile.loading) { + editorManager.emit("file-loaded", activeFile); + } // Re-emit switch-file event editorManager.emit("switch-file", activeFile); } diff --git a/tests/unit/iconTooltips.test.js b/tests/unit/iconTooltips.test.js index 0423eaa948..17ed23e910 100644 --- a/tests/unit/iconTooltips.test.js +++ b/tests/unit/iconTooltips.test.js @@ -34,6 +34,91 @@ function fixture() { ); return { root, owner, button, icon, show, hide, send }; } + +function click(target, detail = 1) { + const event = new MouseEvent("click", { + bubbles: true, + composed: true, + cancelable: true, + detail, + }); + target.dispatchEvent(event); + return event; +} + +it.each([ + ["plain content", false], + ["plain content", true], + ["unlabeled control", false], + ["unlabeled control", true], + ["quicktools", false], + ["quicktools", true], +])("passes ordinary clicks on %s (Shadow DOM: %s) without browser errors", (kind, shadow) => { + const f = fixture(); + const container = document.createElement("div"); + container.innerHTML = + kind === "plain content" + ? "
Content
" + : ""; + if (kind === "quicktools") { + container.id = "quick-tools"; + container.firstChild.setAttribute("aria-label", "Quicktool"); + } + (shadow ? f.root : document.body).append(container); + const target = container.querySelector("svg") || container.firstChild; + const activated = vi.fn(); + container.firstChild.addEventListener("click", activated); + const errors = vi.fn((event) => event.preventDefault()); + window.addEventListener("error", errors); + try { + for (const type of ["pointerdown", "pointerup"]) { + target.dispatchEvent( + new PointerEvent(type, { + bubbles: true, + composed: true, + pointerId: 1, + button: 0, + }), + ); + } + expect(click(target).defaultPrevented).toBe(false); + expect(activated).toHaveBeenCalledOnce(); + expect(errors).not.toHaveBeenCalled(); + expect(f.show).not.toHaveBeenCalled(); + } finally { + window.removeEventListener("error", errors); + } +}); + +it.each([ + "expired", + "keyboard", + "programmatic", + "different target", +])("allows %s clicks after a long press and clears suppression", async (kind) => { + const f = fixture(), + activated = vi.fn(); + f.button.addEventListener("click", activated); + f.send("pointerdown"); + await vi.advanceTimersByTimeAsync(500); + f.send("pointerup"); + if (kind === "expired") await vi.advanceTimersByTimeAsync(750); + if (kind === "programmatic") f.button.click(); + else if (kind === "different target") { + const other = document.createElement("button"); + document.body.append(other); + other.addEventListener("click", activated); + expect(click(other).defaultPrevented).toBe(false); + } else { + expect(click(f.icon, kind === "keyboard" ? 0 : 1).defaultPrevented).toBe( + false, + ); + } + expect(activated).toHaveBeenCalledOnce(); + expect(click(f.icon).defaultPrevented).toBe(false); + expect(activated).toHaveBeenCalledTimes(2); +}); + it("reads metadata through Shadow DOM and consumes the release click without activating the icon", async () => { const f = fixture(), clicked = vi.fn(); @@ -49,25 +134,11 @@ it("reads metadata through Shadow DOM and consumes the release click without act "Existing description", ); f.send("pointerup"); - f.icon.dispatchEvent( - new MouseEvent("click", { - bubbles: true, - composed: true, - cancelable: true, - detail: 1, - }), - ); + expect(click(f.icon).defaultPrevented).toBe(true); expect(clicked).not.toHaveBeenCalled(); f.send("pointerdown"); f.send("pointerup"); - f.icon.dispatchEvent( - new MouseEvent("click", { - bubbles: true, - composed: true, - cancelable: true, - detail: 1, - }), - ); + expect(click(f.icon).defaultPrevented).toBe(false); expect(clicked).toHaveBeenCalledOnce(); }); it("cancels scrolling, movement, extra touches and removal; leaves stock quicktools alone", async () => { diff --git a/tests/unit/pluginFileRestoration.test.js b/tests/unit/pluginFileRestoration.test.js new file mode 100644 index 0000000000..49bb395ba8 --- /dev/null +++ b/tests/unit/pluginFileRestoration.test.js @@ -0,0 +1,293 @@ +// @vitest-environment happy-dom +import { readFileSync } from "node:fs"; +import vm from "node:vm"; +import { parse } from "@babel/parser"; +import { Compartment, EditorSelection, EditorState } from "@codemirror/state"; +import { EditorView, placeholder } from "@codemirror/view"; +import { + blurEditorIfReadOnly, + createEditorReadOnlyExtension, +} from "cm/editorReadOnly"; +import tag from "html-tag-js"; +import { readRemoteFilePreview } from "utils/remoteFilePreview"; +import { afterEach, expect, it, vi } from "vitest"; +import { loadSourceModule } from "../helpers/loadSourceModule"; + +afterEach(() => { + vi.clearAllTimers(); + vi.useRealTimers(); + document.body.replaceChildren(); +}); + +it("waits for the provider without blocking local files, losing recovery data, or reviving closed tabs", async () => { + vi.useFakeTimers(); + let finishPluginLoad; + const ready = new Promise((resolve) => { + finishPluginLoad = resolve; + }); + let registered = false; + const cache = new Map([ + ["file:///local.js", "local content"], + ["file:///cache/repo", "unsaved edit"], + ]); + const write = vi.fn(); + const remote = { + exists: async () => true, + stat: async () => ({}), + readFile: vi.fn(async () => "remote content"), + }; + const fs = (uri) => + uri.startsWith("gh:") + ? registered + ? remote + : undefined + : { + exists: async () => cache.has(uri), + readFile: async () => cache.get(uri), + stat: async () => ({}), + writeFile: write, + createFile: write, + delete: async () => cache.delete(uri), + lsDir: async () => [{ url: "file:///plugins/github" }], + }; + fs.hasProvider = (uri) => !uri.startsWith("gh:") || registered; + const url = { + join: (...parts) => parts.join("/"), + basename: (value) => value.split("/").at(-1), + getProtocol: (value) => `${value.split(":")[0]}:`, + }; + const settings = { value: {}, on: vi.fn(), off: vi.fn() }; + const acode = {}; + const plugins = loadSourceModule( + "src/lib/loadPlugins.js", + { + "../fileSystem": fs, + "../utils/Url": url, + "./settings": settings, + "./loadPlugin": async () => { + await ready; + registered = true; + }, + }, + { + acode, + PLUGIN_DIR: "file:///plugins", + window: { log: vi.fn() }, + toast: vi.fn(), + strings: {}, + }, + ); + acode[plugins.onPluginLoadCallback] = vi.fn(); + acode[plugins.onPluginsLoadCompleteCallback] = vi.fn(); + const manager = { + files: [], + activeFile: null, + header: {}, + emit: vi.fn(), + onupdate: vi.fn(), + getFile: (id) => manager.files.find((file) => file.id === id), + addFile: (file) => manager.files.push(file), + }; + const toast = vi.fn(); + const log = vi.fn(); + const unused = Object.fromEntries( + [ + "components/quickTools", + "dialogs/confirm", + "handlers/editorFileTab", + "handlers/quickTools", + "handlers/tabContextMenu", + "dompurify", + "mime-types", + "utils/codeHighlight", + "utils/Path", + "./openFolder", + "./run", + "cm/editorReadOnly", + "lib/quickToolsAdapter", + ].map((id) => [id, {}]), + ); + const { default: EditorFile } = loadSourceModule( + "src/lib/editorFile.js", + { + ...unused, + fileSystem: fs, + "@codemirror/state": { EditorState, EditorSelection }, + "cm/editorUtils": { getDocText: (doc) => doc.toString() }, + "cm/modelist": { + getModeForPath: () => "text", + getMode: () => ({ name: "text" }), + }, + "components/sidebar": { hide: vi.fn() }, + "components/toast": toast, + "components/tile": () => { + const tile = document.createElement("li"); + tile.innerHTML = ''; + tile.tail = vi.fn(); + return tile; + }, + "html-tag-js": tag, + "utils/Url": url, + "utils/remoteFilePreview": { readRemoteFilePreview }, + "utils/helpers": { + normalizeMtime: () => null, + getStatMtime: () => null, + getIconForFile: () => "file", + getVirtualPath: (value) => value, + }, + "./config": { DEFAULT_FILE_SESSION: "default" }, + "./loadPlugins": plugins, + "./settings": settings, + "./saveFile": vi.fn(), + }, + { + document, + window: { log }, + tag, + editorManager: manager, + CACHE_STORAGE: "file:///cache", + strings: {}, + }, + ); + EditorFile.prototype.setMode = vi.fn(); + EditorFile.prototype.render = function () { + manager.activeFile = this; + }; + const { default: restoreFiles } = loadSourceModule( + "src/lib/restoreFiles.js", + { fileSystem: fs, "./editorFile": EditorFile }, + ); + + await restoreFiles([ + { + id: "repo", + filename: "repo.js", + uri: "gh://repo/example@main/repo.js", + isUnsaved: true, + cursorPos: { ranges: [{ from: 5, to: 5 }] }, + }, + { id: "closed", filename: "closed.js", uri: "gh://gist/123/closed.js" }, + { + id: "local", + filename: "local.js", + uri: "file:///local.js", + render: true, + }, + ]); + const [repo, closed, local] = manager.files; + expect(log.mock.calls).toEqual([]); + expect(local.session.doc.toString()).toBe("local content"); + expect(repo.loaded).toBe(false); + expect(manager.emit).toHaveBeenCalledWith( + "file-loading-preview", + repo, + "unsaved edit", + ); + expect(remote.readFile).not.toHaveBeenCalled(); + await repo.writeToCache(); + expect(write).not.toHaveBeenCalled(); + const closedLoad = closed.load(); + await closed.remove(true); + const loading = plugins.default(); + finishPluginLoad(); + await loading; + await Promise.all([repo.load(), closedLoad]); + expect(repo.session.doc.toString()).toBe("unsaved edit"); + expect(repo.session.selection.main.head).toBe(5); + expect(repo.isUnsaved).toBe(true); + expect(cache.get("file:///cache/repo")).toBe("unsaved edit"); + expect(closed.session).toBeNull(); + expect(remote.readFile).toHaveBeenCalledOnce(); + expect(manager.activeFile).toBe(local); + expect(toast).not.toHaveBeenCalled(); +}); + +it.each(["cached text", "", undefined])( + "keeps the loading view for cache %j until the session is ready", + (cached) => { + const source = readFileSync("src/lib/editorManager.js", "utf8"); + const body = parse(source, { + sourceType: "module", + plugins: ["jsx"], + }).program.body.find( + (node) => + node.type === "FunctionDeclaration" && node.id.name === "EditorManager", + ).body.body; + const names = [ + "showLoadingEditor", + "applyFileToEditor", + "recreateActiveEditorState", + "getRawEditorState", + "isReusableEditorState", + ]; + const editor = new EditorView({ parent: document.body }); + const file = { + type: "editor", + filename: "file.js", + loaded: false, + loading: true, + session: EditorState.create(), + __cmSessionReady: true, + __cmExtensionSignature: "test", + }; + const loadingPreviews = new WeakMap(); + if (cached !== undefined) loadingPreviews.set(file, cached); + const context = vm.createContext({ + editor, + EditorState, + placeholder, + createEditorReadOnlyExtension, + blurEditorIfReadOnly, + loadingPreviews, + manager: { activeFile: file }, + touchSelectionController: null, + themeCompartment: new Compartment(), + languageCompartment: new Compartment(), + lspCompartment: new Compartment(), + readOnlyCompartment: new Compartment(), + getConfiguredThemeExtension: () => [], + getBaseExtensionsFromOptions: () => [], + getEditorExtensionSignature: () => "test", + getFileLanguageSignature: () => "text", + applyCurrentEditorOptions: vi.fn(), + shouldApplyLanguage: () => false, + restoreFileScrollPosition: vi.fn(), + scheduleLspForFile: vi.fn(), + }); + // Run the actual render functions with a real EditorView; omit the unrelated app shell. + vm.runInContext( + body + .filter( + (node) => + node.type === "FunctionDeclaration" && names.includes(node.id.name), + ) + .map((node) => source.slice(node.start, node.end)) + .join("\n"), + context, + ); + try { + const savedSession = file.session; + context.applyFileToEditor(file); + const preview = editor.state; + context.recreateActiveEditorState(); + expect(editor.state).toBe(preview); + context.applyFileToEditor(file, { forceRecreate: true }); + expect(editor.state.doc.toString()).toBe(cached ?? ""); + expect(editor.state.readOnly).toBe(true); + expect(editor.contentDOM.getAttribute("contenteditable")).toBe("false"); + expect( + editor.dom.querySelector(".cm-placeholder")?.textContent ?? null, + ).toBe(cached === undefined ? "Loading file.js..." : null); + expect(file.session).toBe(savedSession); + file.session = EditorState.create({ doc: cached ?? "loaded text" }); + file.loaded = true; + file.loading = false; + context.applyFileToEditor(file); + expect(editor.state).toBe(file.session); + expect(editor.state.readOnly).toBe(false); + expect(editor.dom.querySelector(".cm-placeholder")).toBeNull(); + } finally { + editor.destroy(); + } + }, +); diff --git a/tests/unit/restoreFiles.test.js b/tests/unit/restoreFiles.test.js index 339a347111..995e648afc 100644 --- a/tests/unit/restoreFiles.test.js +++ b/tests/unit/restoreFiles.test.js @@ -4,6 +4,10 @@ const runtime = vi.hoisted(() => ({ instances: [], })); +vi.mock("fileSystem", () => ({ + default: { hasProvider: (uri) => !uri.startsWith("plugin:") }, +})); + vi.mock("lib/editorFile", () => ({ default: class MockEditorFile { constructor(filename, options) { @@ -53,7 +57,7 @@ describe("restored file loading", () => { expect(completed).toBe(true); }); - it.each(["ftp", "sftp", "http", "https"])( + it.each(["ftp", "sftp", "http", "https", "gh", "plugin"])( "does not block startup on an unresolved %s tab", async (protocol) => { await restoreFiles([ From 52d7d5d4504d5479bea7bbc63dbf0e083b1cd605 Mon Sep 17 00:00:00 2001 From: Ajit Kumar Date: Tue, 15 Sep 2026 14:04:13 +0530 Subject: [PATCH 5/6] fix: restore cached files without waiting for providers Replace the startup wait with generic cache-first restoration and provider registration notifications. Preserve cached documents and keep uncached tabs idle until their filesystem becomes available. --- src/fileSystem/index.js | 16 +- src/lib/editorFile.js | 96 +++---- src/lib/editorManager.js | 17 +- src/lib/loadPlugins.js | 13 +- src/lib/restoreFiles.js | 8 +- src/lib/saveFile.js | 9 +- tests/unit/pluginFileRestoration.test.js | 333 ++++++++++++++++------- tests/unit/restoreFiles.test.js | 6 +- 8 files changed, 324 insertions(+), 174 deletions(-) diff --git a/src/fileSystem/index.js b/src/fileSystem/index.js index c1ed9db8e0..3adbadee71 100644 --- a/src/fileSystem/index.js +++ b/src/fileSystem/index.js @@ -7,6 +7,13 @@ import internalFs from "./internalFs"; import Sftp from "./sftp"; const fsList = []; +const registrationListeners = new Set(); + +// Internal notification; plugins keep using fsOperation.extend as before. +export function onProviderRegistered(listener) { + registrationListeners.add(listener); + return () => registrationListeners.delete(listener); +} /** * @typedef {Object} Stat @@ -62,10 +69,17 @@ export default function fsOperation(...url) { } // Check registration without constructing a transport or opening a file. -fsOperation.hasProvider = (url) => fsList.some((fs) => fs.test(url)); +export function hasProvider(url) { + return fsList.some((fs) => fs.test(url)); +} fsOperation.extend = (test, fs) => { fsList.push({ test, fs }); + for (const listener of registrationListeners) { + Promise.resolve() + .then(() => listener(test)) + .catch(console.error); + } }; fsOperation.remove = (test) => { diff --git a/src/lib/editorFile.js b/src/lib/editorFile.js index dd9e73ed49..1546d90eda 100644 --- a/src/lib/editorFile.js +++ b/src/lib/editorFile.js @@ -1,4 +1,4 @@ -import fsOperation from "fileSystem"; +import fsOperation, { hasProvider } from "fileSystem"; // CodeMirror imports for document state management import { EditorSelection, EditorState } from "@codemirror/state"; import { @@ -25,10 +25,7 @@ import Path from "utils/Path"; import { readRemoteFilePreview } from "utils/remoteFilePreview"; import Url from "utils/Url"; import config from "./config"; -import { - isInitialPluginLoadComplete, - waitForInitialPluginLoad, -} from "./loadPlugins"; +import { isInitialPluginLoadComplete } from "./loadPlugins"; import openFolder from "./openFolder"; import run from "./run"; import saveFile from "./saveFile"; @@ -1463,6 +1460,7 @@ export default class EditorFile { get canSave() { return ( !!this.#tab && + (this.type !== "editor" || (this.loaded && !this.loading)) && (this.type === "editor" || typeof this.onsave === "function" || this.#events.save.length > 0) @@ -1488,7 +1486,7 @@ export default class EditorFile { reconfigureEditorReadOnly( targetEditor, readOnlyCompartment, - readOnly, + readOnly || !this.loaded || this.loading, ); } } @@ -1696,7 +1694,8 @@ export default class EditorFile { * Reuses an in-flight load so session restoration can safely preload tabs. */ load() { - if (this.type !== "editor" || this.loaded) return Promise.resolve(this); + if (this.type !== "editor" || this.loaded || !this.#tab) + return Promise.resolve(this); if (this.#loadPromise) return this.#loadPromise; this.#loadPromise = this.#loadText().finally(() => { @@ -1845,29 +1844,31 @@ export default class EditorFile { let value = ""; const protocol = this.uri ? Url.getProtocol(this.uri) : ""; const isTransportFile = protocol === "ftp:" || protocol === "sftp:"; - const isRemoteFile = isTransportFile || protocol === "gh:"; - const { cursorPos, editable } = this.#loadOptions; - - this.#loadOptions = null; - - if (!editable) { - this.setReadOnly(true); - } - this.loading = true; - this.markChanged = false; - if (isRemoteFile) this.#setRemoteLoading(true); - this.#emit("loadstart", createFileEvent(this)); + let started = false; try { const cacheFs = fsOperation(this.cacheFile); + const cacheExists = await cacheFs.exists(); + if (!this.#tab) return; + if (cacheExists) value = await cacheFs.readFile(this.encoding); + if (!this.#tab) return; + + // An uncached tab stays idle until its filesystem registers. + if (!cacheExists && this.uri && !hasProvider(this.uri)) return; + + started = true; + this.loading = true; + this.markChanged = false; + this.#setRemoteLoading(true); + this.#emit("loadstart", createFileEvent(this)); + if (!this.#tab) return; let file = null; - let cacheExists; let loadedMtime = this.savedMtime; let savedDoc = null; - if (isRemoteFile) { - file = isTransportFile ? fsOperation(this.uri) : null; + if (!cacheExists && isTransportFile) { + file = fsOperation(this.uri); let transportCache = null; try { const localName = file?.localName; @@ -1884,41 +1885,25 @@ export default class EditorFile { encoding: this.encoding, }); if (!this.#tab) return; - cacheExists = preview.editorCacheExists; - if (cacheExists) value = preview.text; if (preview.text !== null) { editorManager.emit("file-loading-preview", this, preview.text); } - } else { - cacheExists = await cacheFs.exists(); - if (cacheExists) { - value = await cacheFs.readFile(this.encoding); - } } - if (this.uri) { - if ( - !fsOperation.hasProvider(this.uri) && - !isInitialPluginLoadComplete() - ) { - await waitForInitialPluginLoad(); - } - if (!this.#tab) return; + if (!this.#tab) return; + if (!cacheExists && this.uri) { file ||= fsOperation(this.uri); - const fileExists = await file.exists(); - if (!fileExists && cacheExists) { - this.deletedFile = true; - this.isUnsaved = true; - } else if (fileExists) { - const stat = await file.stat().catch(() => null); + const fileExists = file.exists ? await file.exists() : true; + if (!this.#tab) return; + if (fileExists) { + const stat = await file.stat?.().catch(() => null); + if (!this.#tab) return; loadedMtime = helpers.getStatMtime(stat); const diskValue = await file.readFile(this.encoding); savedDoc = EditorState.create({ doc: diskValue }).doc; - if (!cacheExists) { - value = diskValue; - } - } else if (!cacheExists && !fileExists) { + value = diskValue; + } else { window.log("error", "unable to load file"); throw new Error("Unable to load file"); } @@ -1935,7 +1920,13 @@ export default class EditorFile { this.__cmSessionReady = false; this.__cmLanguageReady = false; this.__cmLanguageSignature = null; - this.markLoaded({ mtime: loadedMtime, isUnsaved, savedDoc }); + if (cacheExists) { + // Recovery data is the document, not a newly verified disk snapshot. + this.#savedDoc = isUnsaved ? null : this.#rawSession.doc; + } else { + this.markLoaded({ mtime: loadedMtime, isUnsaved, savedDoc }); + } + this.#loadOptions = null; this.markChanged = true; this.loaded = true; this.loading = false; @@ -1949,7 +1940,7 @@ export default class EditorFile { } setTimeout(() => { - this.#emit("load", createFileEvent(this)); + if (this.#tab) this.#emit("load", createFileEvent(this)); }, 0); } catch (error) { if (!this.#tab) return; @@ -1959,8 +1950,11 @@ export default class EditorFile { window.log("error", "Unable to load: " + this.filename); window.log("error", error); } finally { - if (isRemoteFile) this.#setRemoteLoading(false); - this.#emit("loadend", createFileEvent(this)); + this.loading = false; + if (started && this.#tab) { + this.#setRemoteLoading(false); + this.#emit("loadend", createFileEvent(this)); + } } } diff --git a/src/lib/editorManager.js b/src/lib/editorManager.js index 99aa681535..3da350ac8d 100644 --- a/src/lib/editorManager.js +++ b/src/lib/editorManager.js @@ -68,6 +68,7 @@ import { } from "cm/modelist"; import createTouchSelectionMenu from "cm/touchSelectionMenu"; import "cm/supportedModes"; +import { onProviderRegistered } from "fileSystem"; import { autocompletion } from "@codemirror/autocomplete"; import { serverCompletionSource } from "@codemirror/lsp-client"; import colorView from "cm/colorView"; @@ -3721,6 +3722,20 @@ async function EditorManager($header, $body) { } // Register critical listeners + onProviderRegistered((test) => { + for (const file of manager.files) { + if ( + file.type === "editor" && + file.tab && + !file.loaded && + file.uri && + test(file.uri) + ) { + void file.load().catch(console.error); + } + } + }); + manager.on(["file-loaded"], (file) => { if (!file || file.type !== "editor") return; const pane = getFilePane(file); @@ -3753,7 +3768,7 @@ async function EditorManager($header, $body) { const file = manager.activeFile; if (file?.type !== "editor") return; try { - const ro = !file.editable || !!file.loading; + const ro = !file.editable || !file.loaded || file.loading; reconfigureEditorReadOnly(editor, readOnlyCompartment, ro); touchSelectionController?.onStateChanged(); } catch (error) { diff --git a/src/lib/loadPlugins.js b/src/lib/loadPlugins.js index bfde1d50f9..c3475fdaba 100644 --- a/src/lib/loadPlugins.js +++ b/src/lib/loadPlugins.js @@ -37,10 +37,6 @@ const PLUGIN_LOAD_TIMEOUT = 15000; const PLUGIN_DISABLE_TIMEOUT = 60000; let pluginDisabledUpdateQueue = Promise.resolve(); let initialPluginLoadComplete = false; -let resolveInitialPluginLoad; -const initialPluginLoad = new Promise((resolve) => { - resolveInitialPluginLoad = resolve; -}); class PluginLoadTimeoutError extends Error { constructor() { @@ -50,6 +46,10 @@ class PluginLoadTimeoutError extends Error { } export default async function loadPlugins(loadOnlyTheme = false) { + if (!loadOnlyTheme) { + initialPluginLoadComplete = false; + } + try { const plugins = await fsOperation(PLUGIN_DIR).lsDir(); const results = []; @@ -118,7 +118,6 @@ export default async function loadPlugins(loadOnlyTheme = false) { } finally { if (!loadOnlyTheme) { initialPluginLoadComplete = true; - resolveInitialPluginLoad(); } } } @@ -127,10 +126,6 @@ export function isInitialPluginLoadComplete() { return initialPluginLoadComplete; } -export function waitForInitialPluginLoad() { - return initialPluginLoad; -} - export async function loadPluginWithTimeout(pluginId, justInstalled = false) { const pluginState = { settled: false }; const pluginLoadPromise = loadPlugin(pluginId, justInstalled) diff --git a/src/lib/restoreFiles.js b/src/lib/restoreFiles.js index 980394f8ae..fa50829105 100644 --- a/src/lib/restoreFiles.js +++ b/src/lib/restoreFiles.js @@ -1,4 +1,3 @@ -import fsOperation from "fileSystem"; import EditorFile from "./editorFile"; /** @@ -10,7 +9,6 @@ export default async function restoreFiles(files) { const localLoads = []; files.forEach((file, index) => { - const waitsForProvider = file.uri && !fsOperation.hasProvider(file.uri); const render = file.render || (!hasRenderedFile && index === files.length - 1); const options = { @@ -21,7 +19,7 @@ export default async function restoreFiles(files) { const restoredFile = new EditorFile(file.filename, options); const load = Promise.resolve(restoredFile.load?.()); - if (isRemoteUri(file.uri) || waitsForProvider) { + if (file.uri && !/^(?:file|content):/i.test(file.uri)) { void load.catch((error) => { console.warn(`Failed to preload restored file: ${file.uri}`, error); }); @@ -37,7 +35,3 @@ export default async function restoreFiles(files) { // Remote and plugin tabs must not block the plugin startup they depend on. await Promise.all(localLoads); } - -function isRemoteUri(uri) { - return /^(?:https?|s?ftp|gh):/i.test(uri || ""); -} diff --git a/src/lib/saveFile.js b/src/lib/saveFile.js index a01286b84e..8b38295136 100644 --- a/src/lib/saveFile.js +++ b/src/lib/saveFile.js @@ -1,5 +1,6 @@ -import fsOperation from "fileSystem"; +import fsOperation, { hasProvider } from "fileSystem"; import { getDocText } from "cm/editorUtils"; +import toast from "components/toast"; import prompt from "dialogs/prompt"; import select from "dialogs/select"; import recents from "lib/recents"; @@ -22,7 +23,11 @@ const SELECT_FOLDER = "select-folder"; */ async function saveFile(file, isSaveAs = false) { // If file is loading, return - if (file.loading) return; + if (!file.loaded || file.loading) return; + if (!isSaveAs && file.uri && !hasProvider(file.uri)) { + toast(strings["file provider unavailable"] || "File provider unavailable"); + return; + } /** * If set, new file needs to be created diff --git a/tests/unit/pluginFileRestoration.test.js b/tests/unit/pluginFileRestoration.test.js index 49bb395ba8..e499e956a6 100644 --- a/tests/unit/pluginFileRestoration.test.js +++ b/tests/unit/pluginFileRestoration.test.js @@ -19,77 +19,112 @@ afterEach(() => { document.body.replaceChildren(); }); -it("waits for the provider without blocking local files, losing recovery data, or reviving closed tabs", async () => { +const managerSource = readFileSync("src/lib/editorManager.js", "utf8"); +const managerBody = parse(managerSource, { + sourceType: "module", + plugins: ["jsx"], +}).program.body.find( + (node) => + node.type === "FunctionDeclaration" && node.id.name === "EditorManager", +).body.body; + +function setup() { vi.useFakeTimers(); - let finishPluginLoad; - const ready = new Promise((resolve) => { - finishPluginLoad = resolve; - }); - let registered = false; - const cache = new Map([ - ["file:///local.js", "local content"], - ["file:///cache/repo", "unsaved edit"], - ]); - const write = vi.fn(); + const cache = new Map([["file:///local.js", "local content"]]); + const read = vi.fn(async (uri) => cache.get(uri)); + const write = vi.fn(async (uri, text) => cache.set(uri, text)); const remote = { exists: async () => true, stat: async () => ({}), readFile: vi.fn(async () => "remote content"), }; - const fs = (uri) => - uri.startsWith("gh:") - ? registered - ? remote - : undefined - : { - exists: async () => cache.has(uri), - readFile: async () => cache.get(uri), - stat: async () => ({}), - writeFile: write, - createFile: write, - delete: async () => cache.delete(uri), - lsDir: async () => [{ url: "file:///plugins/github" }], - }; - fs.hasProvider = (uri) => !uri.startsWith("gh:") || registered; + const remoteFactory = vi.fn(() => remote); + const localFs = (uri) => ({ + exists: async () => cache.has(uri), + readFile: (encoding) => read(uri, encoding), + stat: async () => ({}), + writeFile: (text) => write(uri, text), + createFile: (name, text) => write(`${uri}/${name}`, text), + delete: async () => cache.delete(uri), + }); const url = { join: (...parts) => parts.join("/"), basename: (value) => value.split("/").at(-1), getProtocol: (value) => `${value.split(":")[0]}:`, }; - const settings = { value: {}, on: vi.fn(), off: vi.fn() }; - const acode = {}; - const plugins = loadSourceModule( - "src/lib/loadPlugins.js", - { - "../fileSystem": fs, - "../utils/Url": url, - "./settings": settings, - "./loadPlugin": async () => { - await ready; - registered = true; - }, + const filesystem = loadSourceModule("src/fileSystem/index.js", { + "lib/ajax": { get: remote.readFile }, + "utils/encodings": { decode: (value) => value }, + "utils/Url": url, + "./internalFs": { + test: (uri) => uri.startsWith("file:"), + createFs: localFs, }, - { - acode, - PLUGIN_DIR: "file:///plugins", - window: { log: vi.fn() }, - toast: vi.fn(), - strings: {}, + "./externalFs": { + test: (uri) => uri.startsWith("content:"), + createFs: localFs, }, - ); - acode[plugins.onPluginLoadCallback] = vi.fn(); - acode[plugins.onPluginsLoadCompleteCallback] = vi.fn(); + "./ftp": { + test: (uri) => uri.startsWith("ftp:"), + fromUrl: remoteFactory, + }, + "./sftp": { + test: (uri) => uri.startsWith("sftp:"), + fromUrl: remoteFactory, + }, + }); + const { default: fs, onProviderRegistered } = filesystem; + const settings = { value: {}, on: vi.fn(), off: vi.fn() }; const manager = { files: [], activeFile: null, header: {}, emit: vi.fn(), onupdate: vi.fn(), - getFile: (id) => manager.files.find((file) => file.id === id), + getFile: (id, key = "id") => manager.files.find((file) => file[key] === id), addFile: (file) => manager.files.push(file), }; - const toast = vi.fn(); - const log = vi.fn(); + // Use the actual registration handler, with the same registry used by EditorFile. + const registration = managerBody.find( + (node) => + node.type === "ExpressionStatement" && + node.expression.callee?.name === "onProviderRegistered", + ); + vm.runInNewContext( + managerSource.slice(registration.start, registration.end), + { onProviderRegistered, manager, console }, + ); + const toast = vi.fn(), + log = vi.fn(); + const helpers = { + normalizeMtime: (value) => value ?? null, + getStatMtime: () => null, + getIconForFile: () => "file", + getVirtualPath: (value) => value, + fixFilename: (value) => value, + }; + const selectLocation = vi.fn(async () => ({ + val: { url: "file:///export" }, + })); + const { default: saveFile } = loadSourceModule( + "src/lib/saveFile.js", + { + fileSystem: filesystem, + "cm/editorUtils": { getDocText: (doc) => doc.toString() }, + "components/toast": toast, + "dialogs/prompt": async () => "", + "dialogs/select": {}, + "lib/recents": { select: selectLocation }, + "pages/fileBrowser": {}, + "utils/helpers": helpers, + "utils/Url": url, + "./config": {}, + "./editorFile": {}, + "./openFolder": {}, + "./settings": settings, + }, + { editorManager: manager, strings: {} }, + ); const unused = Object.fromEntries( [ "components/quickTools", @@ -111,7 +146,7 @@ it("waits for the provider without blocking local files, losing recovery data, o "src/lib/editorFile.js", { ...unused, - fileSystem: fs, + fileSystem: filesystem, "@codemirror/state": { EditorState, EditorSelection }, "cm/editorUtils": { getDocText: (doc) => doc.toString() }, "cm/modelist": { @@ -129,16 +164,11 @@ it("waits for the provider without blocking local files, losing recovery data, o "html-tag-js": tag, "utils/Url": url, "utils/remoteFilePreview": { readRemoteFilePreview }, - "utils/helpers": { - normalizeMtime: () => null, - getStatMtime: () => null, - getIconForFile: () => "file", - getVirtualPath: (value) => value, - }, + "utils/helpers": helpers, "./config": { DEFAULT_FILE_SESSION: "default" }, - "./loadPlugins": plugins, + "./loadPlugins": { isInitialPluginLoadComplete: () => false }, "./settings": settings, - "./saveFile": vi.fn(), + "./saveFile": saveFile, }, { document, @@ -155,64 +185,171 @@ it("waits for the provider without blocking local files, losing recovery data, o }; const { default: restoreFiles } = loadSourceModule( "src/lib/restoreFiles.js", - { fileSystem: fs, "./editorFile": EditorFile }, + { fileSystem: filesystem, "./editorFile": EditorFile }, ); + return { + cache, + read, + write, + remote, + remoteFactory, + fs, + manager, + toast, + log, + selectLocation, + restoreFiles, + }; +} - await restoreFiles([ +it("restores populated and empty recovery caches as usable documents for every filesystem", async () => { + const f = setup(); + const records = [ + "file", + "content", + "ftp", + "sftp", + "https", + "gh", + "plugin", + ].map((protocol, index) => { + const text = index % 2 ? "unsaved content" : ""; + f.cache.set(`file:///cache/${protocol}`, text); + return { + id: protocol, + filename: "file.js", + uri: `${protocol}://example/file.js`, + render: protocol === "gh", + isUnsaved: !!text, + docVersion: 7, + savedVersion: text ? 4 : 7, + cacheVersion: 7, + savedMtime: 100, + diskMtime: 200, + hasDiskConflict: !!text, + pinned: true, + editable: protocol !== "https", + encoding: "utf-16le", + scrollTop: 120, + cursorPos: { ranges: [{ from: text ? 5 : 0, to: text ? 5 : 0 }] }, + }; + }); + await f.restoreFiles(records); + await Promise.all(f.manager.files.map((file) => file.load())); + for (const [index, file] of f.manager.files.entries()) { + const record = records[index]; + expect(file.loaded).toBe(true); + expect(file.loading).toBe(false); + expect(file.session.doc.toString()).toBe(f.cache.get(file.cacheFile)); + expect(file.session.selection.main.head).toBe( + record.cursorPos.ranges[0].to, + ); + for (const key of [ + "isUnsaved", + "docVersion", + "savedVersion", + "cacheVersion", + "savedMtime", + "diskMtime", + "hasDiskConflict", + "pinned", + "editable", + "encoding", + ]) + expect(file[key]).toBe(record[key]); + expect(file.lastScrollTop).toBe(120); + expect(file.canSave).toBe(true); + expect(f.read).toHaveBeenCalledWith(file.cacheFile, "utf-16le"); + } + const github = f.manager.activeFile; + await github.save(); + expect(f.toast).toHaveBeenCalledWith("File provider unavailable"); + await github.saveAs(); // Cancelling the filename prompt still proves Save As is available. + expect(f.selectLocation).toHaveBeenCalledOnce(); + github.session = EditorState.create({ doc: "new offline edit" }); + github.markEdited(); + await github.writeToCache(); + expect(f.cache.get(github.cacheFile)).toBe("new offline edit"); + expect(github.isUnsaved).toBe(true); + f.fs.extend((uri) => /^(gh|plugin):/.test(uri), f.remoteFactory); + await vi.runAllTimersAsync(); + expect(f.remoteFactory).not.toHaveBeenCalled(); + expect(f.remote.readFile).not.toHaveBeenCalled(); + expect(f.log).not.toHaveBeenCalled(); +}); + +it("keeps uncached tabs idle, then resumes only matching open files without blocking local restoration", async () => { + const f = setup(); + let finish; + const response = new Promise((resolve) => { + finish = resolve; + }); + f.remote.readFile.mockReturnValue(response); + await f.restoreFiles([ { - id: "repo", - filename: "repo.js", - uri: "gh://repo/example@main/repo.js", - isUnsaved: true, + id: "pending", + filename: "pending.js", + uri: "custom://pending", cursorPos: { ranges: [{ from: 5, to: 5 }] }, }, - { id: "closed", filename: "closed.js", uri: "gh://gist/123/closed.js" }, + { id: "closed", filename: "closed.js", uri: "custom://closed" }, + { id: "missing", filename: "missing.js", uri: "disabled://missing" }, { id: "local", filename: "local.js", uri: "file:///local.js", render: true, }, + { id: "http", filename: "remote.js", uri: "https://example/remote.js" }, + { id: "closing", filename: "closing.js", uri: "custom://closing" }, ]); - const [repo, closed, local] = manager.files; - expect(log.mock.calls).toEqual([]); + const [pending, closed, missing, local, http, closing] = f.manager.files; expect(local.session.doc.toString()).toBe("local content"); - expect(repo.loaded).toBe(false); - expect(manager.emit).toHaveBeenCalledWith( - "file-loading-preview", - repo, - "unsaved edit", - ); - expect(remote.readFile).not.toHaveBeenCalled(); - await repo.writeToCache(); - expect(write).not.toHaveBeenCalled(); - const closedLoad = closed.load(); + expect(http.loading).toBe(true); + const firstAttempt = pending.load(); + await firstAttempt; + expect(pending.load()).not.toBe(firstAttempt); // No promise is waiting for a plugin. + await pending.load(); + expect(pending.loaded).toBe(false); + expect(pending.loading).toBe(false); + await pending.writeToCache(); + expect(await pending.save()).toBe(false); + expect(await pending.saveAs()).toBe(false); + expect(f.write).not.toHaveBeenCalled(); + expect(f.selectLocation).not.toHaveBeenCalled(); await closed.remove(true); - const loading = plugins.default(); - finishPluginLoad(); - await loading; - await Promise.all([repo.load(), closedLoad]); - expect(repo.session.doc.toString()).toBe("unsaved edit"); - expect(repo.session.selection.main.head).toBe(5); - expect(repo.isUnsaved).toBe(true); - expect(cache.get("file:///cache/repo")).toBe("unsaved edit"); + await vi.advanceTimersByTimeAsync(65000); + expect(pending.tab).not.toBeNull(); + expect(missing.tab).not.toBeNull(); + f.fs.extend((uri) => uri.startsWith("custom:"), f.remoteFactory); + f.fs.extend((uri) => uri.startsWith("custom:"), f.remoteFactory); + await vi.advanceTimersByTimeAsync(0); + expect(f.remote.readFile).toHaveBeenCalledTimes(3); + expect(pending.loading).toBe(true); + const completion = Promise.all([pending.load(), http.load(), closing.load()]); + await closing.remove(true); + finish("remote content"); + await completion; + expect(pending.session.doc.toString()).toBe("remote content"); + expect(pending.session.selection.main.head).toBe(5); + expect(pending.loading).toBe(false); + expect(pending.loaded).toBe(true); expect(closed.session).toBeNull(); - expect(remote.readFile).toHaveBeenCalledOnce(); - expect(manager.activeFile).toBe(local); - expect(toast).not.toHaveBeenCalled(); + expect(closing.session).toBeNull(); + expect(http.session.doc.toString()).toBe("remote content"); + expect(f.manager.activeFile).toBe(local); + expect(missing.loaded).toBe(false); + expect(missing.loading).toBe(false); + expect(f.cache.has(pending.cacheFile)).toBe(false); + expect(f.toast).not.toHaveBeenCalled(); + expect(f.log).not.toHaveBeenCalled(); }); it.each(["cached text", "", undefined])( "keeps the loading view for cache %j until the session is ready", (cached) => { - const source = readFileSync("src/lib/editorManager.js", "utf8"); - const body = parse(source, { - sourceType: "module", - plugins: ["jsx"], - }).program.body.find( - (node) => - node.type === "FunctionDeclaration" && node.id.name === "EditorManager", - ).body.body; + const source = managerSource; + const body = managerBody; const names = [ "showLoadingEditor", "applyFileToEditor", @@ -225,7 +362,7 @@ it.each(["cached text", "", undefined])( type: "editor", filename: "file.js", loaded: false, - loading: true, + loading: false, session: EditorState.create(), __cmSessionReady: true, __cmExtensionSignature: "test", diff --git a/tests/unit/restoreFiles.test.js b/tests/unit/restoreFiles.test.js index 995e648afc..18a8eee3e9 100644 --- a/tests/unit/restoreFiles.test.js +++ b/tests/unit/restoreFiles.test.js @@ -4,10 +4,6 @@ const runtime = vi.hoisted(() => ({ instances: [], })); -vi.mock("fileSystem", () => ({ - default: { hasProvider: (uri) => !uri.startsWith("plugin:") }, -})); - vi.mock("lib/editorFile", () => ({ default: class MockEditorFile { constructor(filename, options) { @@ -57,7 +53,7 @@ describe("restored file loading", () => { expect(completed).toBe(true); }); - it.each(["ftp", "sftp", "http", "https", "gh", "plugin"])( + it.each(["ftp", "sftp", "http", "https", "gh", "plugin", "custom"])( "does not block startup on an unresolved %s tab", async (protocol) => { await restoreFiles([ From 51fe0cbd2fac4c14ecd9f7ff6f6de14cd8292999 Mon Sep 17 00:00:00 2001 From: Ajit Kumar Date: Tue, 15 Sep 2026 16:17:17 +0530 Subject: [PATCH 6/6] fix: load file from cache or defer loading if file system unavailable --- src/lib/checkFiles.js | 32 ++- src/lib/commands.js | 5 +- src/lib/editorFile.js | 90 ++++---- src/lib/editorManager.js | 4 +- src/lib/saveFile.js | 82 ++++++- src/utils/remoteFilePreview.js | 2 +- tests/unit/fileSaveEvents.test.js | 3 +- tests/unit/pluginFileRestoration.test.js | 268 ++++++++++++++++++++++- tests/unit/remoteFilePreview.test.js | 4 +- tests/unit/saveCommands.test.js | 22 +- 10 files changed, 419 insertions(+), 93 deletions(-) diff --git a/src/lib/checkFiles.js b/src/lib/checkFiles.js index 6044498b21..93751f1ee9 100644 --- a/src/lib/checkFiles.js +++ b/src/lib/checkFiles.js @@ -26,19 +26,12 @@ export default async function checkFiles() { /** @type {{ editor: import('@codemirror/view').EditorView }} */ const { editor } = editorManager; - recursiveFileCheck([...files]); - - /** - * Checks if the file has been changed - * @param {EditorFile[]} files List of files to check - */ - async function recursiveFileCheck(files) { - const file = files.pop(); - await checkFile(file); - if (files.length) { - recursiveFileCheck(files); + for (const file of [...files].reverse()) { + try { + await checkFile(file); + } catch (error) { + console.warn("Failed to check file", error); } - return; } /** @@ -51,11 +44,14 @@ export default async function checkFiles() { * @returns {Promise} */ async function checkFile(file) { - if (file === undefined || !file.loaded || file.loading) return; + if (file?.type !== "editor" || !file.tab || !file.loaded || file.loading) + return; if (file.uri) { const fs = fsOperation(file.uri); - const exists = await fs.exists(); + if (!fs) return; + const exists = fs.exists ? await fs.exists() : true; + if (!file.tab) return; if (!exists && !file.readOnly) { file.isUnsaved = true; @@ -74,7 +70,8 @@ export default async function checkFiles() { let mtime = null; if (file.hasVersionMetadata && file.savedMtime != null) { - const stat = await fs.stat().catch(() => null); + const stat = await fs.stat?.().catch(() => null); + if (!file.tab) return; mtime = helpers.getStatMtime(stat); if (mtime != null) { if (mtime === file.savedMtime) return; @@ -104,6 +101,7 @@ export default async function checkFiles() { if (file.isUnsaved) return; const text = await fs.readFile(file.encoding); + if (!file.tab) return; const diskDoc = Text.of(String(text ?? "").split("\n")); const currentDoc = file.session?.doc; @@ -114,7 +112,7 @@ export default async function checkFiles() { file.filename + strings["file changed"], ); - if (!confirmation) return; + if (!confirmation || !file.tab) return; const cursorPos = editor.getCursorPosition(); editorManager.getFile(file.id, "id")?.makeActive(); @@ -126,7 +124,7 @@ export default async function checkFiles() { } finally { file.markChanged = true; } - await file.writeToCache(); + await file.flushCacheWrite(); editor.gotoLine(cursorPos.row, cursorPos.column); } catch (error) { // ignore diff --git a/src/lib/commands.js b/src/lib/commands.js index 918ffef25a..de3a005a8e 100644 --- a/src/lib/commands.js +++ b/src/lib/commands.js @@ -79,6 +79,7 @@ function resolveExactFile(referenceFile) { export function canSaveFile(file = editorManager.activeFile) { return ( (file?.type === "editor" || file?.canSave === true) && + file.canSave !== false && typeof file.save === "function" && typeof file.saveAs === "function" ); @@ -509,7 +510,7 @@ export default { const { activeFile } = editorManager; if (!canSaveFile(activeFile)) return; const saved = await activeFile.save(); - if (showToast && (activeFile.type === "editor" || saved === true)) { + if (showToast && saved === true) { toast(strings["file saved"]); } } catch (error) { @@ -521,7 +522,7 @@ export default { const { activeFile } = editorManager; if (!canSaveFile(activeFile)) return; const saved = await activeFile.saveAs(); - if (showToast && (activeFile.type === "editor" || saved === true)) { + if (showToast && saved === true) { toast(strings["file saved"]); } } catch (error) { diff --git a/src/lib/editorFile.js b/src/lib/editorFile.js index 1546d90eda..03a8c3ba0e 100644 --- a/src/lib/editorFile.js +++ b/src/lib/editorFile.js @@ -32,6 +32,8 @@ import saveFile from "./saveFile"; import appSettings from "./settings"; let mainCSSStyleSheet = null; +// Internal save origin; plugin save events keep their existing shape. +export const AUTO_SAVE = Symbol("auto-save"); function restoreSessionSelection(state, selection) { if (!selection?.ranges?.length) return state; @@ -471,6 +473,7 @@ export default class EditorFile { #hasVersionMetadata = false; #cacheWriteTimer = null; #cacheWritePromise = null; + #hasCache = false; #savedDoc = null; /** * Whether to show run button or not @@ -774,6 +777,7 @@ export default class EditorFile { } if (options?.render ?? true) this.render(); + if (this.loaded) void this.scheduleCacheWrite(0); } get type() { @@ -1101,7 +1105,7 @@ export default class EditorFile { const normalizedMtime = helpers.normalizeMtime(mtime); this.docVersion = isUnsaved ? 1 : 0; this.savedVersion = isUnsaved ? 0 : this.docVersion; - this.cacheVersion = isUnsaved ? this.docVersion : this.savedVersion; + this.cacheVersion = -1; this.savedMtime = normalizedMtime; this.diskMtime = normalizedMtime; this.hasDiskConflict = false; @@ -1154,43 +1158,40 @@ export default class EditorFile { } scheduleCacheWrite(delay = 1500) { - if (this.type !== "editor") return Promise.resolve(); - if (this.cacheVersion === this.docVersion && this.#hasVersionMetadata) { + if ( + this.type !== "editor" || + this.id === config.DEFAULT_FILE_SESSION || + !this.canSave + ) + return Promise.resolve(); + if (this.#hasCache && this.cacheVersion === this.docVersion) { return this.#cacheWritePromise || Promise.resolve(); } if (this.#cacheWriteTimer) clearTimeout(this.#cacheWriteTimer); - if (delay <= 0) { - this.#cacheWriteTimer = null; - this.#cacheWritePromise = this.writeToCache().finally(() => { - this.#cacheWritePromise = null; - }); - return this.#cacheWritePromise; - } + if (delay <= 0) return this.flushCacheWrite(); this.#cacheWriteTimer = setTimeout(() => { this.#cacheWriteTimer = null; - this.#cacheWritePromise = this.writeToCache().finally(() => { - this.#cacheWritePromise = null; - }); + void this.flushCacheWrite(); }, delay); return Promise.resolve(); } async flushCacheWrite() { + if ( + this.type !== "editor" || + this.id === config.DEFAULT_FILE_SESSION || + !this.canSave + ) + return; if (this.#cacheWriteTimer) { clearTimeout(this.#cacheWriteTimer); this.#cacheWriteTimer = null; - if (!this.#cacheWritePromise) { - this.#cacheWritePromise = this.writeToCache().finally(() => { - this.#cacheWritePromise = null; - }); - } } - if (this.#cacheWritePromise) await this.#cacheWritePromise; - if (this.cacheVersion !== this.docVersion) { - if (this.#cacheWriteTimer) { - clearTimeout(this.#cacheWriteTimer); - this.#cacheWriteTimer = null; - } + if (this.#cacheWritePromise) { + await this.#cacheWritePromise; + return this.flushCacheWrite(); + } + if (!this.#hasCache || this.cacheVersion !== this.docVersion) { this.#cacheWritePromise = this.writeToCache().finally(() => { this.#cacheWritePromise = null; }); @@ -1203,18 +1204,18 @@ export default class EditorFile { if (!this.loaded || this.loading || !this.#tab) return; const writeVersion = this.docVersion; const text = getDocText(this.session.doc); - const fs = fsOperation(this.cacheFile); try { - if (!(await fs.exists())) { + const fs = fsOperation(this.cacheFile); + const exists = await fs.exists(); + if (!this.#tab) return; + if (!exists) { await fsOperation(CACHE_STORAGE).createFile(this.id, text); - this.cacheVersion = writeVersion; - this.#hasVersionMetadata = true; - if (this.docVersion !== writeVersion) this.scheduleCacheWrite(); - return; + } else { + await fs.writeFile(text); } - - await fs.writeFile(text); + if (!this.#tab) return; + this.#hasCache = true; this.cacheVersion = writeVersion; this.#hasVersionMetadata = true; if (this.docVersion !== writeVersion) this.scheduleCacheWrite(); @@ -1444,8 +1445,8 @@ export default class EditorFile { * Saves the file. * @returns {Promise} true if file is saved, false if not. */ - save() { - return this.#save(false); + save(origin) { + return this.#save(false, origin === AUTO_SAVE); } /** @@ -1851,8 +1852,9 @@ export default class EditorFile { const cacheFs = fsOperation(this.cacheFile); const cacheExists = await cacheFs.exists(); if (!this.#tab) return; - if (cacheExists) value = await cacheFs.readFile(this.encoding); + if (cacheExists) value = await cacheFs.readFile("utf-8"); if (!this.#tab) return; + this.#hasCache = cacheExists; // An uncached tab stays idle until its filesystem registers. if (!cacheExists && this.uri && !hasProvider(this.uri)) return; @@ -1930,6 +1932,7 @@ export default class EditorFile { this.markChanged = true; this.loaded = true; this.loading = false; + if (!cacheExists) void this.scheduleCacheWrite(0); const { activeFile, emit } = editorManager; const pane = editorManager.getFilePane?.(this); @@ -1982,20 +1985,19 @@ export default class EditorFile { // editorManager.editor._emit("scrollleft", e); // } - #save(as) { + #save(as, automatic = false) { if (!this.canSave) return Promise.resolve(false); - if (this.type === "editor") return this.#dispatchSave(as); if (this.#pendingSave) return this.#pendingSave; // Set the promise before dispatch so re-entrant requests also share it. this.#pendingSave = Promise.resolve() - .then(() => (this.canSave ? this.#dispatchSave(as) : false)) + .then(() => (this.canSave ? this.#dispatchSave(as, automatic) : false)) .finally(() => { this.#pendingSave = null; }); return this.#pendingSave; } - #dispatchSave(as) { + #dispatchSave(as, automatic) { const event = new SaveFileEvent(this, as); try { this.#emit("save", event); @@ -2016,7 +2018,10 @@ export default class EditorFile { }); if (event.defaultPrevented || this.type !== "editor") return Promise.resolve(false); - return Promise.all([this.flushCacheWrite(), saveFile(this, as)]); + return Promise.all([ + this.flushCacheWrite(), + saveFile(this, as, { automatic, savedDoc: this.#savedDoc }), + ]).then(([, saved]) => saved === true); } #run(file) { @@ -2060,10 +2065,11 @@ export default class EditorFile { clearTimeout(this.#cacheWriteTimer); this.#cacheWriteTimer = null; } - this.#cacheWritePromise = null; this.#savedDoc = null; if (this.type === "editor") { - this.#removeCache(); + void Promise.resolve(this.#cacheWritePromise).then(() => + this.#removeCache(), + ); // CodeMirror EditorState doesn't need explicit cleanup this.session = null; } else if (this.content) { diff --git a/src/lib/editorManager.js b/src/lib/editorManager.js index 3da350ac8d..ea2b573299 100644 --- a/src/lib/editorManager.js +++ b/src/lib/editorManager.js @@ -98,7 +98,7 @@ import SideButton, { sideButtonContainer } from "components/sideButton"; import keyboardHandler, { keydownState } from "handlers/keyboard"; import { animate } from "motion"; import config from "./config"; -import EditorFile from "./editorFile"; +import EditorFile, { AUTO_SAVE } from "./editorFile"; import openFile from "./openFile"; import { addedFolder } from "./openFolder"; import appSettings from "./settings"; @@ -3706,7 +3706,7 @@ async function EditorManager($header, $body) { if (file.uri && file.isUnsaved && autosave) { timers.autosaveTimeout = setTimeout(() => { timers.autosaveTimeout = null; - file.save()?.catch?.((error) => { + file.save(AUTO_SAVE)?.catch?.((error) => { warnRecoverable( `Failed to autosave ${file.filename || file.uri}`, error, diff --git a/src/lib/saveFile.js b/src/lib/saveFile.js index 8b38295136..0f43feaac5 100644 --- a/src/lib/saveFile.js +++ b/src/lib/saveFile.js @@ -1,6 +1,8 @@ import fsOperation, { hasProvider } from "fileSystem"; +import { Text } from "@codemirror/state"; import { getDocText } from "cm/editorUtils"; import toast from "components/toast"; +import confirm from "dialogs/confirm"; import prompt from "dialogs/prompt"; import select from "dialogs/select"; import recents from "lib/recents"; @@ -20,14 +22,26 @@ const SELECT_FOLDER = "select-folder"; * Saves a file to it's location, if file is new, it will ask for location * @param {EditorFile} file * @param {boolean} [isSaveAs] + * @param {{automatic?: boolean, savedDoc?: import('@codemirror/state').Text}} [options] Internal save context. + * @returns {Promise} */ -async function saveFile(file, isSaveAs = false) { +async function saveFile( + file, + isSaveAs = false, + { automatic = false, savedDoc: baseline = null } = {}, +) { // If file is loading, return - if (!file.loaded || file.loading) return; + if (!file.loaded || file.loading || !file.tab) return false; if (!isSaveAs && file.uri && !hasProvider(file.uri)) { - toast(strings["file provider unavailable"] || "File provider unavailable"); - return; + if (!automatic) + toast( + strings["file provider unavailable"] || "File provider unavailable", + ); + return false; } + const tab = file.tab; + let targetUri = file.uri; + const isCurrent = () => file.tab === tab && file.uri === targetUri; /** * If set, new file needs to be created @@ -73,12 +87,14 @@ async function saveFile(file, isSaveAs = false) { "dir", // type strings["select folder"], // title ); + if (!option || !isCurrent()) return false; if (option === SELECT_FOLDER) { newUrl = await selectFolder(); } else { newUrl = option.val.url; } + if (!newUrl || !isCurrent()) return false; if (isSaveAs) { filename = await getfilename(newUrl, file.filename); @@ -87,8 +103,8 @@ async function saveFile(file, isSaveAs = false) { } // in case if user cancels the dialog - if (!filename) { - return; + if (!filename || !isCurrent()) { + return false; } } @@ -98,6 +114,7 @@ async function saveFile(file, isSaveAs = false) { $text.textContent = strings.saving + "..."; file.isSaving = true; + let saved = false; try { if (isSaveAs || newUrl) { @@ -105,13 +122,17 @@ async function saveFile(file, isSaveAs = false) { const fileUri = Url.join(newUrl, file.filename); fileOnDevice = fsOperation(fileUri); - if (!(await fileOnDevice.exists())) { + const exists = await fileOnDevice.exists(); + if (!isCurrent()) return false; + if (!exists) { await fsOperation(newUrl).createFile(file.filename); } + if (!isCurrent()) return false; const openedFile = editorManager.getFile(fileUri, "uri"); if (openedFile) openedFile.uri = null; file.uri = fileUri; + targetUri = fileUri; recents.addFile(fileUri); const folder = openFolder.find(newUrl); @@ -121,6 +142,34 @@ async function saveFile(file, isSaveAs = false) { if (!fileOnDevice) { fileOnDevice = fsOperation(file.uri); } + if (!fileOnDevice || !isCurrent()) return false; + + if (!isSaveAs && !isNewFile) { + const source = Text.of( + (await fileOnDevice.readFile(encoding)).split(/\r\n?|\n/), + ); + if (!isCurrent()) return false; + let unchanged = source.eq(file.session.doc); + if (!unchanged && baseline) { + unchanged = source.eq(baseline); + } else if (!unchanged && file.savedMtime != null) { + const stat = await fileOnDevice.stat?.().catch(() => null); + if (!isCurrent()) return false; + unchanged = helpers.getStatMtime(stat) === file.savedMtime; + } + if (!unchanged) { + file.hasDiskConflict = true; + file.refreshUnsavedState(); + editorManager.onupdate("file-changed"); + editorManager.emit("update", "file-changed"); + if (automatic) return false; + const overwrite = await confirm( + strings.warning || "Warning", + `${file.filename}: the source has changed or its saved version could not be verified. Overwrite it with this document?`, + ); + if (!overwrite || !isCurrent()) return false; + } + } if (appSettings.value.formatOnSave) { editorManager.activeFile.markChanged = false; @@ -134,14 +183,18 @@ async function saveFile(file, isSaveAs = false) { const savedDoc = file.session?.doc || null; const savedVersion = file.docVersion; const data = getDocText(savedDoc); + if (!isCurrent()) return false; await fileOnDevice.writeFile(data, encoding); - const stat = await fileOnDevice.stat().catch(() => null); + if (!isCurrent()) return false; + const stat = await fileOnDevice.stat?.().catch(() => null); + if (!isCurrent()) return false; file.markSaved({ mtime: helpers.getStatMtime(stat), savedDoc, savedVersion, }); + saved = true; if (file.location) { recents.addFolder(file.location); @@ -149,6 +202,7 @@ async function saveFile(file, isSaveAs = false) { clearTimeout(saveTimeout); saveTimeout = setTimeout(() => { + if (!isCurrent()) return; file.isSaving = false; if (newUrl) recents.addFile(file.uri); editorManager.onupdate("save-file"); @@ -156,14 +210,20 @@ async function saveFile(file, isSaveAs = false) { editorManager.emit("save-file", file); resetText(); }, editorManager.TIMEOUT_VALUE + 100); + return true; } catch (err) { - helpers.error(err); + if (!automatic && isCurrent()) helpers.error(err); + return false; + } finally { + if (isCurrent()) { + if (!saved) file.isSaving = false; + resetText(); + } } - resetText(); function resetText() { setTimeout(() => { - $text.textContent = file.filename; + if (isCurrent()) $text.textContent = file.filename; }, editorManager.TIMEOUT_VALUE); } diff --git a/src/utils/remoteFilePreview.js b/src/utils/remoteFilePreview.js index d022383ef9..04c19e8434 100644 --- a/src/utils/remoteFilePreview.js +++ b/src/utils/remoteFilePreview.js @@ -19,7 +19,7 @@ export async function readRemoteFilePreview({ if (editorCacheExists) { return { editorCacheExists, - text: await editorCache.readFile(encoding), + text: await editorCache.readFile("utf-8"), }; } diff --git a/tests/unit/fileSaveEvents.test.js b/tests/unit/fileSaveEvents.test.js index 8374b7368f..ad6c5aa73f 100644 --- a/tests/unit/fileSaveEvents.test.js +++ b/tests/unit/fileSaveEvents.test.js @@ -31,6 +31,7 @@ function setup() { "./saveFile": write, }, { document, window, tag, editorManager: manager }); EditorFile.prototype.setMode = vi.fn(); // Language setup is unrelated to save routing. + EditorFile.prototype.writeToCache = vi.fn(async () => {}); const file = (type = "docs") => new EditorFile("test.txt", { id: String(manager.files.length), type, content: document.createElement("div"), render: false, text: "original", }); @@ -93,7 +94,7 @@ it("preserves text fallback and existing cancellation/listener ordering", async file.flushCacheWrite = vi.fn(async () => {}); expect(file.canSave).toBe(true); await file.saveAs(); - expect(f.write).toHaveBeenCalledWith(file, true); + expect(f.write).toHaveBeenCalledWith(file, true, { automatic: false, savedDoc: file.session.doc }); const cancel = e => e.preventDefault(), observe = vi.fn(); file.onsave = cancel; file.on("save", observe); diff --git a/tests/unit/pluginFileRestoration.test.js b/tests/unit/pluginFileRestoration.test.js index e499e956a6..2f33bf5940 100644 --- a/tests/unit/pluginFileRestoration.test.js +++ b/tests/unit/pluginFileRestoration.test.js @@ -2,7 +2,12 @@ import { readFileSync } from "node:fs"; import vm from "node:vm"; import { parse } from "@babel/parser"; -import { Compartment, EditorSelection, EditorState } from "@codemirror/state"; +import { + Compartment, + EditorSelection, + EditorState, + Text, +} from "@codemirror/state"; import { EditorView, placeholder } from "@codemirror/view"; import { blurEditorIfReadOnly, @@ -31,18 +36,24 @@ const managerBody = parse(managerSource, { function setup() { vi.useFakeTimers(); const cache = new Map([["file:///local.js", "local content"]]); - const read = vi.fn(async (uri) => cache.get(uri)); + const read = vi.fn(async (uri, encoding) => { + const value = cache.get(uri); + return ArrayBuffer.isView(value) + ? new TextDecoder(encoding).decode(value) + : value; + }); const write = vi.fn(async (uri, text) => cache.set(uri, text)); + const stat = vi.fn(async () => ({})); const remote = { exists: async () => true, - stat: async () => ({}), + stat, readFile: vi.fn(async () => "remote content"), }; const remoteFactory = vi.fn(() => remote); const localFs = (uri) => ({ exists: async () => cache.has(uri), readFile: (encoding) => read(uri, encoding), - stat: async () => ({}), + stat, writeFile: (text) => write(uri, text), createFile: (name, text) => write(`${uri}/${name}`, text), delete: async () => cache.delete(uri), @@ -76,6 +87,7 @@ function setup() { const { default: fs, onProviderRegistered } = filesystem; const settings = { value: {}, on: vi.fn(), off: vi.fn() }; const manager = { + TIMEOUT_VALUE: 0, files: [], activeFile: null, header: {}, @@ -96,9 +108,12 @@ function setup() { ); const toast = vi.fn(), log = vi.fn(); + const confirm = vi.fn(async () => false); + const error = vi.fn(); const helpers = { normalizeMtime: (value) => value ?? null, - getStatMtime: () => null, + getStatMtime: (value) => value?.modifiedDate ?? null, + error, getIconForFile: () => "file", getVirtualPath: (value) => value, fixFilename: (value) => value, @@ -110,11 +125,16 @@ function setup() { "src/lib/saveFile.js", { fileSystem: filesystem, + "@codemirror/state": { Text }, "cm/editorUtils": { getDocText: (doc) => doc.toString() }, "components/toast": toast, + "dialogs/confirm": confirm, "dialogs/prompt": async () => "", "dialogs/select": {}, - "lib/recents": { select: selectLocation }, + "lib/recents": { + select: selectLocation, + addFolder: vi.fn(), + }, "pages/fileBrowser": {}, "utils/helpers": helpers, "utils/Url": url, @@ -142,7 +162,7 @@ function setup() { "lib/quickToolsAdapter", ].map((id) => [id, {}]), ); - const { default: EditorFile } = loadSourceModule( + const { default: EditorFile, AUTO_SAVE } = loadSourceModule( "src/lib/editorFile.js", { ...unused, @@ -188,6 +208,10 @@ function setup() { { fileSystem: filesystem, "./editorFile": EditorFile }, ); return { + AUTO_SAVE, + confirm, + error, + stat, cache, read, write, @@ -259,7 +283,7 @@ it("restores populated and empty recovery caches as usable documents for every f expect(file[key]).toBe(record[key]); expect(file.lastScrollTop).toBe(120); expect(file.canSave).toBe(true); - expect(f.read).toHaveBeenCalledWith(file.cacheFile, "utf-16le"); + expect(f.read).toHaveBeenCalledWith(file.cacheFile, "utf-8"); } const github = f.manager.activeFile; await github.save(); @@ -271,6 +295,9 @@ it("restores populated and empty recovery caches as usable documents for every f await github.writeToCache(); expect(f.cache.get(github.cacheFile)).toBe("new offline edit"); expect(github.isUnsaved).toBe(true); + await vi.advanceTimersByTimeAsync(65000); + expect(github.tab).not.toBeNull(); + expect(f.cache.get(github.cacheFile)).toBe("new offline edit"); f.fs.extend((uri) => /^(gh|plugin):/.test(uri), f.remoteFactory); await vi.runAllTimersAsync(); expect(f.remoteFactory).not.toHaveBeenCalled(); @@ -304,6 +331,8 @@ it("keeps uncached tabs idle, then resumes only matching open files without bloc { id: "closing", filename: "closing.js", uri: "custom://closing" }, ]); const [pending, closed, missing, local, http, closing] = f.manager.files; + await local.flushCacheWrite(); + f.write.mockClear(); expect(local.session.doc.toString()).toBe("local content"); expect(http.loading).toBe(true); const firstAttempt = pending.load(); @@ -340,11 +369,232 @@ it("keeps uncached tabs idle, then resumes only matching open files without bloc expect(f.manager.activeFile).toBe(local); expect(missing.loaded).toBe(false); expect(missing.loading).toBe(false); - expect(f.cache.has(pending.cacheFile)).toBe(false); + await pending.flushCacheWrite(); + expect(f.cache.get(pending.cacheFile)).toBe("remote content"); expect(f.toast).not.toHaveBeenCalled(); expect(f.log).not.toHaveBeenCalled(); }); +it("caches unchanged and empty documents from both opening paths, then restores without source reads", async () => { + for (const supplied of [false, true]) { + for (const text of ["café", ""]) { + const f = setup(); + f.cache.set("file:///local.js", text); + const record = { + id: "initial", + uri: "file:///local.js", + filename: "local.js", + encoding: "utf-16le", + isUnsaved: false, + }; + await f.restoreFiles([{ ...record, ...(supplied ? { text } : {}) }]); + const file = f.manager.files[0]; + await file.flushCacheWrite(); + expect(f.cache.get(file.cacheFile)).toBe(text); + expect(file.isUnsaved).toBe(false); + await file.flushCacheWrite(); + expect(f.write).toHaveBeenCalledOnce(); + // Recovery bytes are UTF-8 even when the original file uses UTF-16. + const reopened = setup(); + reopened.cache.set(file.cacheFile, new TextEncoder().encode(text)); + await reopened.restoreFiles([record]); + const restored = reopened.manager.files[0]; + expect(restored.session.doc.toString()).toBe(text); + expect(restored.encoding).toBe("utf-16le"); + expect(reopened.read.mock.calls.map(([uri]) => uri)).toEqual([ + file.cacheFile, + ]); + expect(reopened.write).not.toHaveBeenCalled(); + } + } +}); + +it("retries a failed initial cache write and removes an in-flight cache when its tab closes", async () => { + const f = setup(); + f.write.mockRejectedValueOnce(Error("disk full")); + await f.restoreFiles([ + { id: "retry", uri: "file:///local.js", filename: "local.js" }, + ]); + await vi.runAllTimersAsync(); + const file = f.manager.files[0]; + expect(f.cache.has(file.cacheFile)).toBe(false); + expect(file.cacheVersion).not.toBe(file.docVersion); + await file.flushCacheWrite(); + expect(f.cache.get(file.cacheFile)).toBe("local content"); + let finish; + f.write.mockImplementationOnce( + (uri, text) => + new Promise((resolve) => { + finish = () => { + f.cache.set(uri, text); + resolve(); + }; + }), + ); + file.session.setValue("new edit"); + const writing = file.flushCacheWrite(); + await vi.waitFor(() => expect(finish).toBeTypeOf("function")); + await file.remove(true); + finish(); + await writing; + await vi.runAllTimersAsync(); + expect(f.cache.has(file.cacheFile)).toBe(false); + expect(file.session).toBeNull(); +}); + +it("checks a clean cache before saving, cancels safely, and shares overlapping overwrite requests", async () => { + const f = setup(); + f.cache.set("file:///cache/stale", "old text"); + f.cache.set("file:///local.js", "external changes"); + await f.restoreFiles([ + { + id: "stale", + uri: "file:///local.js", + filename: "local.js", + isUnsaved: false, + }, + ]); + const file = f.manager.files[0]; + expect(await file.save()).toBe(false); + expect(f.cache.get(file.uri)).toBe("external changes"); + expect(f.cache.get(file.cacheFile)).toBe("old text"); + expect(file.hasDiskConflict).toBe(true); + expect(file.isSaving).toBe(false); + f.confirm.mockClear(); + expect(await file.save(f.AUTO_SAVE)).toBe(false); + expect(f.confirm).not.toHaveBeenCalled(); + let approve; + f.confirm.mockImplementationOnce( + () => + new Promise((resolve) => { + approve = resolve; + }), + ); + const saving = file.save(); + expect(file.save()).toBe(saving); + await vi.waitFor(() => expect(approve).toBeTypeOf("function")); + approve(true); + expect(await saving).toBe(true); + expect(f.confirm).toHaveBeenCalledOnce(); + expect(f.cache.get(file.uri)).toBe("old text"); + expect(file.isUnsaved).toBe(false); + expect(file.hasDiskConflict).toBe(false); +}); + +it("verifies dirty recovery metadata and preserves unverifiable edits", async () => { + for (const mtime of [10, 20, null]) { + const f = setup(); + f.stat.mockResolvedValue({ modifiedDate: mtime }); + f.cache.set("file:///cache/dirty", "recovered edits"); + await f.restoreFiles([ + { + id: "dirty", + uri: "file:///local.js", + filename: "local.js", + isUnsaved: true, + savedMtime: 10, + }, + ]); + const file = f.manager.files[0]; + expect(await file.save()).toBe(mtime === 10); + expect(f.confirm).toHaveBeenCalledTimes(mtime === 10 ? 0 : 1); + expect(f.cache.get(file.uri)).toBe( + mtime === 10 ? "recovered edits" : "local content", + ); + expect(f.cache.get(file.cacheFile)).toBe("recovered edits"); + } +}); + +it("allows unchanged-source edits and stops failed or obsolete source verification", async () => { + const f = setup(); + f.cache.set("file:///local.js", "local\r\ncontent"); + await f.restoreFiles([ + { id: "safe", uri: "file:///local.js", filename: "local.js" }, + ]); + const file = f.manager.files[0]; + await file.flushCacheWrite(); + file.session.setValue("my edit"); + expect(await file.save(f.AUTO_SAVE)).toBe(true); + expect(f.confirm).not.toHaveBeenCalled(); + expect(f.cache.get(file.uri)).toBe("my edit"); + f.read.mockRejectedValueOnce(Error("offline")); + expect(await file.save()).toBe(false); + expect(f.error).toHaveBeenCalledOnce(); + let finish; + f.read.mockImplementationOnce( + () => + new Promise((resolve) => { + finish = resolve; + }), + ); + const saving = file.save(); + await vi.waitFor(() => expect(finish).toBeTypeOf("function")); + await file.remove(true); + const writes = f.write.mock.calls.length; + finish("my edit"); + expect(await saving).toBe(false); + expect(f.write).toHaveBeenCalledTimes(writes); + await vi.runAllTimersAsync(); + expect(f.cache.has(file.cacheFile)).toBe(false); +}); + +it("continues resume checks past unavailable, failing, and read-only HTTP providers", async () => { + const f = setup(); + f.cache.set("file:///cache/offline", "cached"); + f.cache.set("file:///cache/broken", "cached"); + f.cache.set("file:///cache/http", "remote content"); + await f.restoreFiles([ + { + id: "local", + uri: "file:///local.js", + filename: "local.js", + encoding: "utf-8", + }, + { + id: "http", + uri: "https://example/file", + filename: "file", + readOnly: true, + }, + { id: "broken", uri: "broken://file", filename: "file" }, + { + id: "offline", + uri: "disabled://file", + filename: "file", + isUnsaved: true, + }, + { id: "pending", uri: "disabled://pending", filename: "pending" }, + ]); + await Promise.all(f.manager.files.map((file) => file.load())); + f.fs.extend( + (uri) => uri.startsWith("broken:"), + () => ({ + exists: async () => { + throw Error("offline"); + }, + }), + ); + const warn = vi.fn(); + const { default: checkFiles } = loadSourceModule( + "src/lib/checkFiles.js", + { + fileSystem: f.fs, + "@codemirror/state": { Text }, + "dialogs/alert": vi.fn(), + "dialogs/confirm": f.confirm, + "utils/helpers": { getStatMtime: () => null }, + }, + { editorManager: f.manager, strings: {}, console: { warn } }, + ); + f.read.mockClear(); + await checkFiles(); + expect(warn).toHaveBeenCalledOnce(); + expect(f.read).toHaveBeenCalledWith("file:///local.js", "utf-8"); + expect(f.confirm).not.toHaveBeenCalled(); + expect(f.cache.get("file:///cache/offline")).toBe("cached"); + expect(f.manager.files.every((file) => file.tab)).toBe(true); +}); + it.each(["cached text", "", undefined])( "keeps the loading view for cache %j until the session is ready", (cached) => { diff --git a/tests/unit/remoteFilePreview.test.js b/tests/unit/remoteFilePreview.test.js index f82fe47f6c..ed4debcae2 100644 --- a/tests/unit/remoteFilePreview.test.js +++ b/tests/unit/remoteFilePreview.test.js @@ -19,10 +19,10 @@ describe("readRemoteFilePreview", () => { readRemoteFilePreview({ editorCache, transportCache, - encoding: "UTF-8", + encoding: "utf-16le", }), ).resolves.toEqual({ editorCacheExists: true, text: "unsaved" }); - expect(editorCache.readFile).toHaveBeenCalledWith("UTF-8"); + expect(editorCache.readFile).toHaveBeenCalledWith("utf-8"); expect(transportCache.exists).not.toHaveBeenCalled(); }); diff --git a/tests/unit/saveCommands.test.js b/tests/unit/saveCommands.test.js index 55bcdcb9a7..2576fdf490 100644 --- a/tests/unit/saveCommands.test.js +++ b/tests/unit/saveCommands.test.js @@ -13,8 +13,8 @@ function setup(files) { return { ...module, manager, toast, error }; } function tab(id, type = "docs") { - const file = { id, type, canSave: type === "docs", isUnsaved: true, remove: vi.fn(async () => true) }; - file.save = vi.fn(async () => { file.isUnsaved = false; return type === "editor" ? [undefined, undefined] : true; }); + const file = { id, type, canSave: type === "docs" || type === "editor", isUnsaved: true, remove: vi.fn(async () => true) }; + file.save = vi.fn(async () => { file.isUnsaved = false; return true; }); file.saveAs = vi.fn(async () => true); file.hasUnsavedChanges = () => file.isUnsaved; return file; @@ -39,6 +39,16 @@ it("does not report custom-tab success on cancellation or failure", async () => expect(f.toast).not.toHaveBeenCalled(); expect(f.error).toHaveBeenCalledOnce(); await f.default.save(true); expect(f.toast).toHaveBeenCalledOnce(); }); +it("does not save pending editors or report a cancelled editor save as successful", async () => { + const file = tab("pending", "editor"), f = setup([file]); + file.canSave = false; + await f.default.save(true); await f.default["save-as"](true); + expect(file.save).not.toHaveBeenCalled(); expect(file.saveAs).not.toHaveBeenCalled(); + file.canSave = true; + file.save.mockResolvedValue(false); file.saveAs.mockResolvedValue(false); + await f.default.save(true); await f.default["save-as"](true); + expect(f.toast).not.toHaveBeenCalled(); +}); it("saves sequentially without clearing flags and stops at cancellation or newer edits", async () => { const one = tab("one"), two = tab("two"), code = tab("code", "editor"); const f = setup([one, two, code]); @@ -67,11 +77,11 @@ it.each([false, true, "failure"])("save-and-close keeps edits after an unsuccess expect(file.isUnsaved).toBe(true); }); -it.each([0, 1, 2])("skips an unsavable dirty tab at position %s while saving eligible tabs", async position => { +it.each(["terminal", "editor"])("skips an unsavable dirty %s while saving eligible tabs", async type => { for (const command of ["save-all-changes", "close-all-tabs"]) { - const unsupported = tab("unsupported", "terminal"), one = tab("one"), two = tab("two", "editor"); - const files = [one, two]; - files.splice(position, 0, unsupported); + const unsupported = tab("unsupported", type), one = tab("one"), two = tab("two", "editor"); + unsupported.canSave = false; + const files = [unsupported, one, two]; const f = setup(files); expect(await f.default[command]()).toBe(false); expect(unsupported.save).not.toHaveBeenCalled();