Skip to content

Commit deddefd

Browse files
claremacraeclaremacrae
authored
Merge pull request #3457 from obsidian-tasks-group/refactor-add-IncludesSettingsService
Refactor add includes settings service
2 parents 1bb8b29 + 73f528e commit deddefd

File tree

3 files changed

+368
-27
lines changed

3 files changed

+368
-27
lines changed

‎src/Config/IncludesSettingsService.ts‎

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
import { renameKeyInRecordPreservingOrder } from '../lib/RecordHelpers';
2+
import type { IncludesMap } from './Settings';
3+
4+
export class IncludesSettingsService {
5+
/**
6+
* Adds a new include to the map with a unique key
7+
* @param includes The current includes map (will not be modified)
8+
* @returns An object with the updated includes map and the new key
9+
*/
10+
public addInclude(includes: Readonly<IncludesMap>): { includes: IncludesMap; newKey: string } {
11+
const newKey = this.generateUniqueKey(includes);
12+
const newIncludes = { ...includes };
13+
newIncludes[newKey] = '';
14+
return {
15+
includes: newIncludes,
16+
newKey,
17+
};
18+
}
19+
20+
/**
21+
* Renames a key in the includes map, preserving order
22+
* @param includes The current includes map (will not be modified)
23+
* @param keyBeingRenamed The existing key that would be renamed
24+
* @param proposedNewName The new name being considered
25+
* @returns The updated includes map, or null if the operation failed (for example, duplicate key)
26+
*/
27+
public renameInclude(
28+
includes: Readonly<IncludesMap>,
29+
keyBeingRenamed: string,
30+
proposedNewName: string,
31+
): IncludesMap | null {
32+
// Validate inputs
33+
if (!proposedNewName || proposedNewName.trim() === '') {
34+
return null; // Empty keys are not allowed
35+
}
36+
37+
proposedNewName = proposedNewName.trim();
38+
39+
// Check if this would create a duplicate
40+
if (this.wouldCreateDuplicateKey(includes, keyBeingRenamed, proposedNewName)) {
41+
return null;
42+
}
43+
44+
return renameKeyInRecordPreservingOrder(includes, keyBeingRenamed, proposedNewName);
45+
}
46+
47+
/**
48+
* Deletes an include from the map
49+
* @param includes The current includes map (will not be modified)
50+
* @param key The key to delete
51+
* @returns The updated includes map
52+
*/
53+
public deleteInclude(includes: Readonly<IncludesMap>, key: string): IncludesMap {
54+
const newIncludes = { ...includes };
55+
delete newIncludes[key];
56+
return newIncludes;
57+
}
58+
59+
/**
60+
* Updates the value of an include
61+
* @param includes The current includes map (will not be modified)
62+
* @param key The key to update
63+
* @param value The new value
64+
* @returns The updated includes map
65+
*/
66+
public updateIncludeValue(includes: Readonly<IncludesMap>, key: string, value: string): IncludesMap {
67+
const newIncludes = { ...includes };
68+
newIncludes[key] = value;
69+
return newIncludes;
70+
}
71+
72+
/**
73+
* Checks if renaming a key would create a duplicate in the includes map
74+
* @param includes The includes map to check against
75+
* @param keyBeingRenamed The existing key that would be renamed
76+
* @param proposedNewName The new name being considered
77+
* @returns True if the proposed new name would conflict with an existing key
78+
*/
79+
public wouldCreateDuplicateKey(
80+
includes: Readonly<IncludesMap>,
81+
keyBeingRenamed: string,
82+
proposedNewName: string,
83+
): boolean {
84+
// Normalize the proposedNewName once
85+
const normalizedNewName = proposedNewName.trim();
86+
87+
// If it's the same key (after trimming), it's not a duplicate
88+
if (keyBeingRenamed.trim() === normalizedNewName) {
89+
return false;
90+
}
91+
92+
// Check against all existing keys
93+
for (const existingKey of Object.keys(includes)) {
94+
// Skip the key being renamed (exact reference check)
95+
if (existingKey !== keyBeingRenamed && existingKey.trim() === normalizedNewName) {
96+
return true; // Found a duplicate
97+
}
98+
}
99+
100+
return false;
101+
}
102+
103+
/**
104+
* Generates a unique key for a new include
105+
* @param includes The current includes map
106+
* @returns A unique key string
107+
*/
108+
private generateUniqueKey(includes: Readonly<IncludesMap>): string {
109+
const baseKey = 'new_key';
110+
let suffix = 1;
111+
while (Object.prototype.hasOwnProperty.call(includes, `${baseKey}_${suffix}`)) {
112+
suffix++;
113+
}
114+
return `${baseKey}_${suffix}`;
115+
}
116+
}

‎src/Config/SettingsTab.ts‎

Lines changed: 38 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,15 @@ import { Status } from '../Statuses/Status';
66
import type { StatusCollection } from '../Statuses/StatusCollection';
77
import { createStatusRegistryReport } from '../Statuses/StatusRegistryReport';
88
import { i18n } from '../i18n/i18n';
9-
import { renameKeyInRecordPreservingOrder } from '../lib/RecordHelpers';
109
import * as Themes from './Themes';
11-
import { type HeadingState, type Settings, TASK_FORMATS } from './Settings';
10+
import { type HeadingState, type IncludesMap, type Settings, TASK_FORMATS } from './Settings';
1211
import { getSettings, isFeatureEnabled, updateGeneralSetting, updateSettings } from './Settings';
1312
import { GlobalFilter } from './GlobalFilter';
1413
import { StatusSettings } from './StatusSettings';
1514

1615
import { CustomStatusModal } from './CustomStatusModal';
1716
import { GlobalQuery } from './GlobalQuery';
17+
import { IncludesSettingsService } from './IncludesSettingsService';
1818

1919
export class SettingsTab extends PluginSettingTab {
2020
// If the UI needs a more complex setting you can create a
@@ -26,6 +26,7 @@ export class SettingsTab extends PluginSettingTab {
2626
};
2727

2828
private readonly plugin: TasksPlugin;
29+
private readonly includesSettingsService = new IncludesSettingsService();
2930

3031
constructor({ plugin }: { plugin: TasksPlugin }) {
3132
super(plugin.app, plugin);
@@ -662,13 +663,10 @@ export class SettingsTab extends PluginSettingTab {
662663
// Handle renaming an include
663664
const commitRename = async () => {
664665
if (newKey && newKey !== key) {
665-
const newIncludes = renameKeyInRecordPreservingOrder(settings.includes, key, newKey);
666-
updateSettings({ includes: newIncludes });
667-
await this.plugin.saveSettings();
668-
669-
// Refresh settings after replacing the includes object to avoid stale data in the next render.
670-
Object.assign(settings, getSettings());
671-
renderIncludes();
666+
const updatedIncludes = this.includesSettingsService.renameInclude(settings.includes, key, newKey);
667+
if (updatedIncludes) {
668+
await this.saveIncludesSettings(updatedIncludes, settings, renderIncludes);
669+
}
672670
}
673671
};
674672

@@ -689,9 +687,12 @@ export class SettingsTab extends PluginSettingTab {
689687
this.setupAutoResizingTextarea(textArea);
690688

691689
return textArea.onChange(async (newValue) => {
692-
settings.includes[key] = newValue;
693-
updateSettings({ includes: settings.includes });
694-
await this.plugin.saveSettings();
690+
const updatedIncludes = this.includesSettingsService.updateIncludeValue(
691+
settings.includes,
692+
key,
693+
newValue,
694+
);
695+
await this.saveIncludesSettings(updatedIncludes, settings, null);
695696
});
696697
});
697698

@@ -700,10 +701,8 @@ export class SettingsTab extends PluginSettingTab {
700701
btn.setIcon('cross')
701702
.setTooltip('Delete')
702703
.onClick(async () => {
703-
delete settings.includes[key];
704-
updateSettings({ includes: settings.includes });
705-
await this.plugin.saveSettings();
706-
renderIncludes();
704+
const updatedIncludes = this.includesSettingsService.deleteInclude(settings.includes, key);
705+
await this.saveIncludesSettings(updatedIncludes, settings, renderIncludes);
707706
});
708707
});
709708
}
@@ -726,22 +725,34 @@ export class SettingsTab extends PluginSettingTab {
726725
btn.setButtonText('Add new include')
727726
.setCta()
728727
.onClick(async () => {
729-
const newKey = this.generateUniqueIncludeKey(settings);
730-
settings.includes[newKey] = '';
731-
updateSettings({ includes: settings.includes });
732-
await this.plugin.saveSettings();
733-
renderIncludes();
728+
const { includes: updatedIncludes } = this.includesSettingsService.addInclude(settings.includes);
729+
await this.saveIncludesSettings(updatedIncludes, settings, renderIncludes);
734730
});
735731
});
736732
}
737733

738-
private generateUniqueIncludeKey(settings: Settings) {
739-
const baseKey = 'new_key';
740-
let suffix = 1;
741-
while (Object.prototype.hasOwnProperty.call(settings.includes, `${baseKey}_${suffix}`)) {
742-
suffix++;
734+
/**
735+
* Updates settings with new includes and refreshes UI if needed
736+
* @param updatedIncludes The new includes map
737+
* @param settings The current settings object to update
738+
* @param refreshView Callback to refresh the view (pass null if no refresh is needed)
739+
*/
740+
private async saveIncludesSettings(
741+
updatedIncludes: IncludesMap,
742+
settings: Settings,
743+
refreshView: (() => void) | null,
744+
): Promise<void> {
745+
// Update the settings in storage
746+
updateSettings({ includes: updatedIncludes });
747+
await this.plugin.saveSettings();
748+
749+
// Update the local settings object to reflect the changes
750+
settings.includes = { ...updatedIncludes };
751+
752+
// Refresh the view if a callback was provided
753+
if (refreshView) {
754+
refreshView();
743755
}
744-
return `${baseKey}_${suffix}`;
745756
}
746757

747758
private static renderFolderArray(folders: string[]): string {

0 commit comments

Comments
 (0)