From 7e44237185d9ce67d3cb2ea9ca7a7e7a7ac83dbf Mon Sep 17 00:00:00 2001
From: Gary Ritchie <gary.t.ritchie@gmail.com>
Date: Fri, 3 Apr 2026 09:13:41 -0600
Subject: [PATCH] Serialize settings saves with copy-on-write semantics (#27)

* Fix release workflow tag filter and serialize all settings saves

The release workflow tag filter used regex syntax ([0-9]+) but GitHub
Actions expects glob patterns, so it would never match semver tags.
Replace with *.*.*; the manifest/tag equality check remains as strict
validation.

Move the settings save queue from BulkPropertiesSettingTab into the
plugin so all callers (settings tab, bulk-edit modal) share a single
serialized write path, preventing concurrent saves from overwriting
each other.

* Revert release workflow tag filter change

The original [0-9]+.[0-9]+.[0-9]+ pattern is correct. GitHub Actions
filter patterns support + as "one or more of the preceding character",
unlike standard globs. The review finding was based on the incorrect
assumption that + is literal in this context.

* Use copy-on-write semantics for serialized settings saves

updateSetting() on the plugin builds a candidate snapshot and only
assigns it to this.settings after saveData() succeeds. On failure,
in-memory settings remain unchanged, preventing a later unrelated
save from persisting a value that was reported as failed.

Both the settings tab and bulk-edit modal now use this single method
instead of mutating settings before saving.
---
 src/bulk-edit-modal.ts |  3 +--
 src/main.ts            | 19 +++++++++++++++++--
 src/settings.ts        | 30 +++++++++++-------------------
 3 files changed, 29 insertions(+), 23 deletions(-)

diff --git a/src/bulk-edit-modal.ts b/src/bulk-edit-modal.ts
index 0a5e493..68db0b2 100644
--- a/src/bulk-edit-modal.ts
+++ b/src/bulk-edit-modal.ts
@@ -644,9 +644,8 @@ export class BulkEditModal extends Modal {
 		const selProp = this.plugin.settings.selectionProperty;
 		const deselect = this.deselectWhenFinished;
 
-		this.plugin.settings.lastSelectedProperty = property;
 		try {
-			await this.plugin.saveSettings();
+			await this.plugin.updateSetting("lastSelectedProperty", property);
 		} catch (err: unknown) {
 			console.warn("bulk-properties: failed to save lastSelectedProperty:", err);
 		}
diff --git a/src/main.ts b/src/main.ts
index 970b8dd..b4a840e 100644
--- a/src/main.ts
+++ b/src/main.ts
@@ -10,6 +10,7 @@ export default class BulkPropertiesPlugin extends Plugin {
 	settings!: BulkPropertiesSettings;
 	private statusBarEl: HTMLElement | null = null;
 	private statusBarTimer: ReturnType<typeof setTimeout> | null = null;
+	private saveQueue: Promise<void> = Promise.resolve();
 
 	override async onload() {
 		await this.loadSettings();
@@ -167,7 +168,21 @@ export default class BulkPropertiesPlugin extends Plugin {
 		}
 	}
 
-	async saveSettings() {
-		await this.saveData(this.settings);
+	/**
+	 * Serializes settings writes through a queue with copy-on-write
+	 * semantics. Builds a candidate snapshot per call; only assigns
+	 * it to `this.settings` after persistence succeeds.
+	 */
+	updateSetting<K extends keyof BulkPropertiesSettings>(
+		key: K,
+		value: BulkPropertiesSettings[K],
+	): Promise<void> {
+		const save = this.saveQueue.then(async () => {
+			const candidate = {...this.settings, [key]: value};
+			await this.saveData(candidate);
+			this.settings = candidate;
+		});
+		this.saveQueue = save.then(() => {}, () => {});
+		return save;
 	}
 }
diff --git a/src/settings.ts b/src/settings.ts
index af779bf..231463a 100644
--- a/src/settings.ts
+++ b/src/settings.ts
@@ -98,7 +98,6 @@ export const DEFAULT_SETTINGS: BulkPropertiesSettings = {
 
 export class BulkPropertiesSettingTab extends PluginSettingTab {
 	plugin: BulkPropertiesPlugin;
-	private saveQueue: Promise<void> = Promise.resolve();
 
 	constructor(app: App, plugin: BulkPropertiesPlugin) {
 		super(app, plugin);
@@ -106,28 +105,21 @@ export class BulkPropertiesSettingTab extends PluginSettingTab {
 	}
 
 	/**
-	 * Serializes settings writes through a queue so overlapping
-	 * onChange calls cannot race. Each call snapshots the live
-	 * settings only after all prior saves have committed.
+	 * Updates a single setting key via the plugin's serialized
+	 * copy-on-write save queue.
 	 */
-	private updateSetting<K extends keyof BulkPropertiesSettings>(
+	private async updateSetting<K extends keyof BulkPropertiesSettings>(
 		key: K,
 		value: BulkPropertiesSettings[K],
 	): Promise<boolean> {
-		const result = this.saveQueue.then(async () => {
-			const candidate = {...this.plugin.settings, [key]: value};
-			try {
-				await this.plugin.saveData(candidate);
-				this.plugin.settings = candidate;
-				return true;
-			} catch (err: unknown) {
-				console.error("bulk-properties: failed to save settings:", err);
-				new Notice("Failed to save settings. Check disk space and permissions.");
-				return false;
-			}
-		});
-		this.saveQueue = result.then(() => {});
-		return result;
+		try {
+			await this.plugin.updateSetting(key, value);
+			return true;
+		} catch (err: unknown) {
+			console.error("bulk-properties: failed to save settings:", err);
+			new Notice("Failed to save settings. Check disk space and permissions.");
+			return false;
+		}
 	}
 
 	display(): void {