From 6a1a044566c428148aaba734f88e9e2f8663007c Mon Sep 17 00:00:00 2001 From: Olivier Date: Mon, 7 Sep 2026 20:03:36 -0400 Subject: [PATCH] fix: folder renames no longer destroy bookmarks, plus data.json save race - onRename remapped paths by exact equality only, so files inside a moved folder kept stale paths and the view then permanently deleted their bookmarks on next click. Add pure remapRenamedPath() (path-utils.ts, obsidian-free so it is unit-testable) and remap recentFiles + the whole bookmark tree, folder moves included. - saveSettings/saveWaypointData each did loadData -> mutate one key -> saveData, so two concurrent saves carried a stale snapshot of the other key and one silently reverted the other. Both now delegate to persistAll(), which writes both keys from memory through a promise-chain mutex. - data.json is read once in onload instead of twice; period sub-objects are deep-merged so old configs pick up new fields. - detectPeriodType now derives from the configured nameFormat with strict moment parsing instead of hardcoded regexes, so custom formats work. - Merge openPeriodNoteInLeaf into openPeriodNote(period, date, leaf?); stop double-appending .md to templateFile; type the caught error as unknown. - hasNoteForDate is synchronous and O(1) over a maintained basename Set; delete dead getNotesForDate. - Implement the previously dead omittedTags filter and the no-op updateOn: 'file-edit' mode (vault modify event). --- src/main.ts | 455 ++++++++++++++++++++++++---------------- src/utils/path-utils.ts | 21 ++ 2 files changed, 292 insertions(+), 184 deletions(-) create mode 100644 src/utils/path-utils.ts diff --git a/src/main.ts b/src/main.ts index 2336355..c996b23 100644 --- a/src/main.ts +++ b/src/main.ts @@ -3,34 +3,37 @@ import { Plugin, WorkspaceLeaf, - ItemView, Notice, TFile, TAbstractFile, + getAllTags, moment, } from 'obsidian'; import { WaypointSettings, DEFAULT_SETTINGS, PeriodNoteSettings } from 'src/settings'; import { WaypointSettingTab } from 'src/settings-tab'; import { WaypointView, WAYPOINT_VIEW_TYPE } from 'src/views/waypoint-view'; import { BookmarkItem, WaypointData } from 'src/models/bookmark'; +import { remapRenamedPath } from 'src/utils/path-utils'; -const DEFAULT_DATA: WaypointData = { - bookmarks: [], - recentFiles: [], -}; +export type PeriodKey = 'day' | 'week' | 'month' | 'quarter' | 'year'; export default class WaypointPlugin extends Plugin { public settings: WaypointSettings; public waypointData: WaypointData; public recentFiles: { path: string; basename: string }[] = []; - private recentFilesSaveTimer: ReturnType | null = null; + private recentFilesSaveTimer: number | undefined; + /** Serializes writes to data.json so overlapping saves cannot lose updates. */ + private savePromise: Promise = Promise.resolve(); + /** Basenames of every markdown file in the vault, for O(1) calendar lookups. */ + private markdownBasenames: Set = new Set(); async onload(): Promise { console.debug('Waypoint: loading plugin v' + this.manifest.version); - // Load persisted data - await this.loadSettings(); - await this.loadWaypointData(); + // Load persisted data — data.json is read exactly once here. + const saved = await this.loadData() as Record | null; + this.applySettings(saved); + this.applyWaypointData(saved); // Register the sidebar view this.registerView( @@ -156,23 +159,31 @@ export default class WaypointPlugin extends Plugin { ); this.registerEvent( - this.app.vault.on('create', () => this.onVaultChange()), + this.app.vault.on('create', (file: TAbstractFile) => this.onVaultCreate(file)), ); this.registerEvent( - this.app.vault.on('delete', () => this.onVaultChange()), + this.app.vault.on('delete', (file: TAbstractFile) => this.onVaultDelete(file)), ); this.registerEvent( this.app.vault.on('rename', (file, oldPath) => this.onRename(file, oldPath)), ); + this.registerEvent( + this.app.vault.on('modify', (file: TAbstractFile) => this.onFileModify(file)), + ); // Auto-open view on first load this.app.workspace.onLayoutReady(() => { + this.buildMarkdownIndex(); + const leaves = this.app.workspace.getLeavesOfType(WAYPOINT_VIEW_TYPE); if (leaves.length === 0) { const leaf = this.app.workspace.getLeftLeaf(false); if (leaf) { leaf.setViewState({ type: WAYPOINT_VIEW_TYPE }); } + } else { + // A restored view may have rendered before the index existed. + this.broadcastRedraw(); } }); @@ -193,31 +204,33 @@ export default class WaypointPlugin extends Plugin { this.app.workspace.detachLeavesOfType(WAYPOINT_VIEW_TYPE); } - // ── Settings ── + // ── Settings & persistence ── - async loadSettings(): Promise { - const saved = await this.loadData() as Record | null; + /** + * Merge persisted settings over the defaults. Nested objects are merged + * individually so existing configs keep their values while picking up + * fields added in newer versions. + */ + private applySettings(saved: Record | null): void { const s = (saved?.settings || {}) as Partial; this.settings = Object.assign({}, DEFAULT_SETTINGS, s); - // Deep merge nested objects that might be missing new fields this.settings.recentFiles = Object.assign({}, DEFAULT_SETTINGS.recentFiles, s.recentFiles || {}); this.settings.calendar = Object.assign({}, DEFAULT_SETTINGS.calendar, s.calendar || {}); this.settings.display = Object.assign({}, DEFAULT_SETTINGS.display, s.display || {}); + for (const key of PERIOD_SETTING_KEYS) { + this.settings[key] = Object.assign({}, DEFAULT_SETTINGS[key], s[key] || {}); + } } - async saveSettings(): Promise { - const all = (await this.loadData()) as Record || {}; - all.settings = this.settings; - await this.saveData(all); - } - - async loadWaypointData(): Promise { - const saved = await this.loadData() as Record | null; + private applyWaypointData(saved: Record | null): void { const d = (saved?.waypointData || {}) as Partial; - this.waypointData = Object.assign({}, DEFAULT_DATA, d); + this.waypointData = { + bookmarks: Array.isArray(d.bookmarks) ? d.bookmarks : [], + recentFiles: Array.isArray(d.recentFiles) ? d.recentFiles : [], + }; // Load persisted recent files - this.recentFiles = this.waypointData.recentFiles || []; + this.recentFiles = this.waypointData.recentFiles; // Apply current limit (in case maxItems was reduced since last save) if (this.recentFiles.length > this.settings.recentFiles.maxItems) { @@ -226,6 +239,37 @@ export default class WaypointPlugin extends Plugin { } } + /** + * Write both top-level keys of data.json from memory. + * + * Saves never re-read from disk: `settings` and `waypointData` are the only + * keys and both are held in memory, so a read-modify-write would only give + * two concurrent saves a stale snapshot of the other's key. Writes are + * chained onto `savePromise` so they cannot interleave. + */ + private persistAll(): Promise { + const write = this.savePromise.then(() => { + // Sync recentFiles into waypointData before writing + this.waypointData.recentFiles = this.recentFiles; + return this.saveData({ + settings: this.settings, + waypointData: this.waypointData, + }); + }); + // Keep the queue usable after a failed write without leaving an + // unhandled rejection behind; callers still see `write` reject. + this.savePromise = write.catch(() => undefined); + return write; + } + + async saveSettings(): Promise { + await this.persistAll(); + } + + async saveWaypointData(): Promise { + await this.persistAll(); + } + /** * Trim recent files to the current maxItems limit and persist. * Called when the maxItems setting changes. @@ -237,21 +281,13 @@ export default class WaypointPlugin extends Plugin { } } - async saveWaypointData(): Promise { - // Sync recentFiles into waypointData before saving - this.waypointData.recentFiles = this.recentFiles; - const all = (await this.loadData()) as Record || {}; - all.waypointData = this.waypointData; - await this.saveData(all); - } - /** * Persist recent files to disk (debounced to avoid excessive writes on rapid opens). */ persistRecentFiles(): void { this.waypointData.recentFiles = this.recentFiles; - if (this.recentFilesSaveTimer) clearTimeout(this.recentFilesSaveTimer); - this.recentFilesSaveTimer = setTimeout(() => { + window.clearTimeout(this.recentFilesSaveTimer); + this.recentFilesSaveTimer = window.setTimeout(() => { this.saveWaypointData(); }, 300); } @@ -259,24 +295,20 @@ export default class WaypointPlugin extends Plugin { // ── Recent Files ── private onFileOpen(file: TFile): void { - if (this.settings.recentFiles.updateOn === 'file-edit') { - // We'll handle this via quick-preview in a future refinement - return; - } + if (this.settings.recentFiles.updateOn !== 'file-open') return; + this.addToRecentFiles(file); + } + + private onFileModify(file: TAbstractFile): void { + if (this.settings.recentFiles.updateOn !== 'file-edit') return; + if (!(file instanceof TFile)) return; + // Already the most recent entry: nothing to reorder or redraw. + if (this.recentFiles.length > 0 && this.recentFiles[0].path === file.path) return; this.addToRecentFiles(file); } addToRecentFiles(file: TFile): void { - // Apply omitted paths filter - if (this.settings.recentFiles.omittedPaths.length > 0) { - for (const pattern of this.settings.recentFiles.omittedPaths) { - try { - if (new RegExp(pattern).test(file.path)) return; - } catch { - // Invalid regex, skip - } - } - } + if (this.isOmittedFromRecentFiles(file)) return; this.recentFiles = this.recentFiles.filter(f => f.path !== file.path); this.recentFiles.unshift({ path: file.path, basename: file.basename }); @@ -290,23 +322,137 @@ export default class WaypointPlugin extends Plugin { this.broadcastRedraw(); } - private onRename(file: TAbstractFile, oldPath: string): void { - const entry = this.recentFiles.find(f => f.path === oldPath); - if (entry) { - entry.path = file.path; - entry.basename = (file as TFile).basename || file.name.replace(/\.[^/.]+$/, ''); - this.persistRecentFiles(); - this.broadcastRedraw(); + /** + * Apply the omittedPaths / omittedTags filters. Each entry is treated as a + * regex; an invalid pattern is skipped rather than throwing. + */ + private isOmittedFromRecentFiles(file: TFile): boolean { + for (const pattern of this.settings.recentFiles.omittedPaths) { + try { + if (new RegExp(pattern).test(file.path)) return true; + } catch { + // Invalid regex, skip + } } - // Update bookmark file paths - this.updateBookmarkPath(oldPath, file.path); + const omittedTags = this.settings.recentFiles.omittedTags; + if (omittedTags.length === 0) return false; + + const cache = this.app.metadataCache.getFileCache(file); + const tags = (cache ? getAllTags(cache) : null) || []; + if (tags.length === 0) return false; + + const bareTags = tags.map(tag => tag.replace(/^#/, '')); + for (const pattern of omittedTags) { + try { + const regex = new RegExp(pattern); + if (bareTags.some(tag => regex.test(tag))) return true; + } catch { + // Invalid regex, skip + } + } + return false; } - private onVaultChange(): void { + /** + * Obsidian fires `rename` for folders too, so a stored path may need + * remapping either because it is the renamed item or because it sits + * inside a renamed folder. + */ + private onRename(file: TAbstractFile, oldPath: string): void { + const indexChanged = this.syncIndexForRename(file, oldPath); + let dataChanged = false; + + for (const entry of this.recentFiles) { + const remapped = remapRenamedPath(entry.path, oldPath, file.path); + if (remapped === null) continue; + entry.path = remapped; + entry.basename = basenameFromPath(remapped); + dataChanged = true; + } + + const remapBookmarks = (items: BookmarkItem[]): void => { + for (const item of items) { + if (item.filePath) { + const remapped = remapRenamedPath(item.filePath, oldPath, file.path); + if (remapped !== null) { + item.filePath = remapped; + dataChanged = true; + } + } + if (item.children) remapBookmarks(item.children); + } + }; + remapBookmarks(this.waypointData.bookmarks); + + if (dataChanged) { + this.waypointData.recentFiles = this.recentFiles; + this.persistAll(); + } + if (dataChanged || indexChanged) this.broadcastRedraw(); + } + + private onVaultCreate(file: TAbstractFile): void { + if (file instanceof TFile && file.extension === 'md') { + this.markdownBasenames.add(file.basename); + } this.broadcastRedraw(); } + private onVaultDelete(file: TAbstractFile): void { + if (file instanceof TFile && file.extension === 'md') { + this.removeFromMarkdownIndex(file.basename, file.path); + } + this.broadcastRedraw(); + } + + // ── Markdown basename index (calendar note indicators) ── + + private buildMarkdownIndex(): void { + this.markdownBasenames.clear(); + for (const file of this.app.vault.getMarkdownFiles()) { + this.markdownBasenames.add(file.basename); + } + } + + /** + * Drop a basename from the index, unless another markdown file still + * carries it. `path` is excluded from that check because the vault may not + * have dropped the file yet when the event fires. + */ + private removeFromMarkdownIndex(basename: string, path: string): boolean { + if (!this.markdownBasenames.has(basename)) return false; + const stillExists = this.app.vault.getMarkdownFiles() + .some(f => f.basename === basename && f.path !== path); + if (stillExists) return false; + this.markdownBasenames.delete(basename); + return true; + } + + /** Returns true when the index changed. Folder renames never change basenames. */ + private syncIndexForRename(file: TAbstractFile, oldPath: string): boolean { + if (!(file instanceof TFile)) return false; + + let changed = false; + const oldBasename = basenameFromPath(oldPath); + if (oldPath.toLowerCase().endsWith('.md') && (oldBasename !== file.basename || file.extension !== 'md')) { + changed = this.removeFromMarkdownIndex(oldBasename, oldPath); + } + if (file.extension === 'md' && !this.markdownBasenames.has(file.basename)) { + this.markdownBasenames.add(file.basename); + changed = true; + } + return changed; + } + + /** + * Whether any markdown file in the vault is named exactly `dateStr`. + * Synchronous and O(1) — the calendar calls this once per day cell. + */ + hasNoteForDate(dateStr: string): boolean { + return this.markdownBasenames.has(dateStr); + } + // ── Bookmarks ── addBookmark(filePath: string, label: string, type: 'file' | 'group' | 'separator' | 'spacer' = 'file', icon = ''): BookmarkItem { @@ -363,137 +509,72 @@ export default class WaypointPlugin extends Plugin { } } - private updateBookmarkPath(oldPath: string, newPath: string): void { - const updateRecursive = (items: BookmarkItem[]) => { - for (const item of items) { - if (item.filePath === oldPath) { - item.filePath = newPath; - } - if (item.children) updateRecursive(item.children); - } - }; - updateRecursive(this.waypointData.bookmarks); - } - // ── Period note creation/opening ── - async openPeriodNote(period: 'day' | 'week' | 'month' | 'quarter' | 'year', date: moment.Moment): Promise { - // Map period to settings and date format - type PeriodConfig = { - settings: PeriodNoteSettings; - label: string; - }; - - const configs: Record = { - day: { settings: this.settings.daily, label: 'Daily' }, - week: { settings: this.settings.weekly, label: 'Weekly' }, - month: { settings: this.settings.monthly, label: 'Monthly' }, - quarter: { settings: this.settings.quarterly, label: 'Quarterly' }, - year: { settings: this.settings.yearly, label: 'Yearly' }, - }; - - const config = configs[period]; - if (!config) return; - - const { settings: periodSettings } = config; + /** + * Open (creating if needed) the period note for `date`. + * Opens in `leaf` when given, otherwise in the active leaf. + */ + async openPeriodNote(period: PeriodKey, date: moment.Moment, leaf?: WorkspaceLeaf): Promise { + const config = PERIOD_CONFIGS[period]; + const periodSettings = this.settings[config.key]; const filename = date.format(periodSettings.nameFormat) + '.md'; const fullPath = periodSettings.folder ? `${periodSettings.folder}/${filename}` : filename; - // Check if exists let file = this.app.vault.getFileByPath(fullPath); if (!file) { - // Create it from template - try { - // Try to find template file - const templatePath = periodSettings.templateFile + '.md'; - const templateFile = this.app.vault.getFileByPath(templatePath); - - if (templateFile) { - const templateContent = await this.app.vault.read(templateFile); - file = await this.app.vault.create(fullPath, templateContent); - } else { - // Fallback: create with minimal frontmatter - const content = `---\ntype: ${periodSettings.typeProperty}\ndate: ${date.format('YYYY-MM-DD')}\n---\n\n`; - file = await this.app.vault.create(fullPath, content); - } - } catch (err) { - new Notice(`Failed to create ${config.label.toLowerCase()} note: ${err.message}`); - return; - } + file = await this.createPeriodNote(fullPath, periodSettings, date, config.label); + if (!file) return; new Notice(`Created ${config.label.toLowerCase()} note: ${filename}`); } - if (file) { - const leaf = this.app.workspace.getLeaf(false); - await leaf.openFile(file); + const target = leaf || this.app.workspace.getLeaf(false); + await target.openFile(file); + } + + /** Create a period note from its template, or from minimal frontmatter. */ + private async createPeriodNote( + fullPath: string, + periodSettings: PeriodNoteSettings, + date: moment.Moment, + label: string, + ): Promise { + try { + const templateFile = this.resolveTemplateFile(periodSettings.templateFile); + const content = templateFile + ? await this.app.vault.read(templateFile) + : `---\ntype: ${periodSettings.typeProperty}\ndate: ${date.format('YYYY-MM-DD')}\n---\n\n`; + return await this.app.vault.create(fullPath, content); + } catch (err: unknown) { + const message = err instanceof Error ? err.message : String(err); + new Notice(`Failed to create ${label.toLowerCase()} note: ${message}`); + return null; } } - // ── Open period note in a specific leaf (for middle-click) ── - async openPeriodNoteInLeaf(period: 'day' | 'week' | 'month' | 'quarter' | 'year', date: moment.Moment, leaf: any): Promise { - type PeriodConfig = { settings: PeriodNoteSettings; label: string }; - const configs: Record = { - day: { settings: this.settings.daily, label: 'Daily' }, - week: { settings: this.settings.weekly, label: 'Weekly' }, - month: { settings: this.settings.monthly, label: 'Monthly' }, - quarter: { settings: this.settings.quarterly, label: 'Quarterly' }, - year: { settings: this.settings.yearly, label: 'Yearly' }, - }; - const config = configs[period]; - if (!config) return; - const { settings: periodSettings } = config; - const filename = date.format(periodSettings.nameFormat) + '.md'; - const fullPath = periodSettings.folder ? `${periodSettings.folder}/${filename}` : filename; - let file = this.app.vault.getFileByPath(fullPath); - if (!file) { - try { - const templatePath = periodSettings.templateFile + '.md'; - const templateFile = this.app.vault.getFileByPath(templatePath); - if (templateFile) { - const templateContent = await this.app.vault.read(templateFile); - file = await this.app.vault.create(fullPath, templateContent); - } else { - const content = `---\ntype: ${periodSettings.typeProperty}\ndate: ${date.format('YYYY-MM-DD')}\n---\n\n`; - file = await this.app.vault.create(fullPath, content); - } - } catch (err) { - new Notice(`Failed to create ${config.label.toLowerCase()} note: ${err.message}`); - return; - } - } - if (file) { - await leaf.openFile(file); - } + /** The setting may or may not already carry the .md extension. */ + private resolveTemplateFile(templateFile: string): TFile | null { + if (!templateFile) return null; + const path = templateFile.toLowerCase().endsWith('.md') ? templateFile : `${templateFile}.md`; + return this.app.vault.getFileByPath(path); } // ── Period note navigation (next/prev from current file) ── /** - * Detect the period type from a filename's basename. + * Detect the period type from a filename's basename using the configured + * name formats, so custom formats keep working. Parsing is strict, which + * stops a loose format (e.g. YYYY) from swallowing a longer basename. * Returns the period key and the parsed moment, or null if not a period note. */ - private detectPeriodType(basename: string): { period: 'day' | 'week' | 'month' | 'quarter' | 'year'; date: moment.Moment } | null { - // YYYY-MM-DD → daily - if (/^\d{4}-\d{2}-\d{2}$/.test(basename)) { - return { period: 'day', date: moment(basename, 'YYYY-MM-DD') }; - } - // GGGG-WWW → weekly (e.g. 2026-W24) - if (/^\d{4}-W\d{2}$/.test(basename)) { - return { period: 'week', date: moment(basename, 'GGGG-[W]WW') }; - } - // YYYY-MM → monthly - if (/^\d{4}-\d{2}$/.test(basename)) { - return { period: 'month', date: moment(basename, 'YYYY-MM') }; - } - // YYYY-Q# → quarterly - if (/^\d{4}-Q[1-4]$/.test(basename)) { - return { period: 'quarter', date: moment(basename, 'YYYY-[Q]Q') }; - } - // YYYY → yearly - if (/^\d{4}$/.test(basename)) { - return { period: 'year', date: moment(basename, 'YYYY') }; + private detectPeriodType(basename: string): { period: PeriodKey; date: moment.Moment } | null { + for (const period of PERIOD_DETECTION_ORDER) { + const format = this.settings[PERIOD_CONFIGS[period].key].nameFormat; + if (!format) continue; + const date = moment(basename, format, true); + if (date.isValid()) return { period, date }; } return null; } @@ -545,20 +626,26 @@ export default class WaypointPlugin extends Plugin { } } } - - /** - * Get all markdown files that exist on a specific date. - * Used to show note indicators on the calendar. - */ - async getNotesForDate(dateStr: string): Promise { - return this.app.vault.getFiles().filter(f => - f.extension === 'md' && f.basename === dateStr, - ); - } - - async hasNoteForDate(dateStr: string): Promise { - return this.app.vault.getFiles().some(f => - f.extension === 'md' && f.basename === dateStr, - ); - } +} + +// ── Module helpers ── + +type PeriodSettingKey = 'daily' | 'weekly' | 'monthly' | 'quarterly' | 'yearly'; + +const PERIOD_SETTING_KEYS: PeriodSettingKey[] = ['daily', 'weekly', 'monthly', 'quarterly', 'yearly']; + +const PERIOD_CONFIGS: Record = { + day: { key: 'daily', label: 'Daily' }, + week: { key: 'weekly', label: 'Weekly' }, + month: { key: 'monthly', label: 'Monthly' }, + quarter: { key: 'quarterly', label: 'Quarterly' }, + year: { key: 'yearly', label: 'Yearly' }, +}; + +/** Checked day → year so a loose format (YYYY) cannot claim a longer basename. */ +const PERIOD_DETECTION_ORDER: PeriodKey[] = ['day', 'week', 'month', 'quarter', 'year']; + +function basenameFromPath(path: string): string { + const name = path.slice(path.lastIndexOf('/') + 1); + return name.replace(/\.[^/.]+$/, ''); } diff --git a/src/utils/path-utils.ts b/src/utils/path-utils.ts new file mode 100644 index 0000000..8d6ae3c --- /dev/null +++ b/src/utils/path-utils.ts @@ -0,0 +1,21 @@ +// ── Path helpers ── +// Pure functions only: no 'obsidian' imports, so this stays unit-testable in plain node. + +/** + * Remap a stored vault path after a rename. + * + * Obsidian's `vault.on('rename')` fires for folders as well as files, so a + * stored path can be affected either because it *is* the renamed item or + * because it lives inside a renamed folder. + * + * The nested check requires a `/` boundary, so renaming `notes/foo` leaves + * `notes/foobar.md` untouched. + * + * @returns the updated path, or `null` when `path` is unaffected. + */ +export function remapRenamedPath(path: string, oldPath: string, newPath: string): string | null { + if (!path || !oldPath) return null; + if (path === oldPath) return newPath; + if (path.startsWith(oldPath + '/')) return newPath + path.slice(oldPath.length); + return null; +}