diff --git a/package.json b/package.json index 23f5cc1..151e1d3 100644 --- a/package.json +++ b/package.json @@ -10,8 +10,9 @@ "version": "node version-bump.mjs && git add manifest.json versions.json", "lint": "eslint .", "test": "node --experimental-vm-modules node_modules/.bin/jest", - "test:release": "node --test scripts/__tests__/*.test.mjs", - "verify": "npm run lint && npm run build && npm test -- --runInBand && npm run test:release" + "test:release": "node --test scripts/__tests__/validate-release.test.mjs scripts/__tests__/workflow-actions.test.mjs", + "verify": "npm run lint && npm run build && npm test -- --runInBand && npm run test:release && npm run test:fixture", + "test:fixture": "node --test scripts/__tests__/prepare-large-vault-fixture.test.mjs" }, "keywords": [ "obsidian", diff --git a/scripts/__tests__/prepare-large-vault-fixture.test.mjs b/scripts/__tests__/prepare-large-vault-fixture.test.mjs new file mode 100644 index 0000000..e72f562 --- /dev/null +++ b/scripts/__tests__/prepare-large-vault-fixture.test.mjs @@ -0,0 +1,107 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { mkdtemp, mkdir, writeFile, rm, readdir, stat } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { + validateOptions, + resolveFixturePath, + createFixture, + removeFixture, +} from "../prepare-large-vault-fixture.mjs"; + +function parsed(values) { + return { values }; +} + +function expectFailure(fn, messagePart) { + assert.throws(fn, (error) => { + if (messagePart) assert.match(String(error.message ?? ""), new RegExp(messagePart)); + return true; + }); +} + +test("missing --vault fails", () => { + expectFailure(() => validateOptions(parsed({ files: "5000" })), "missing --vault"); +}); + +test("missing --files fails without --remove", () => { + expectFailure(() => validateOptions(parsed({ vault: "/tmp/x" })), "missing --files"); +}); + +test("--files outside 100-50000 fails", () => { + expectFailure(() => validateOptions(parsed({ vault: "/tmp/x", files: "99" })), "between 100 and 50000"); + expectFailure(() => validateOptions(parsed({ vault: "/tmp/x", files: "50001" })), "between 100 and 50000"); + expectFailure(() => validateOptions(parsed({ vault: "/tmp/x", files: "abc" })), "integer"); +}); + +test("cleanup refuses an unmarked directory", async () => { + const vault = await mkdtemp(path.join(tmpdir(), "se-fixture-")); + const fixture = resolveFixturePath(vault); + await mkdir(fixture); + await writeFile(path.join(fixture, "note.md"), "# keep"); + + await assert.rejects(() => removeFixture(vault), /refusing to remove/); + + // Unchanged. + await assert.ok((await stat(fixture)).isDirectory()); + await rm(vault, { recursive: true }); +}); + +test("cleanup never deletes the vault root", async () => { + const vault = await mkdtemp(path.join(tmpdir(), "se-fixture-")); + await writeFile(path.join(vault, "important.md"), "keep"); + + await assert.rejects(() => removeFixture(vault), /refusing to remove/); + await assert.equal((await readFileSafe(path.join(vault, "important.md"))), "keep"); + + await rm(vault, { recursive: true }); +}); + +test("a temp-directory fixture creates and removes exactly its own subtree", async () => { + const vault = await mkdtemp(path.join(tmpdir(), "se-fixture-")); + await writeFile(path.join(vault, "untouched.md"), "keep"); + + await createFixture(vault, 100); + + const fixture = resolveFixturePath(vault); + const entries = await readdir(vault); + assert.ok(entries.includes(".smart-explorer-large-vault-fixture")); + assert.ok(entries.includes("untouched.md")); + + const fixtureEntries = await readdir(fixture); + assert.ok(fixtureEntries.includes(".smart-explorer-fixture-marker")); + const folders = fixtureEntries.filter((entry) => entry.startsWith("folder-")); + assert.equal(folders.length, 100); + + await removeFixture(vault); + + const after = await readdir(vault); + assert.deepEqual(after, ["untouched.md"]); + + await rm(vault, { recursive: true }); +}); + +test("a 5000-file fixture rotates through every attachment format", async () => { + const vault = await mkdtemp(path.join(tmpdir(), "se-fixture-")); + try { + await createFixture(vault, 5000); + const fixture = resolveFixturePath(vault); + const extensions = new Set(); + for (const folder of await readdir(fixture)) { + if (!folder.startsWith("folder-")) continue; + for (const file of await readdir(path.join(fixture, folder))) { + extensions.add(path.extname(file).slice(1)); + } + } + + assert.deepEqual([...extensions].sort(), ["docx", "md", "pdf", "png"]); + } finally { + await rm(vault, { recursive: true }); + } +}); + +async function readFileSafe(file) { + const { readFile } = await import("node:fs/promises"); + return readFile(file, "utf8"); +} diff --git a/scripts/prepare-large-vault-fixture.mjs b/scripts/prepare-large-vault-fixture.mjs new file mode 100644 index 0000000..efbf7aa --- /dev/null +++ b/scripts/prepare-large-vault-fixture.mjs @@ -0,0 +1,133 @@ +#!/usr/bin/env node +/** + * Guarded synthetic-fixture generator for large-vault testing. + * + * May only create or delete `/.smart-explorer-large-vault-fixture`. + * A marker file is written before any file generation; removal refuses to + * run unless the directory name and marker both match, so an unmarked or + * mistyped path can never be deleted. + */ +import process from "node:process"; +import { parseArgs } from "node:util"; +import { mkdir, rm, writeFile, stat, readFile } from "node:fs/promises"; +import path from "node:path"; + +const FIXTURE_DIR_NAME = ".smart-explorer-large-vault-fixture"; +const MARKER_FILE_NAME = ".smart-explorer-fixture-marker"; +const MIN_FILES = 100; +const MAX_FILES = 50000; +const FOLDER_COUNT = 100; + +export function fail(message) { + throw new Error(message); +} + +function parseArguments(argv) { + try { + return parseArgs({ + args: argv, + options: { + vault: { type: "string" }, + files: { type: "string" }, + remove: { type: "boolean", default: false }, + }, + strict: true, + }); + } catch (error) { + fail(error instanceof Error ? error.message : String(error)); + } +} + +export function resolveFixturePath(vault) { + const vaultPath = path.resolve(vault); + return path.join(vaultPath, FIXTURE_DIR_NAME); +} + +export function validateOptions({ values }) { + if (!values.vault) fail("missing --vault "); + if (values.remove) return { remove: true, vault: values.vault, files: null }; + if (values.files === undefined) fail("missing --files <100-50000>"); + const files = Number(values.files); + if (!Number.isInteger(files) || files < MIN_FILES || files > MAX_FILES) { + fail(`--files must be an integer between ${MIN_FILES} and ${MAX_FILES}`); + } + return { remove: false, vault: values.vault, files }; +} + +async function isMarkedFixtureDir(dir) { + let dirStat; + try { + dirStat = await stat(dir); + } catch { + return false; + } + if (!dirStat.isDirectory()) return false; + if (path.basename(dir) !== FIXTURE_DIR_NAME) return false; + try { + const marker = await readFile(path.join(dir, MARKER_FILE_NAME), "utf8"); + return marker.trim() === "smart-explorer-large-vault-fixture"; + } catch { + return false; + } +} + +export async function createFixture(vault, files) { + const dir = resolveFixturePath(vault); + if (path.dirname(dir) === dir) fail("refusing to operate on a filesystem root"); + await mkdir(dir, { recursive: false }); + await writeFile(path.join(dir, MARKER_FILE_NAME), "smart-explorer-large-vault-fixture\n"); + + const perFolder = Math.ceil(files / FOLDER_COUNT); + const attachments = [ + { ext: "png", size: 0 }, + { ext: "pdf", size: 0 }, + { ext: "docx", size: 0 }, + ]; + let created = 0; + let attachmentIndex = 0; + for (let folderIndex = 0; folderIndex < FOLDER_COUNT && created < files; folderIndex++) { + const folder = path.join(dir, `folder-${String(folderIndex).padStart(3, "0")}`); + await mkdir(folder); + for (let fileIndex = 0; fileIndex < perFolder && created < files; fileIndex++) { + const isAttachment = fileIndex % 50 === 49 && fileIndex > 0; + if (isAttachment) { + const attachment = attachments[attachmentIndex % attachments.length]; + attachmentIndex++; + await writeFile(path.join(folder, `attachment-${fileIndex}.${attachment.ext}`), ""); + } else { + await writeFile(path.join(folder, `note-${fileIndex}.md`), "# Fixture note\n"); + } + created++; + } + } + console.log(`created ${created} fixture files in ${dir}`); +} + +export async function removeFixture(vault) { + const dir = resolveFixturePath(vault); + if (!(await isMarkedFixtureDir(dir))) { + fail("refusing to remove: not a marker-protected fixture directory"); + } + await rm(dir, { recursive: true, force: false }); + console.log(`removed ${dir}`); +} + +async function main() { + const parsed = parseArguments(process.argv.slice(2)); + const options = validateOptions(parsed); + if (options.remove) { + await removeFixture(options.vault); + } else { + await createFixture(options.vault, options.files); + } +} + +// Only run main when executed directly (tests import the helpers). +if (process.argv[1] && path.resolve(process.argv[1]) === path.resolve(new URL(import.meta.url).pathname)) { + try { + await main(); + } catch (error) { + console.error(`error: ${error instanceof Error ? error.message : String(error)}`); + process.exit(1); + } +} diff --git a/src/__tests__/main.test.ts b/src/__tests__/main.test.ts index 34d176a..2264916 100644 --- a/src/__tests__/main.test.ts +++ b/src/__tests__/main.test.ts @@ -145,4 +145,62 @@ describe("SmartExplorerPlugin", () => { }); expect(workspace.revealLeaf).toHaveBeenCalledWith(leftLeaf); }); + it("serializes immutable settings snapshots", async () => { + let resolveFirst!: () => void; + const firstWrite = new Promise((resolve) => { resolveFirst = resolve; }); + const plugin = new SmartExplorerPlugin({} as any, {} as any); + await plugin.loadSettings(); + plugin.saveData = jest.fn() + .mockReturnValueOnce(firstWrite) + .mockResolvedValueOnce(undefined); + + plugin.settings.hiddenExtensions = ["png"]; + const first = plugin.saveSettings(); + plugin.settings.hiddenExtensions = ["pdf"]; + const second = plugin.saveSettings(); + + expect(plugin.saveData).toHaveBeenCalledTimes(1); + resolveFirst(); + await first; + await second; + expect(plugin.saveData).toHaveBeenNthCalledWith(1, expect.objectContaining({ hiddenExtensions: ["png"] })); + expect(plugin.saveData).toHaveBeenNthCalledWith(2, expect.objectContaining({ hiddenExtensions: ["pdf"] })); + }); + + it("continues saving after one write rejects", async () => { + const plugin = new SmartExplorerPlugin({} as any, {} as any); + await plugin.loadSettings(); + plugin.saveData = jest.fn() + .mockRejectedValueOnce(new Error("disk full")) + .mockResolvedValueOnce(undefined); + + await expect(plugin.saveSettings()).rejects.toThrow("disk full"); + await expect(plugin.saveSettings()).resolves.toBeUndefined(); + expect(plugin.saveData).toHaveBeenCalledTimes(2); + }); + + it("notifies and recovers through saveSettingsWithNotice", async () => { + const plugin = new SmartExplorerPlugin({} as any, {} as any); + await plugin.loadSettings(); + plugin.saveData = jest.fn() + .mockRejectedValueOnce(new Error("disk full")) + .mockResolvedValueOnce(undefined); + + await expect(plugin.saveSettingsWithNotice("Could not save settings")).resolves.toBe(false); + await expect(plugin.saveSettingsWithNotice("Could not save settings")).resolves.toBe(true); + }); + + it("flushes pending writes", async () => { + let resolveWrite!: () => void; + const write = new Promise((resolve) => { resolveWrite = resolve; }); + const plugin = new SmartExplorerPlugin({} as any, {} as any); + await plugin.loadSettings(); + plugin.saveData = jest.fn().mockReturnValueOnce(write); + const saving = plugin.saveSettings(); + const flushed = plugin.flushSettings(); + resolveWrite(); + await Promise.all([saving, flushed]); + expect(plugin.saveData).toHaveBeenCalledTimes(1); + }); + }); diff --git a/src/explorer/FileIndex.ts b/src/explorer/FileIndex.ts index eb82e0a..e041eff 100644 --- a/src/explorer/FileIndex.ts +++ b/src/explorer/FileIndex.ts @@ -25,6 +25,8 @@ export function normalizeFileRecord( export class FileIndex { private app: App; private records: Map = new Map(); + // Maintained incrementally so tree renders never rescan every loaded file. + private folderPaths = new Set(); constructor(app: App) { this.app = app; @@ -33,13 +35,28 @@ export class FileIndex { build(): FileRecord[] { const files = this.app.vault.getFiles(); this.records.clear(); + this.folderPaths.clear(); for (const file of files) { const record = normalizeFileRecord(file, this.app.metadataCache); this.records.set(file.path, record); + this.addAncestorFolders(record.parentPath); + } + for (const file of this.app.vault.getAllLoadedFiles()) { + if (isFolder(file) && file.path !== "/") { + this.folderPaths.add(file.path); + } } return this.getAll(); } + private addAncestorFolders(parentPath: string): void { + if (!parentPath) return; + const parts = parentPath.split("/"); + for (let i = 1; i <= parts.length; i++) { + this.folderPaths.add(parts.slice(0, i).join("/")); + } + } + getAll(): FileRecord[] { return Array.from(this.records.values()); } @@ -51,12 +68,33 @@ export class FileIndex { addFile(file: TFile): void { const record = normalizeFileRecord(file, this.app.metadataCache); this.records.set(file.path, record); + this.addAncestorFolders(record.parentPath); + } + + addFolder(folderPath: string): void { + if (folderPath && folderPath !== "/") this.folderPaths.add(folderPath); + } + + setFolderPaths(paths: string[]): void { + this.folderPaths = new Set(paths); } removeFile(path: string): void { this.records.delete(path); } + // Obsidian emits delete only for the folder itself; every indexed child + // under it must go explicitly or it stays as a ghost row until reload. + removeFolder(folderPath: string): void { + const prefix = `${folderPath}/`; + for (const path of this.records.keys()) { + if (path.startsWith(prefix)) this.records.delete(path); + } + for (const path of this.folderPaths) { + if (path === folderPath || path.startsWith(prefix)) this.folderPaths.delete(path); + } + } + // Rewrite every record whose path lives at or under oldFolder to its new // location after a folder rename/move. Obsidian only emits a single rename // event for the folder itself (not for each child), so the index must be @@ -84,6 +122,20 @@ export class FileIndex { for (const [path, record] of rewritten) { this.records.set(path, record); } + // Collect rewrites first: mutating the Set during iteration would let + // freshly added entries (e.g. renaming "a" into "a/sub") match the old + // prefix and be rewritten twice. + const renamedFolders: string[] = []; + for (const path of this.folderPaths) { + if (path === oldFolder || path.startsWith(oldPrefix)) { + this.folderPaths.delete(path); + renamedFolders.push(path === oldFolder ? newFolder : `${newFolder}/${path.slice(oldPrefixLen)}`); + } + } + this.addAncestorFolders(newFolder); + for (const path of renamedFolders) { + this.folderPaths.add(path); + } } getExtensions(): string[] { @@ -95,9 +147,7 @@ export class FileIndex { } getFolderPaths(): string[] { - return this.app.vault.getAllLoadedFiles() - .filter((file): file is TFolder => isFolder(file) && file.path !== "/") - .map((folder) => folder.path) + return Array.from(this.folderPaths) .sort((a, b) => a.localeCompare(b, undefined, { numeric: true })); } diff --git a/src/explorer/SmartExplorerView.ts b/src/explorer/SmartExplorerView.ts index 264f91e..c42d022 100644 --- a/src/explorer/SmartExplorerView.ts +++ b/src/explorer/SmartExplorerView.ts @@ -213,7 +213,8 @@ export class SmartExplorerView extends ItemView { if (this.saveOrderTimeout) { window.clearTimeout(this.saveOrderTimeout); this.saveOrderTimeout = null; - void this.plugin.saveSettings(); + await this.plugin.saveSettingsWithNotice("Could not save manual order"); + await this.plugin.flushSettings(); } } @@ -221,6 +222,9 @@ export class SmartExplorerView extends ItemView { const events = this.plugin.app.vault; this.registerEvent(events.on("create", (file) => { + if (file instanceof TFolder) { + this.fileIndex.addFolder(file.path); + } if (file instanceof TFile) { this.fileIndex.addFile(file); // New files are intentionally NOT appended to manualOrder here: @@ -239,9 +243,13 @@ export class SmartExplorerView extends ItemView { this.selectedPath = null; } } else if (file instanceof TFolder) { + if (this.selectedPath?.startsWith(`${file.path}/`)) { + this.selectedPath = null; + } if (this.selectedFolderPath === file.path || this.selectedFolderPath?.startsWith(`${file.path}/`)) { this.selectedFolderPath = null; } + this.fileIndex.removeFolder(file.path); this.collapseFolderPath(file.path); } this.manualOrderNeedsReconcile = true; @@ -271,6 +279,14 @@ export class SmartExplorerView extends ItemView { this.scheduleRebuild(); } })); + + // Keep the selection highlight in sync when files open elsewhere, but + // never auto-reveal: switching tabs must not scroll or expand folders. + this.registerEvent(this.plugin.app.workspace.on("file-open", (file) => { + this.selectedPath = file instanceof TFile ? file.path : null; + this.selectedFolderPath = null; + this.highlightSelected(); + })); } private scheduleRebuild() { @@ -303,9 +319,7 @@ export class SmartExplorerView extends ItemView { this.viewModeBtn.addEventListener("click", () => { this.viewMode = this.viewMode === "tree" ? "list" : "tree"; this.plugin.settings.lastViewMode = this.viewMode; - void this.plugin.saveSettings().catch((error) => { - new Notice(`Could not save view mode: ${error instanceof Error ? error.message : String(error)}`); - }); + void this.plugin.saveSettingsWithNotice("Could not save view mode"); this.renderList(); }); this.createSelect(row1, COMPACT_SORT_OPTIONS, "smart-explorer-sort", "Sort order", (v) => { @@ -1429,6 +1443,9 @@ export class SmartExplorerView extends ItemView { if (this.selectedFolderPath) { this.selectedFolderPath = renameNestedPath(this.selectedFolderPath, oldPath, newPath); } + if (this.selectedPath) { + this.selectedPath = renameNestedPath(this.selectedPath, oldPath, newPath); + } } private updateManualOrderAfterRename(oldPath: string, newPath: string) { @@ -1465,43 +1482,58 @@ export class SmartExplorerView extends ItemView { } private async openFileInLeaf(path: string, leafType: "tab" | "right" | "window") { - const file = this.app.vault.getAbstractFileByPath(path); - if (!(file instanceof TFile)) return; - try { + await this.runAction(async () => { + const file = this.app.vault.getAbstractFileByPath(path); + if (!(file instanceof TFile)) return; const leaf = leafType === "right" ? this.app.workspace.getLeaf("split", "vertical") : leafType === "window" ? this.app.workspace.openPopoutLeaf() : this.app.workspace.getLeaf("tab"); await leaf.openFile(file); - } catch (e) { - new Notice(`Could not open file: ${e instanceof Error ? e.message : String(e)}`); + }, "Could not open file"); + } + + // Single error boundary for user-triggered async actions: failures turn + // into a Notice instead of an unhandled rejection. + private async runAction(action: () => Promise, failure: string): Promise { + try { + await action(); + } catch (error) { + new Notice(`${failure}: ${error instanceof Error ? error.message : String(error)}`); } } private copyPath(path: string) { - void navigator.clipboard.writeText(path); + void this.runAction(async () => { + await navigator.clipboard.writeText(path); + new Notice("Copied path."); + }, "Could not copy path"); } private async openInDefaultApp(path: string) { - const shell = getElectronShell(); - const absolutePath = this.getAbsoluteVaultPath(path); - if (!shell || !absolutePath) { - new Notice("Open in default app is only available for local desktop vaults."); - return; - } - const error = await shell.openPath(absolutePath); - if (error) new Notice(`Could not open in default app: ${error}`); + await this.runAction(async () => { + const shell = getElectronShell(); + const absolutePath = this.getAbsoluteVaultPath(path); + if (!shell || !absolutePath) { + new Notice("Open in default app is only available for local desktop vaults."); + return; + } + const error = await shell.openPath(absolutePath); + if (error) new Notice(`Could not open in default app: ${error}`); + }, "Could not open in default app"); } - private revealInFinder(path: string) { - const shell = getElectronShell(); - const absolutePath = this.getAbsoluteVaultPath(path); - if (!shell || !absolutePath) { - new Notice("Reveal in finder is only available for local desktop vaults."); - return; - } - shell.showItemInFolder(absolutePath); + private async revealInFinder(path: string) { + await this.runAction(async () => { + const shell = getElectronShell(); + const absolutePath = this.getAbsoluteVaultPath(path); + if (!shell || !absolutePath) { + new Notice("Reveal in finder is only available for local desktop vaults."); + return; + } + shell.showItemInFolder(absolutePath); + }, "Could not reveal in finder"); } private getAbsoluteVaultPath(path: string): string | null { @@ -1693,7 +1725,7 @@ export class SmartExplorerView extends ItemView { this.plugin.settings.manualOrder = this.plugin.settings.manualOrder.filter( (p) => allPaths.has(p), ); - void this.plugin.saveSettings(); + void this.plugin.saveSettingsWithNotice("Could not save manual order"); }, 500); } @@ -1788,7 +1820,9 @@ export class SmartExplorerView extends ItemView { }), ); menu.addItem((item) => - item.setTitle("Reveal in finder").setIcon("folder-search").onClick(() => this.revealInFinder(record.path)), + item.setTitle("Reveal in finder").setIcon("folder-search").onClick(() => { + void this.revealInFinder(record.path); + }), ); menu.addSeparator(); menu.addItem((item) => @@ -1830,7 +1864,9 @@ export class SmartExplorerView extends ItemView { item.setTitle("Copy path").setIcon("copy").onClick(() => this.copyPath(folderPath)), ); menu.addItem((item) => - item.setTitle("Reveal in finder").setIcon("folder-search").onClick(() => this.revealInFinder(folderPath)), + item.setTitle("Reveal in finder").setIcon("folder-search").onClick(() => { + void this.revealInFinder(folderPath); + }), ); menu.addSeparator(); menu.addItem((item) => @@ -1906,14 +1942,15 @@ export class SmartExplorerView extends ItemView { } private async openFile(path: string) { - const file = this.app.vault.getAbstractFileByPath(path); - if (file instanceof TFile) { + await this.runAction(async () => { + const file = this.app.vault.getAbstractFileByPath(path); + if (!(file instanceof TFile)) return; let leaf = this.app.workspace.getLeaf(false); if (leaf.view?.getViewType() === SMART_EXPLORER_VIEW_TYPE) { leaf = this.app.workspace.getLeaf("tab"); } await leaf.openFile(file); - } + }, "Could not open file"); } } diff --git a/src/explorer/__tests__/FileIndex.test.ts b/src/explorer/__tests__/FileIndex.test.ts index d763a5a..807bb44 100644 --- a/src/explorer/__tests__/FileIndex.test.ts +++ b/src/explorer/__tests__/FileIndex.test.ts @@ -178,3 +178,38 @@ describe("FileIndex.renameFolder", () => { expect(index.get("new-backup/file.md")).toBeUndefined(); }); }); + +describe("FileIndex.removeFolder and incremental folder paths", () => { + it("removes every indexed child for a deleted folder", () => { + const index = makeIndex(); + for (const path of ["keep.md", "gone/a.md", "gone/nested/b.md"]) { + index.addFile(mockTFile({ path }) as any); + } + + index.removeFolder("gone"); + + expect(index.getAll().map((record) => record.path)).toEqual(["keep.md"]); + }); + + it("rewrites folder paths on rename and clears them on folder delete", () => { + const index = makeIndex(); + index.setFolderPaths(["a", "a/b", "keep"]); + expect(index.getFolderPaths()).toEqual(["a", "a/b", "keep"]); + + index.renameFolder("a", "renamed"); + expect(index.getFolderPaths()).toEqual(["keep", "renamed", "renamed/b"]); + + index.removeFolder("renamed"); + expect(index.getFolderPaths()).toEqual(["keep"]); + }); + + it("adds and removes single folder paths incrementally", () => { + const index = makeIndex(); + index.addFolder("new"); + index.addFolder("new/nested"); + expect(index.getFolderPaths()).toEqual(["new", "new/nested"]); + + index.removeFolder("new"); + expect(index.getFolderPaths()).toEqual([]); + }); +}); diff --git a/src/explorer/__tests__/SmartExplorerView.dom.test.ts b/src/explorer/__tests__/SmartExplorerView.dom.test.ts index 97efb42..6f7ac0d 100644 --- a/src/explorer/__tests__/SmartExplorerView.dom.test.ts +++ b/src/explorer/__tests__/SmartExplorerView.dom.test.ts @@ -68,6 +68,8 @@ function makeView() { manualOrder: [], }, saveSettings: jest.fn().mockResolvedValue(undefined), + saveSettingsWithNotice: jest.fn().mockResolvedValue(true), + flushSettings: jest.fn().mockResolvedValue(undefined), }; const view = new SmartExplorerView({ app } as never, plugin as never) as any; const container = view.containerEl.children[1] as HTMLElement; @@ -101,7 +103,8 @@ describe("SmartExplorerView DOM foundation", () => { expect(first.view.viewMode).toBe("list"); expect(second.viewMode).toBe("tree"); expect(first.view.plugin.settings.lastViewMode).toBe("list"); - expect(first.view.plugin.saveSettings).toHaveBeenCalledTimes(1); + expect(first.view.plugin.saveSettingsWithNotice).toHaveBeenCalledTimes(1); + expect(first.view.plugin.saveSettings).not.toHaveBeenCalled(); }); it("applies current Obsidian element info fields used by the explorer", () => { @@ -889,3 +892,109 @@ describe("SmartExplorerView windowed list rendering", () => { } }); }); + +describe("SmartExplorerView close persistence", () => { + it("awaits a pending manual-order save before close completes", async () => { + const view = Object.create(SmartExplorerView.prototype) as any; + view.searchRenderScheduler = { cancel: jest.fn() }; + view.tooltipEl = null; + view.virtualList = null; + view.dragSortManager = null; + view.listContainer = null; + view.rebuildTimeout = null; + view.saveOrderTimeout = window.setTimeout(() => {}, 10000); + const order: string[] = []; + view.plugin = { + saveSettingsWithNotice: jest.fn(async () => { + order.push("save"); + }), + flushSettings: jest.fn(async () => { + order.push("flush"); + }), + }; + + await view.onClose(); + + expect(order).toEqual(["save", "flush"]); + expect(view.saveOrderTimeout).toBeNull(); + }); +}); + +describe("SmartExplorerView workspace selection sync", () => { + it("syncs selection on file-open without scrolling or expanding folders", () => { + const { view, container } = makeView(); + document.body.appendChild(container); + try { + view.fileIndex = { + getAll: () => [makeRecord("notes/one.md", "md"), makeRecord("notes/two.md", "md")], + getFolderPaths: () => ["notes"], + }; + view.viewMode = "list"; + view.renderList(); + const list = container.querySelector(".smart-explorer-list")!; + list.scrollTop = 120; + + let fileOpenHandler: ((file: any) => void) | null = null; + view.plugin.app = { + vault: { on: jest.fn() }, + workspace: { on: (_name: string, cb: (file: { path: string } | null) => void) => { fileOpenHandler = cb; } }, + }; + view.registerVaultEvents(); + + const { TFile } = jest.requireMock("obsidian") as { TFile: new () => any }; + const openedFile = new TFile(); + openedFile.path = "notes/two.md"; + fileOpenHandler!(openedFile); + const row = container.querySelector('.smart-explorer-row[data-path="notes/two.md"]')!; + expect(view.selectedPath).toBe("notes/two.md"); + expect(row.classList.contains("is-selected")).toBe(true); + expect(row.getAttribute("aria-selected")).toBe("true"); + expect(list.scrollTop).toBe(120); + expect(view.treeExpandedPaths.size).toBe(0); + + fileOpenHandler!(null); + expect(view.selectedPath).toBeNull(); + expect(row.classList.contains("is-selected")).toBe(false); + expect(row.getAttribute("aria-selected")).toBe("false"); + expect(list.scrollTop).toBe(120); + } finally { + document.body.removeChild(container); + } + }); + + it("purges deleted folder subtrees from the index and manual-order reconcile flag", () => { + const { view } = makeView(); + const removeFolder = jest.fn(); + const addFolder = jest.fn(); + const scheduleRebuild = jest.fn(); + view.scheduleRebuild = scheduleRebuild; + view.collapseFolderPath = jest.fn(); + view.fileIndex = { removeFolder, addFolder }; + let vaultHandlers: Record void> = {}; + view.plugin.app = { + vault: { on: (name: string, cb: (file: unknown, oldPath?: string) => void) => { vaultHandlers[name] = cb; } }, + workspace: { on: jest.fn() }, + }; + view.registerVaultEvents(); + + const folder = { constructor: Object }; + Object.setPrototypeOf(folder, { [Symbol.toStringTag]: "TFolder" }); + vaultHandlers.delete!(folder); + expect(removeFolder).not.toHaveBeenCalled(); + + // Real TFolder instances flow through the instanceof branch. + const { TFolder } = jest.requireMock("obsidian") as { TFolder: new () => unknown }; + const realFolder = new (TFolder as new () => any)(); + realFolder.path = "gone"; + vaultHandlers.delete!(realFolder); + expect(removeFolder).toHaveBeenCalledWith("gone"); + expect(view.manualOrderNeedsReconcile).toBe(true); + + const newFolder = new (TFolder as new () => any)(); + newFolder.path = "created"; + view.manualOrderNeedsReconcile = false; + vaultHandlers.create!(newFolder); + expect(addFolder).toHaveBeenCalledWith("created"); + expect(view.manualOrderNeedsReconcile).toBe(true); + }); +}); diff --git a/src/explorer/__tests__/SmartExplorerView.integration.test.ts b/src/explorer/__tests__/SmartExplorerView.integration.test.ts new file mode 100644 index 0000000..fa63068 --- /dev/null +++ b/src/explorer/__tests__/SmartExplorerView.integration.test.ts @@ -0,0 +1,305 @@ +/** @jest-environment jsdom */ + +jest.mock( + "obsidian", + () => { + const notices: string[] = []; + return { + Plugin: class { + app: unknown; + manifest: unknown; + constructor(app: unknown, manifest: unknown) { + this.app = app; + this.manifest = manifest; + } + async loadData() { + return null; + } + async saveData(_data: unknown) {} + registerView() {} + addRibbonIcon() {} + addCommand() {} + addSettingTab() {} + }, + ItemView: class { + app: unknown; + containerEl: HTMLElement; + + constructor(leaf: { app: unknown }) { + this.app = leaf.app; + this.containerEl = document.createElement("div"); + this.containerEl.append(document.createElement("div"), document.createElement("div")); + } + + registerEvent() {} + }, + Menu: class {}, + PluginSettingTab: class {}, + Modal: class {}, + Notice: class { + constructor(message: string) { + notices.push(message); + } + }, + Platform: { isMobile: false }, + Setting: class {}, + setIcon: jest.fn(), + TFile: class TFile {}, + TFolder: class TFolder {}, + WorkspaceLeaf: class {}, + __notices: notices, + }; + }, + { virtual: true }, +); + +import "../../test-utils/obsidianDom"; +import { TFile, TFolder } from "obsidian"; + +(globalThis as typeof globalThis & { activeWindow: Window }).activeWindow = window; +import SmartExplorerPlugin from "../../main"; +import { normalizeSettings } from "../../settings/settings-normalization"; +import { SmartExplorerView } from "../SmartExplorerView"; + +function makeTFile(path: string): TFile & { path: string } { + const file = new TFile() as TFile & { path: string }; + const name = path.split("/").pop() ?? path; + const dot = name.lastIndexOf("."); + file.path = path; + (file as any).basename = dot > 0 ? name.slice(0, dot) : name; + (file as any).extension = dot > 0 ? name.slice(dot + 1) : ""; + (file as any).parent = path.includes("/") ? { path: path.slice(0, path.lastIndexOf("/")) } : null; + (file as any).stat = { size: 10, ctime: 1, mtime: 1 }; + return file; +} + +function makeTFolder(path: string): TFolder & { path: string } { + const folder = new TFolder() as TFolder & { path: string }; + folder.path = path; + (folder as any).children = []; + return folder; +} + +function makeHarness() { + const files = new Map>(); + const folders = new Set(); + const vaultHandlers: Record void> = {}; + const workspaceHandlers: Record void> = {}; + + const workspace: any = { + getActiveFile: () => null, + getLeaf: () => ({ view: null, openFile: async () => {} }), + getLeavesOfType: () => [], + on: (name: string, cb: (file: unknown) => void) => { + workspaceHandlers[name] = cb; + }, + }; + const app = { + vault: { + getFiles: () => Array.from(files.values()), + getAllLoadedFiles: () => [ + ...Array.from(files.values()), + ...Array.from(folders).map((path) => makeTFolder(path)), + ], + getAbstractFileByPath: (path: string) => files.get(path) ?? null, + on: (name: string, cb: (file: unknown, oldPath?: string) => void) => { + vaultHandlers[name] = cb; + }, + }, + metadataCache: null, + workspace, + }; + + const plugin = new SmartExplorerPlugin(app as never, { id: "test" } as never); + plugin.settings = normalizeSettings(null); + plugin.saveData = jest.fn(async () => {}); + + const view = new SmartExplorerView({ app } as never, plugin as never) as any; + workspace.getLeavesOfType = () => [{ view }]; + const container = view.containerEl.children[1] as HTMLElement; + document.body.appendChild(container); + view.renderShell(container); + view.fileIndex.build(); + view.renderList(); + view.registerVaultEvents(); + + const notices = (jest.requireMock("obsidian") as { __notices: string[] }).__notices; + + return { + view, plugin, container, files, folders, notices, workspace, workspaceHandlers, vaultHandlers, + add(path: string) { + const file = makeTFile(path); + files.set(path, file); + folders.add(path.includes("/") ? path.slice(0, path.lastIndexOf("/")) : ""); + return file; + }, + remove(path: string) { + files.delete(path); + }, + }; +} + +describe("SmartExplorerView lifecycle integration", () => { + beforeEach(() => { + jest.useFakeTimers(); + (jest.requireMock("obsidian") as { __notices: string[] }).__notices.length = 0; + }); + + afterEach(() => { + jest.useRealTimers(); + document.body.innerHTML = ""; + }); + + it("create file grows the index and refreshes the debounced DOM count", () => { + const harness = makeHarness(); + harness.view.viewMode = "list"; + harness.add("existing.md"); + harness.view.fileIndex.build(); + harness.view.renderList(); + const countBefore = harness.container.querySelector(".smart-explorer-file-count")!.textContent; + + const file = harness.add("notes/created.md"); + harness.vaultHandlers.create!(file); + jest.advanceTimersByTime(300); + + expect(countBefore).toBe("1 file"); + expect(harness.container.querySelector(".smart-explorer-file-count")!.textContent).toContain("2 files"); + expect(harness.container.querySelector('[data-path="notes/created.md"]')).not.toBeNull(); + }); + + it("delete folder removes every child from the index and the DOM", () => { + const harness = makeHarness(); + for (const path of ["keep.md", "gone/a.md", "gone/nested/b.md"]) { + const file = harness.add(path); + harness.vaultHandlers.create!(file); + jest.advanceTimersByTime(300); + } + expect(harness.view.fileIndex.getAll()).toHaveLength(3); + harness.view.selectedPath = "gone/nested/b.md"; + + harness.remove("gone/a.md"); + harness.remove("gone/nested/b.md"); + harness.vaultHandlers.delete!(makeTFolder("gone")); + jest.advanceTimersByTime(300); + + expect(harness.view.fileIndex.getAll().map((record: any) => record.path)).toEqual(["keep.md"]); + expect(harness.container.querySelector('[data-path="gone/a.md"]')).toBeNull(); + expect(harness.container.querySelector('[data-path="gone/nested/b.md"]')).toBeNull(); + expect(harness.view.selectedPath).toBeNull(); + }); + + it("rename folder rewrites child paths and manual order", () => { + const harness = makeHarness(); + harness.plugin.settings.manualOrder = ["keep.md", "old/a.md", "old/nested/b.md"]; + for (const path of ["keep.md", "old/a.md", "old/nested/b.md"]) { + harness.add(path); + } + harness.view.fileIndex.build(); + harness.view.selectedPath = "old/nested/b.md"; + harness.vaultHandlers.rename!(makeTFolder("new"), "old"); + jest.advanceTimersByTime(300); + + expect(harness.plugin.settings.manualOrder).toEqual(["keep.md", "new/a.md", "new/nested/b.md"]); + expect(harness.view.fileIndex.get("new/nested/b.md")).toBeDefined(); + expect(harness.view.selectedPath).toBe("new/nested/b.md"); + }); + + it("coalesces an event burst into one render", () => { + const harness = makeHarness(); + const renderSpy = jest.spyOn(harness.view, "renderList"); + + for (let index = 0; index < 5; index++) { + harness.vaultHandlers.create!(harness.add(`burst-${index}.md`)); + } + jest.advanceTimersByTime(300); + expect(renderSpy).toHaveBeenCalledTimes(1); + }); + + it("refreshes an open view after a hidden-extension settings change", () => { + const harness = makeHarness(); + harness.add("a.md"); + harness.add("b.pdf"); + harness.view.fileIndex.build(); + harness.view.renderList(); + expect(harness.container.querySelectorAll(".smart-explorer-row")).toHaveLength(2); + + harness.plugin.settings.hiddenExtensions = ["pdf"]; + harness.plugin.refreshExplorerViews(); + + expect(harness.container.querySelectorAll(".smart-explorer-row")).toHaveLength(1); + expect(harness.container.querySelector('[data-path="b.pdf"]')).toBeNull(); + }); + + it("shows a Notice containing the error when opening a file fails", async () => { + const harness = makeHarness(); + const file = harness.add("broken.md"); + harness.view.app = { + ...harness.view.app, + vault: { + ...harness.view.app.vault, + getAbstractFileByPath: () => file, + }, + workspace: { + ...harness.workspace, + getLeaf: () => ({ + view: null, + openFile: async () => { + throw new Error("leaf exploded"); + }, + }), + }, + }; + + await harness.view.openFile("broken.md"); + + expect(harness.notices.some((message) => message.includes("leaf exploded"))).toBe(true); + }); + + it("reports Electron shell failures instead of rejecting or throwing", async () => { + const harness = makeHarness(); + (harness.view.app.vault as any).adapter = { getBasePath: () => "/vault" }; + const originalRequire = (window as Window & { require?: unknown }).require; + (window as Window & { require?: unknown }).require = () => ({ + shell: { + openPath: async () => { throw new Error("open exploded"); }, + showItemInFolder: () => { throw new Error("reveal exploded"); }, + }, + }); + try { + await expect(harness.view.openInDefaultApp("broken.pdf")).resolves.toBeUndefined(); + await expect(harness.view.revealInFinder("broken.pdf")).resolves.toBeUndefined(); + + expect(harness.notices).toEqual(expect.arrayContaining([ + expect.stringContaining("open exploded"), + expect.stringContaining("reveal exploded"), + ])); + } finally { + if (originalRequire === undefined) { + Reflect.deleteProperty(window, "require"); + } else { + (window as Window & { require?: unknown }).require = originalRequire; + } + } + }); + + it("resolves a pending manual-order save before close completes", async () => { + const harness = makeHarness(); + harness.view.plugin.settings.manualOrder = ["a.md"]; + harness.view.scheduleSaveOrder(); + expect(harness.view.saveOrderTimeout).not.toBeNull(); + + let finishSave!: () => void; + harness.plugin.saveData = jest.fn(() => new Promise((resolve) => { finishSave = resolve; })); + + let closed = false; + const closing = harness.view.onClose().then(() => { closed = true; }); + await Promise.resolve(); + await Promise.resolve(); + expect(closed).toBe(false); + expect(harness.plugin.saveData).toHaveBeenCalledTimes(1); + + finishSave(); + await closing; + expect(closed).toBe(true); + }); +}); diff --git a/src/explorer/__tests__/SmartExplorerView.test.ts b/src/explorer/__tests__/SmartExplorerView.test.ts index 0f386c7..f55e031 100644 --- a/src/explorer/__tests__/SmartExplorerView.test.ts +++ b/src/explorer/__tests__/SmartExplorerView.test.ts @@ -189,3 +189,4 @@ describe("SmartExplorerView reveal state", () => { expect(view.rebuildView).toHaveBeenCalledTimes(1); }); }); + diff --git a/src/main.ts b/src/main.ts index f6b3c07..a35c5b2 100644 --- a/src/main.ts +++ b/src/main.ts @@ -1,4 +1,4 @@ -import { Plugin } from "obsidian"; +import { Notice, Plugin } from "obsidian"; import { SMART_EXPLORER_VIEW_TYPE } from "./constants"; import { SmartExplorerView } from "./explorer/SmartExplorerView"; import { SmartExplorerSettingTab } from "./settings/settings-tab"; @@ -56,8 +56,36 @@ export default class SmartExplorerPlugin extends Plugin { this.settings = normalizeSettings(await this.loadData()); } - async saveSettings() { - await this.saveData(this.settings); + // Writes are serialized through an immutable snapshot so a pending slow + // write can never be overtaken by a newer one, and a rejected write + // notifies its caller without poisoning the queue for the next save. + private settingsSaveQueue: Promise | null = null; + + saveSettings(): Promise { + const snapshot: SmartExplorerSettings = { + ...this.settings, + hiddenExtensions: [...this.settings.hiddenExtensions], + manualOrder: [...this.settings.manualOrder], + }; + const operation = this.settingsSaveQueue === null + ? this.saveData(snapshot) + : this.settingsSaveQueue.then(() => this.saveData(snapshot)); + this.settingsSaveQueue = operation.catch(() => undefined); + return operation; + } + + async flushSettings(): Promise { + await this.settingsSaveQueue; + } + + async saveSettingsWithNotice(failure: string): Promise { + try { + await this.saveSettings(); + return true; + } catch (error) { + new Notice(`${failure}: ${error instanceof Error ? error.message : String(error)}`); + return false; + } } refreshExplorerViews() { diff --git a/src/settings/__tests__/settings-tab.test.ts b/src/settings/__tests__/settings-tab.test.ts index 635d15d..a9d2266 100644 --- a/src/settings/__tests__/settings-tab.test.ts +++ b/src/settings/__tests__/settings-tab.test.ts @@ -72,7 +72,7 @@ describe("SmartExplorerSettingTab", () => { let finishSave!: () => void; const plugin = { settings: { hiddenExtensions: [], manualOrder: [] }, - saveSettings: jest.fn(() => new Promise((resolve) => { finishSave = resolve; })), + saveSettingsWithNotice: jest.fn(() => new Promise((resolve) => { finishSave = () => resolve(true); })), refreshExplorerViews: jest.fn(), resetExplorerManualOrderViews: jest.fn(), }; @@ -91,7 +91,7 @@ describe("SmartExplorerSettingTab", () => { await Promise.resolve(); await Promise.resolve(); - expect(plugin.saveSettings).toHaveBeenCalledTimes(1); + expect(plugin.saveSettingsWithNotice).toHaveBeenCalledTimes(1); expect(plugin.refreshExplorerViews).toHaveBeenCalledTimes(1); expect(plugin.resetExplorerManualOrderViews).not.toHaveBeenCalled(); jest.useRealTimers(); @@ -102,7 +102,7 @@ describe("SmartExplorerSettingTab", () => { const onChange = jest.fn(); const plugin = { settings: { hiddenExtensions: [], manualOrder: [] }, - saveSettings: jest.fn().mockRejectedValue(new Error("disk full")), + saveSettingsWithNotice: jest.fn().mockResolvedValue(false), refreshExplorerViews: jest.fn(), resetExplorerManualOrderViews: jest.fn(), }; @@ -127,7 +127,7 @@ describe("SmartExplorerSettingTab", () => { it("does not refresh or show reset success when saving fails", async () => { const plugin = { settings: { hiddenExtensions: [], manualOrder: ["a"] }, - saveSettings: jest.fn().mockRejectedValue(new Error("disk full")), + saveSettingsWithNotice: jest.fn().mockResolvedValue(false), refreshExplorerViews: jest.fn(), resetExplorerManualOrderViews: jest.fn(), }; @@ -149,7 +149,7 @@ describe("SmartExplorerSettingTab", () => { let finishSave!: () => void; const plugin = { settings: { hiddenExtensions: [], manualOrder: ["a"] }, - saveSettings: jest.fn(() => new Promise((resolve) => { finishSave = resolve; })), + saveSettingsWithNotice: jest.fn(() => new Promise((resolve) => { finishSave = () => resolve(true); })), refreshExplorerViews: jest.fn(), resetExplorerManualOrderViews: jest.fn(), }; @@ -163,10 +163,43 @@ describe("SmartExplorerSettingTab", () => { await Promise.resolve(); await Promise.resolve(); - expect(plugin.saveSettings).toHaveBeenCalledTimes(1); + expect(plugin.saveSettingsWithNotice).toHaveBeenCalledTimes(1); expect(plugin.resetExplorerManualOrderViews).toHaveBeenCalledTimes(1); expect(plugin.refreshExplorerViews).not.toHaveBeenCalled(); expect(button.setButtonText).toHaveBeenCalledWith("Done!"); jest.useRealTimers(); }); + + it("still saves a later change after a failed hidden-extension save", async () => { + jest.useFakeTimers(); + const onChange = jest.fn(); + const plugin = { + settings: { hiddenExtensions: [], manualOrder: [] }, + saveSettingsWithNotice: jest.fn() + .mockResolvedValueOnce(false) + .mockResolvedValueOnce(true), + refreshExplorerViews: jest.fn(), + resetExplorerManualOrderViews: jest.fn(), + }; + const tab = new SmartExplorerSettingTab({} as any, plugin as any); + const setting = { addText: (build: any) => build({ + setPlaceholder() { return this; }, + setValue() { return this; }, + onChange(callback: (value: string) => void) { onChange.mockImplementation(callback); return this; }, + }) }; + (tab as any).addHiddenExtensionsControl(setting); + + onChange("pdf"); + jest.advanceTimersByTime(500); + await Promise.resolve(); + await Promise.resolve(); + onChange("png"); + jest.advanceTimersByTime(500); + await Promise.resolve(); + await Promise.resolve(); + + expect(plugin.saveSettingsWithNotice).toHaveBeenCalledTimes(2); + expect(plugin.refreshExplorerViews).toHaveBeenCalledTimes(1); + jest.useRealTimers(); + }); }); diff --git a/src/settings/settings-tab.ts b/src/settings/settings-tab.ts index 58d9a94..dac8537 100644 --- a/src/settings/settings-tab.ts +++ b/src/settings/settings-tab.ts @@ -64,7 +64,7 @@ export class SmartExplorerSettingTab extends PluginSettingTab { dd.setValue(this.plugin.settings.defaultSort) .onChange(async (v) => { this.plugin.settings.defaultSort = v as SortMode; - await this.plugin.saveSettings(); + await this.plugin.saveSettingsWithNotice("Could not save settings"); }); }); @@ -78,7 +78,7 @@ export class SmartExplorerSettingTab extends PluginSettingTab { dd.setValue(this.plugin.settings.defaultGroup) .onChange(async (v) => { this.plugin.settings.defaultGroup = v as GroupMode; - await this.plugin.saveSettings(); + await this.plugin.saveSettingsWithNotice("Could not save settings"); }); }); @@ -106,9 +106,10 @@ export class SmartExplorerSettingTab extends PluginSettingTab { ...this.plugin.settings, hiddenExtensions: v.split(","), }).hiddenExtensions; - void this.plugin.saveSettings() - .then(() => this.plugin.refreshExplorerViews()) - .catch(() => {}); + void this.plugin.saveSettingsWithNotice("Could not save settings") + .then((saved) => { + if (saved) void this.plugin.refreshExplorerViews(); + }); }, 500); }); }); @@ -118,11 +119,12 @@ export class SmartExplorerSettingTab extends PluginSettingTab { setting.addButton((btn) => { btn.setButtonText("Reset").onClick(() => { this.plugin.settings.manualOrder = []; - void this.plugin.saveSettings().then(() => { + void this.plugin.saveSettingsWithNotice("Could not save settings").then((saved) => { + if (!saved) return; this.plugin.resetExplorerManualOrderViews(); btn.setButtonText("Done!"); window.setTimeout(() => { btn.setButtonText("Reset"); }, 1500); - }).catch(() => {}); + }); }); }); }