Desktop, Mobile: Resolves #13048: Auto-disable plugin settings when conflicting built-in settings are enabled (#13055)

packages/lib/models/Setting.test.ts

@@ -521,4 +521,25 @@ describe('models/Setting', () => {
 		Setting.setValue('revisionService.ttlDays', 0);
 		expect(Setting.value('revisionService.ttlDays')).toBe(1);
 	});
+
+	test('should adjust settings to avoid conflicts', async () => {
+		const testSettingId = 'plugin-plugin.calebjohn.rich-markdown.inlineImages';
+		await Setting.registerSetting(testSettingId, {
+			public: true,
+			value: false,
+			type: SettingItemType.Bool,
+			storage: SettingStorage.File,
+		});
+
+		Setting.setValue('editor.imageRendering', true);
+
+		// Setting one conflicting setting should update the other
+		Setting.setValue(testSettingId, true);
+		expect(Setting.value('editor.imageRendering')).toBe(false);
+		expect(Setting.value(testSettingId)).toBe(true);
+
+		Setting.setValue('editor.imageRendering', true);
+		expect(Setting.value('editor.imageRendering')).toBe(true);
+		expect(Setting.value(testSettingId)).toBe(false);
+	});
 });

packages/lib/models/Setting.ts

@@ -169,6 +169,30 @@ const userSettingMigration: UserSettingMigration[] = [
 	},
 ];
 
+// Certain settings for similar (or the same) functionality can conflict. This map
+// allows automatically adjusting settings when conflicting settings are changed.
+// See https://github.com/laurent22/joplin/issues/13048
+const conflictingSettings = [
+	{
+		key1: 'plugin-io.github.personalizedrefrigerator.codemirror6-settings.hideMarkdown',
+		value1: 'some',
+		alternate1: 'none',
+
+		key2: 'editor.inlineRendering',
+		value2: true,
+		alternate2: false,
+	},
+	{
+		key1: 'plugin-plugin.calebjohn.rich-markdown.inlineImages',
+		value1: true,
+		alternate1: false,
+
+		key2: 'editor.imageRendering',
+		value2: true,
+		alternate2: false,
+	},
+];
+
 export type SettingMetadataSection = {
 	name: string;
 	isScreen?: boolean;
@@ -724,65 +748,77 @@ class Setting extends BaseModel {
 	public static setValue<T extends string>(key: T, value: SettingValueType<T>) {
 		if (!this.cache_) throw new Error('Settings have not been initialized!');
 
-		value = this.formatValue(key, value);
-		value = this.filterValue(key, value);
-
 		const md = this.settingMetadata(key);
-		const enforceLimits = (value: SettingValueType<T>) => {
+		const processValue = <Key extends string> (value: SettingValueType<Key>) => {
-			// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Partial refactor of old code before rule was applied
+			value = this.formatValue(key, value);
-			if ('minimum' in md && value < md.minimum) value = md.minimum as any;
+			value = this.filterValue(key, value);
-			// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Partial refactor of old code before rule was applied
+
-			if ('maximum' in md && value > md.maximum) value = md.maximum as any;
+			if ('minimum' in md && value < md.minimum) value = md.minimum as SettingValueType<Key>;
+			if ('maximum' in md && value > md.maximum) value = md.maximum as SettingValueType<Key>;
+
 			return value;
 		};
 
-		for (let i = 0; i < this.cache_.length; i++) {
+		const setValueInternal = <Key extends string> (key: Key, value: SettingValueType<Key>) => {
-			const c = this.cache_[i];
+			value = processValue(value);
-			if (c.key === key) {
+			for (let i = 0; i < this.cache_.length; i++) {
-				if (md.isEnum === true) {
+				const c = this.cache_[i];
-					if (!this.isAllowedEnumOption(key, value)) {
+				if (c.key === key) {
-						throw new Error(_('Invalid option value: "%s". Possible values are: %s.', value, this.enumOptionsDoc(key)));
+					if (md.isEnum === true) {
+						if (!this.isAllowedEnumOption(key, value)) {
+							throw new Error(_('Invalid option value: "%s". Possible values are: %s.', value, this.enumOptionsDoc(key)));
+						}
 					}
+
+					if (c.value === value) return;
+
+					this.changedKeys_.push(key);
+
+					// Don't log this to prevent sensitive info (passwords, auth tokens...) to end up in logs
+					// logger.info('Setting: ' + key + ' = ' + c.value + ' => ' + value);
+
+					c.value = value;
+
+					this.dispatch({
+						type: 'SETTING_UPDATE_ONE',
+						key: key,
+						value: c.value,
+					});
+
+					this.scheduleSave();
+					this.scheduleChangeEvent();
+					return;
 				}
+			}
 
-				if (c.value === value) return;
+			this.cache_.push({
+				key: key,
+				value: this.formatValue(key, value),
+			});
 
-				this.changedKeys_.push(key);
+			this.dispatch({
+				type: 'SETTING_UPDATE_ONE',
+				key: key,
+				value: this.formatValue(key, value),
+			});
 
-				// Don't log this to prevent sensitive info (passwords, auth tokens...) to end up in logs
+			this.changedKeys_.push(key);
-				// logger.info('Setting: ' + key + ' = ' + c.value + ' => ' + value);
 
-				c.value = enforceLimits(value);
+			this.scheduleSave();
+			this.scheduleChangeEvent();
+		};
 
-				this.dispatch({
+		setValueInternal(key, value);
-					type: 'SETTING_UPDATE_ONE',
-					key: key,
-					value: c.value,
-				});
 
-				this.scheduleSave();
+		// Prevent conflicts. Use setValueInternal to avoid infinite recursion in the case
-				this.scheduleChangeEvent();
+		// where conflictingSettings has invalid data.
-				return;
+		for (const conflict of conflictingSettings) {
+			if (conflict.key1 === key && conflict.value1 === value) {
+				setValueInternal(conflict.key2, conflict.alternate2);
+			} else if (conflict.key2 === key && conflict.value2 === value) {
+				setValueInternal(conflict.key1, conflict.alternate1);
 			}
 		}
-
-		value = enforceLimits(value);
-
-		this.cache_.push({
-			key: key,
-			value: this.formatValue(key, value),
-		});
-
-		this.dispatch({
-			type: 'SETTING_UPDATE_ONE',
-			key: key,
-			value: this.formatValue(key, value),
-		});
-
-		this.changedKeys_.push(key);
-
-		this.scheduleSave();
-		this.scheduleChangeEvent();
 	}
 
 	// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied