From b854ce14ce8dd9a2e84703a378dd924938bc31fd Mon Sep 17 00:00:00 2001 From: SebastianMC <23032356+SebastianMC@users.noreply.github.com> Date: Sat, 21 Oct 2023 00:40:38 +0200 Subject: [PATCH] #74 - Integration with Bookmarks core plugin and support for indirect drag & drop arrangement - functionality completed!!! - increased coverage of the new functionality with unit tests - more unit tests possible - basic manual tests done - next step: real-life usage tests --- package.json | 1 + src/custom-sort/custom-sort.ts | 11 +- src/main.ts | 76 +- ...ookmarks Core Plugin integration design.md | 55 + .../BookmarksCorePluginSignature.spec.ts | 1065 +++++++++++++++++ src/utils/BookmarksCorePluginSignature.ts | 399 ++++-- yarn.lock | 15 + 7 files changed, 1508 insertions(+), 114 deletions(-) create mode 100644 src/utils/Bookmarks Core Plugin integration design.md create mode 100644 src/utils/BookmarksCorePluginSignature.spec.ts diff --git a/package.json b/package.json index aee27aeb..9655eb6a 100644 --- a/package.json +++ b/package.json @@ -27,6 +27,7 @@ "jest": "^28.1.1", "monkey-around": "^2.3.0", "obsidian": "^0.15.4", + "obsidian-1.4.11": "npm:obsidian@1.4.11", "ts-jest": "^28.0.5", "tslib": "2.4.0", "typescript": "4.7.4" diff --git a/src/custom-sort/custom-sort.ts b/src/custom-sort/custom-sort.ts index 5b0fb010..6d1fb239 100644 --- a/src/custom-sort/custom-sort.ts +++ b/src/custom-sort/custom-sort.ts @@ -615,10 +615,8 @@ export const determineBookmarksOrderIfNeeded = (folderItems: Array Object.assign({} as CustomSortGroup, group)) - - // expand folder-specific macros const parentFolderName: string|undefined = this.file.name expandMacros(sortingSpec, parentFolderName) @@ -655,10 +653,15 @@ export const folderSort = function (sortingSpec: CustomSortSpec, ctx: Processing }; // Returns a sorted copy of the input array, intentionally to keep it intact -export const sortFolderItemsForBookmarking = function (items: Array, sortingSpec: CustomSortSpec|null|undefined, ctx: ProcessingContext, uiSortOrder: string): Array { +export const sortFolderItemsForBookmarking = function (folder: TFolder, items: Array, sortingSpec: CustomSortSpec|null|undefined, ctx: ProcessingContext, uiSortOrder: string): Array { if (sortingSpec) { const folderItemsByPath: { [key: string]: TAbstractFile } = {} + // shallow copy of groups and expand folder-specific macros on them + sortingSpec.groupsShadow = sortingSpec.groups?.map((group) => Object.assign({} as CustomSortGroup, group)) + const parentFolderName: string|undefined = folder.name + expandMacros(sortingSpec, parentFolderName) + const folderItems: Array = items.map((entry: TFile | TFolder) => { folderItemsByPath[entry.path] = entry const itemForSorting: FolderItemForSorting = determineSortingGroup(entry, sortingSpec, ctx) diff --git a/src/main.ts b/src/main.ts index d237a73c..3ab223af 100644 --- a/src/main.ts +++ b/src/main.ts @@ -46,7 +46,8 @@ import { import {getStarredPlugin} from "./utils/StarredPluginSignature"; import { BookmarksPluginInterface, - getBookmarksPlugin + getBookmarksPlugin, + groupNameForPath } from "./utils/BookmarksCorePluginSignature"; import {getIconFolderPlugin} from "./utils/ObsidianIconFolderPluginSignature"; import {lastPathComponent} from "./utils/utils"; @@ -357,6 +358,17 @@ export default class CustomSortPlugin extends Plugin { } }); }; + const unbookmarkThisMenuItem = (item: MenuItem) => { + item.setTitle('Custom sort: UNbookmark from sorting.'); + item.setIcon('hashtag'); + item.onClick(() => { + const bookmarksPlugin = getBookmarksPlugin(plugin.settings.bookmarksGroupToConsumeAsOrderingReference) + if (bookmarksPlugin) { + bookmarksPlugin.unbookmarkFolderItem(file) + bookmarksPlugin.saveDataAndUpdateBookmarkViews(true) + } + }); + }; const bookmarkAllMenuItem = (item: MenuItem) => { item.setTitle('Custom sort: bookmark+siblings for sorting.'); item.setIcon('hashtag'); @@ -369,15 +381,73 @@ export default class CustomSortPlugin extends Plugin { } }); }; + const unbookmarkAllMenuItem = (item: MenuItem) => { + item.setTitle('Custom sort: UNbookmark+all siblings from sorting.'); + item.setIcon('hashtag'); + item.onClick(() => { + const bookmarksPlugin = getBookmarksPlugin(plugin.settings.bookmarksGroupToConsumeAsOrderingReference) + if (bookmarksPlugin) { + const orderedChildren: Array = file.parent.children.map((entry: TFile | TFolder) => entry) + bookmarksPlugin.unbookmarkSiblings(orderedChildren) + bookmarksPlugin.saveDataAndUpdateBookmarkViews(true) + } + }); + }; const itemAlreadyBookmarkedForSorting: boolean = bookmarksPlugin.isBookmarkedForSorting(file) if (!itemAlreadyBookmarkedForSorting) { menu.addItem(bookmarkThisMenuItem) + } else { + menu.addItem(unbookmarkThisMenuItem) } menu.addItem(bookmarkAllMenuItem) + menu.addItem(unbookmarkAllMenuItem) }) ) + if (requireApiVersion('1.4.11')) { + this.registerEvent( + // "files-menu" event was exposed in 1.4.11 + // @ts-ignore + app.workspace.on("files-menu", (menu: Menu, files: TAbstractFile[], source: string, leaf?: WorkspaceLeaf) => { + if (!this.settings.bookmarksContextMenus) return; // Don't show the context menus at all + + const bookmarksPlugin = getBookmarksPlugin(plugin.settings.bookmarksGroupToConsumeAsOrderingReference) + if (!bookmarksPlugin) return; // Don't show context menu if bookmarks plugin not available and not enabled + + const bookmarkSelectedMenuItem = (item: MenuItem) => { + item.setTitle('Custom sort: bookmark selected for sorting.'); + item.setIcon('hashtag'); + item.onClick(() => { + const bookmarksPlugin = getBookmarksPlugin(plugin.settings.bookmarksGroupToConsumeAsOrderingReference) + if (bookmarksPlugin) { + files.forEach((file) => { + bookmarksPlugin.bookmarkFolderItem(file) + }) + bookmarksPlugin.saveDataAndUpdateBookmarkViews(true) + } + }); + }; + const unbookmarkSelectedMenuItem = (item: MenuItem) => { + item.setTitle('Custom sort: UNbookmark selected from sorting.'); + item.setIcon('hashtag'); + item.onClick(() => { + const bookmarksPlugin = getBookmarksPlugin(plugin.settings.bookmarksGroupToConsumeAsOrderingReference) + if (bookmarksPlugin) { + files.forEach((file) => { + bookmarksPlugin.unbookmarkFolderItem(file) + }) + bookmarksPlugin.saveDataAndUpdateBookmarkViews(true) + } + }); + }; + + menu.addItem(bookmarkSelectedMenuItem) + menu.addItem(unbookmarkSelectedMenuItem) + }) + ) + } + this.registerEvent( app.vault.on("rename", (file: TAbstractFile, oldPath: string) => { const bookmarksPlugin = getBookmarksPlugin(plugin.settings.bookmarksGroupToConsumeAsOrderingReference) @@ -504,6 +574,7 @@ export default class CustomSortPlugin extends Plugin { const has: HasSortingOrGrouping = collectSortingAndGroupingTypes(sortSpec) return sortFolderItemsForBookmarking( + folder, folder.children, sortSpec, this.createProcessingContextForSorting(has), @@ -669,7 +740,8 @@ class CustomSortSettingTab extends PluginSettingTab { .setPlaceholder('e.g. Group for sorting') .setValue(this.plugin.settings.bookmarksGroupToConsumeAsOrderingReference) .onChange(async (value) => { - this.plugin.settings.bookmarksGroupToConsumeAsOrderingReference = value.trim() ? pathToFlatString(normalizePath(value)) : ''; + value = groupNameForPath(value.trim()).trim() + this.plugin.settings.bookmarksGroupToConsumeAsOrderingReference = value ? pathToFlatString(normalizePath(value)) : ''; await this.plugin.saveSettings(); })); diff --git a/src/utils/Bookmarks Core Plugin integration design.md b/src/utils/Bookmarks Core Plugin integration design.md new file mode 100644 index 00000000..7307afad --- /dev/null +++ b/src/utils/Bookmarks Core Plugin integration design.md @@ -0,0 +1,55 @@ +Integration with Bookmarks core plugin: +- support two approaches _at the same time_: + - (A) structured bookmarks inside a dedicated bookmarks group, and + - (B) a flat list of bookmarks inside the dedicated bookmarks group + +For (A): +- preferred +- a folder is represented by a group in bookmarks +- a file is represented by a file-with-block + - this also applied to non-md files, like jpg and others +- guarantees _'hiding'_ the bookmarks-for-sorting from regular bookmarks usage scenarios + - bookmark entries for sorting are encapsulated in the dedicated group + - they don't interfere with bookmarking of files and folders via standard bookmarking +- only exact location of file bookmark / group matches for sorting order in file explorer +- the contextual bookmark menus always work in (A) mode + - the contextual menus create / modify the bookmarks structure on-the-fly + +For (B): +- discouraged, yet supported (exception for some edge cases) +- typically a result of manual bookmarks management +- for small number of items seems reasonable +- for flat vaults it could look same as for (A) +- groups don't have a 'path' attribute, their path is determined by their location +- bookmarked folders represent folders if inside the bookmarks group for sorting + - yet in this way they interfere with regular bookmarks scenario +- file bookmarks work correctly in non-interfering way thanks to the _'artificial block reference'_ +- file bookmarks not having the _'artificial block ref'_ work as well + - if they are in the designated bookmarks group + - if there isn't a duplicate, which has the _'artificial block ref'_ + - yet in this way they interfere with regular bookmarks scenario + +-[ ] TODO: review again the 'item moved' and 'item deleted' scenarios (they look ok, check if they don't delete/move too much) + - [x] fundamental question 1: should 'move' create a bookmark entry/structure if it is not covered by bookmarks? + - Answer: the moved item is removed from bookmarks. If it is a group with descendants not transparent for sorting, + it is renamed to become transparent for sorting. + By design, the order of items is property of the parent folder (the container) and not the items + - [x] fundamental question 2: should 'move' create a bookmark entry if moved item was not bookmarked, yet is moved to a folder covered by bookmarks? + - Answer: same as for previous point. + - [x] review from (A) and (B) perspective + - Answer: scenario (A) is fully handled by 'item moved' and 'item deleted'. + scenario (B) is partially handled for 'item moved'. Details to be read from code (too complex to cover here) + - [x] consider deletion of item outside of bookmarks sorting container group + Answer: bookmark items outside of bookmarks sorting container are not manipulated by custom-sort plugin + to not interfere with standard Bookmarks scenarios + - [x] consider moving an item outside of bookmarks group + - Answer: question not relevant. Items are moved in file explorer and bookmarks only reflect that, if needed. + Hence there is no concept of 'moving an item outside of bookmarks group' - bookmarks group only exists in bookmarks + - [x] edge case: bookmarked item is a group, the deleted/moved is a file, not a folder --> what to do? + - Answer: for moved files, only file bookmarks are scanned (and handles), for moved folders, only groups are scanned (and handled). + - [x] delete all instances at any level of bookmarks structure in 'delete' handler + - Answer: only instances of (A) or (B) are deleted. Items outside of bookmarks container for sorting or + in invalid locations in bookmarks hierarchy are ignored + + + diff --git a/src/utils/BookmarksCorePluginSignature.spec.ts b/src/utils/BookmarksCorePluginSignature.spec.ts new file mode 100644 index 00000000..a83d0eb8 --- /dev/null +++ b/src/utils/BookmarksCorePluginSignature.spec.ts @@ -0,0 +1,1065 @@ +import { + _unitTests, + BookmarkedParentFolder, + OrderedBookmarks +} from "./BookmarksCorePluginSignature"; +import {extractParentFolderPath} from "./utils"; + +type Bookmarks_PluginInstance = any + +const getEmptyBookmarksMock = (): Bookmarks_PluginInstance => ({} as Bookmarks_PluginInstance) + +const getNullBookmarksMock = (): Bookmarks_PluginInstance => ({ + items: null + } as unknown as Bookmarks_PluginInstance) + +const getBookmarksMock = (items: any): Bookmarks_PluginInstance => ({ + items: items +} as Bookmarks_PluginInstance) + +const getBrokenBookmarksMock1 = (): Bookmarks_PluginInstance => ({ + items: 123 +} as Bookmarks_PluginInstance) + +const getBrokenBookmarksMock2 = (): Bookmarks_PluginInstance => ({ + items: () => { return [] } +} as Bookmarks_PluginInstance) + +const derivePath = (previousPath: string|undefined, path: string): string => { + const match = path?.match(/(^\s+\/)/) + if (match && previousPath) { + const lengthToTake = match[1].length + let derivedPart = previousPath.substring(0, lengthToTake) + derivedPart = derivedPart.endsWith('/') ? derivedPart : `${derivedPart}/` + const newPathSuffix = path.substring(lengthToTake) + return `${derivedPart}${newPathSuffix}` + } else { + return path + } +} + +interface PathAndOrder { + path: string + order: number +} + +class bkCacheMock { + entries: Array = [] + coveredPaths: {[key: string]: boolean} = {} + + getCache() { + const cache: OrderedBookmarks = {} + this.entries.forEach((it) => { + cache[it.path] = it.order + }) + return cache + } + getPathsCoverage() { + return this.coveredPaths + } + getExpected() { + return [this.getCache(), this.getPathsCoverage()] + } + add(path: string, order: number): bkCacheMock { + path = derivePath(this.entries.slice(-1)?.[0]?.path, path) + this.entries.push({ + path: path, + order: order + }) + this.coveredPaths[extractParentFolderPath(path) || '/'] = true + return this + } +} + +const attachRemoveFunction = (arr: Array): Array => { + arr.remove = (itemToRemove: any) => { + if (itemToRemove) { + let index + do { + index = arr?.findIndex((it) => it === itemToRemove) + if (index !== -1) { + arr?.splice(index, 1) + } + } while (index !== -1) + } + } + return arr +} + +const consumeBkMock = (tree: any): Array => { + const bkmrks: Array = attachRemoveFunction([]) + const pathOf = (s: string) => { + const match = s.match(/^\d+:(.+)$/) + return match ? match[1] : s + } + const typeOf = (s: string) => { + const match = s.match(/(file|folder)/) + return match ? match[1] : 'unknown' + } + const hasIndicator = (s: string) => { + return s.endsWith('^') + } + const consumeLevel = (entries: any, container?: Array) => { + Object.keys(entries).forEach((entryKey) => { + container = container ?? attachRemoveFunction([]) + const value = entries[entryKey] + if ('string' === typeof value) { // file or folder + const path = pathOf(entryKey) + container.push({ + type: typeOf(value), + path: pathOf(entryKey), + subpath: hasIndicator(value) ? '#^-' : undefined + }) + } else { // group + container.push({ + type: 'group', + items: consumeLevel(value), + title: entryKey + }) + } + }) + return container || attachRemoveFunction([]) + } + consumeLevel(tree, bkmrks) + return bkmrks +} + +describe('getOrderedBookmarks - basic scenarios', () => { + it('should correctly handle no bookmarks plugin scenario', () => { + const result = _unitTests.getOrderedBookmarks(null!) + expect(result).toEqual([undefined, undefined]) + }) + it('should correctly handle no bookmarks scenario', () => { + const result = _unitTests.getOrderedBookmarks(getEmptyBookmarksMock()) + expect(result).toEqual([undefined, undefined]) + }) + it('should correctly handle edge case of the bookmarks plugin responding with null', () => { + const result = _unitTests.getOrderedBookmarks(getNullBookmarksMock()) + expect(result).toEqual([undefined, undefined]) + }) + it('should correctly handle bookmarks plugin responding with empty collection', () => { + const items: [] = [] + const result = _unitTests.getOrderedBookmarks(getBookmarksMock(items)) + expect(result).toEqual([{}, {}]) + }) + it('should correctly handle basic scenario - one bookmarked item at root level - file', () => { + const items: Array = consumeBkMock({'some note.md': 'file'}) + const expected = new bkCacheMock() + .add('some note.md', 1) + .getExpected() + const result = _unitTests.getOrderedBookmarks(getBookmarksMock(items)) + expect(result).toEqual(expected) + }) + it('should correctly handle basic scenario - one bookmarked item at root level - file with subpath', () => { + const items: Array = consumeBkMock({'some note.md': 'file^'}) + const expected = new bkCacheMock() + .add('some note.md', 1) + .getExpected() + const result = _unitTests.getOrderedBookmarks(getBookmarksMock(items)) + expect(result).toEqual(expected) + }) + it('should correctly handle basic scenario - one bookmarked item at root level - folder', () => { + const items: Array = consumeBkMock({'some folder': 'folder'}) + const expected = new bkCacheMock() + .add('some folder', 1) + .getExpected() + const result = _unitTests.getOrderedBookmarks(getBookmarksMock(items)) + expect(result).toEqual(expected) + }) + it('should correctly handle basic scenario - one bookmarked item at root level - group', () => { + const items: Array = consumeBkMock({'sortspec': {}}) + const expected = new bkCacheMock() + .add('sortspec', 1) + .getExpected() + const result = _unitTests.getOrderedBookmarks(getBookmarksMock(items)) + expect(result).toEqual(expected) + }) + it('should correctly handle basic scenario - only the group with expected name - the container', () => { + const items: Array = consumeBkMock({'sortspec': {}}) + const result = _unitTests.getOrderedBookmarks(getBookmarksMock(items), 'sortspec') + expect(result).toEqual([{}, {}]) + }) + it('should correctly handle basic scenario - one bookmarked item in the group of expected name', () => { + const items: [] = [] + const result = _unitTests.getOrderedBookmarks(getBookmarksMock(items)) + expect(result).toEqual([{}, {}]) + }) +}) + +describe('getOrderedBookmarks edge cases', () => { + it('should correctly handle bookmarks plugin not returning collection (a number)', () => { + const result = _unitTests.getOrderedBookmarks(getBrokenBookmarksMock1()) + expect(result).toEqual([undefined, undefined]) + }) + it('should correctly handle bookmarks plugin not returning collection (a function)', () => { + const result = _unitTests.getOrderedBookmarks(getBrokenBookmarksMock1()) + expect(result).toEqual([undefined, undefined]) + }) + it('edge case - group vs group', () => { + const items = consumeBkMock({ + "sortspec": { + "\\\\folder l1": {}, // order 1 + "folder l1": {} + } + }) + + const expected = new bkCacheMock() + .add("folder l1", 1) + .getExpected() + const result = _unitTests.getOrderedBookmarks(getBookmarksMock(items), DEFAULT_BKMRK_FOLDER) + expect(result).toEqual(expected) + }) + it('edge case - folder vs group', () => { + const items = consumeBkMock({ + "sortspec": { + "1:folder l1": "folder", // order 1 + "folder l1": {} // order 2 + } + }) + + const expected = new bkCacheMock() + .add("folder l1", 2) + .getExpected() + const result = _unitTests.getOrderedBookmarks(getBookmarksMock(items), DEFAULT_BKMRK_FOLDER) + expect(result).toEqual(expected) + }) + it('edge case - file w/ and w/o indicator vs group - group overwrites w/o indicator, regardless of matching (or not) path of file', () => { + const items = consumeBkMock({ + "sortspec": { + "1:artificial": "file", // order 1 + "artificial": {}, // order 2 + "subf": { // order 3 + "subf/item in subf": "file", // order 4 + "item in subf": {} // order 5 + } + } + }) + + const expected = new bkCacheMock() + .add("artificial", 2) + .add("subf", 3) + .add(" /item in subf", 5) + .getExpected() + const result = _unitTests.getOrderedBookmarks(getBookmarksMock(items), DEFAULT_BKMRK_FOLDER) + expect(result).toEqual(expected) + }) + it('edge case - file w/ and w/o indicator vs group - group is not overwritten by w/o indicator', () => { + const items = consumeBkMock({ + "sortspec": { + "artificial": {}, // order 1 + "1:artificial": "file", + "\\\\subf": { + "item in subf": {}, // order 2 + "subf/item in subf": "file", + } + } + }) + + const expected = new bkCacheMock() + .add("artificial", 1) + .add("subf/item in subf", 2) + .getExpected() + const result = _unitTests.getOrderedBookmarks(getBookmarksMock(items), DEFAULT_BKMRK_FOLDER) + expect(result).toEqual(expected) + }) + it('edge case - file w/ and w/o indicator vs group - group doesn`t overwrite w/ indicator', () => { + const items = consumeBkMock({ + "sortspec": { + "1:artificial": "file", // order 1 + "2:artificial": "file^", // order 2 + "artificial": {} // order 3 + } + }) + + const expected = new bkCacheMock() + .add("artificial", 2) + .getExpected() + const result = _unitTests.getOrderedBookmarks(getBookmarksMock(items), DEFAULT_BKMRK_FOLDER) + expect(result).toEqual(expected) + }) + it('edge case - file w/ and w/o indicator vs group - w/ indicator overwrites group', () => { + const items = consumeBkMock({ + "sortspec": { + "1:artificial": "file", // order 1 + "artificial": {}, // order 2 + "2:artificial": "file^", // order 3 + } + }) + + const expected = new bkCacheMock() + .add("artificial", 3) + .getExpected() + const result = _unitTests.getOrderedBookmarks(getBookmarksMock(items), DEFAULT_BKMRK_FOLDER) + expect(result).toEqual(expected) + }) + it('edge case - folder is treated as a file w/o indicator - scenario 1', () => { + const items = consumeBkMock({ + "sortspec": { + "\\\\group": { + "1:group/artificial": "folder", // order 1 + "2:group/artificial": "file", // order 2 + } + } + }) + + const expected = new bkCacheMock() + .add("group/artificial", 1) + .getExpected() + const result = _unitTests.getOrderedBookmarks(getBookmarksMock(items), DEFAULT_BKMRK_FOLDER) + expect(result).toEqual(expected) + }) + it('edge case - folder is treated as a file w/o indicator - scenario 2', () => { + const items = consumeBkMock({ + "sortspec": { + "\\\\group": { + "1:group/artificial": "folder", // order 1 + "2:group/artificial": "file^", // order 2 + } + } + }) + + const expected = new bkCacheMock() + .add("group/artificial", 2) + .getExpected() + const result = _unitTests.getOrderedBookmarks(getBookmarksMock(items), DEFAULT_BKMRK_FOLDER) + expect(result).toEqual(expected) + }) +}) + +const DEFAULT_BKMRK_FOLDER = 'sortspec' + +describe('getOrderedBookmarks', () => { + describe('files', () => { + it('case 1 - both w/ indicator and not matching path. Ignore the latter.', () => { + const items = consumeBkMock({ + "sortspec": { + "folder l1/not relevant for test.md": "file^", // order 1 + "1:folder l1/folder l2/file 1 at level 2.md": "file^", // order 2 + "2:folder l1/folder l2/file 1 at level 2.md": "file^" // order 3 -> reject, a duplicate + } + }) + const expected = new bkCacheMock() + .add('folder l1/not relevant for test.md', 1) + .add(' /folder l2/file 1 at level 2.md', 2) + .getExpected() + const result = _unitTests.getOrderedBookmarks(getBookmarksMock(items), DEFAULT_BKMRK_FOLDER) + expect(result).toEqual(expected) + }) + it('case 2 - both w/ indicator, matching path wins.', () => { + const items = consumeBkMock({ + "sortspec": { + "\\\\folder l1": { + "folder l1/folder l2/file 1 at level 2.md": "file^", // ignore => invalid location + "\\\\folder l2": { + "folder l1/folder l2/not relevant for test.md": "file^", // order 1 + "\\\\folder l3": { + "folder l1/folder l2/folder l3/file 1 at level 3.md": "file^", // order 2, to be taken -> location match + }, + "1:folder l1/folder l2/folder l3/file 1 at level 3.md": "file^", + "2:folder l1/folder l2/folder l3/file 1 at level 3.md": "folder", + } + }, + "folder l1/folder l2/folder l3/file 1 at level 3.md": "file^", // reject => already found in correct location + } + }) + + const expected = new bkCacheMock() + .add('folder l1/folder l2/not relevant for test.md', 1) + .add(" /folder l3/file 1 at level 3.md", 2) + .getExpected() + const result = _unitTests.getOrderedBookmarks(getBookmarksMock(items), DEFAULT_BKMRK_FOLDER) + expect(result).toEqual(expected) + }) + it('case 3 - w/ indicator wins', () => { + const items = consumeBkMock({ + "sortspec": { + "folder l1/folder l2/folder l3/file 1 at level 3.md": "file", // order 1 -> overwritten by w/ indicator + "folder l1/folder l2/not relevant for test.md": "file^", // order 2 + "1:folder l1/folder l2/folder l3/file 1 at level 3.md": "file^", // order 3 -> taken, overwrites item w/o indicator + } + }) + + const expected = new bkCacheMock() + .add('folder l1/folder l2/not relevant for test.md', 2) + .add(" /folder l3/file 1 at level 3.md", 3) + .getExpected() + const result = _unitTests.getOrderedBookmarks(getBookmarksMock(items), DEFAULT_BKMRK_FOLDER) + expect(result).toEqual(expected) + }) + it('case 4 - w/ indicator wins', () => { + const items = consumeBkMock({ + "sortspec": { + "folder l1/folder l2/not relevant for test.md": "file^", // order 1 + "folder l1": { // order 2 + "\\\\folder l2": { + "\\\\folder l3": { + "folder l1/folder l2/folder l3/file 1 at level 3.md": "file", // order 3 -> overwritten by w/ indicator + } + } + }, + "folder l1/folder l2/folder l3/file 1 at level 3.md": "file^", // order 4 -> taken, overwrites item w/o indicator + } + }) + + const expected = new bkCacheMock() + .add("folder l1", 2) + .add("folder l1/folder l2/not relevant for test.md", 1) + .add(" /folder l3/file 1 at level 3.md", 4) + .getExpected() + const result = _unitTests.getOrderedBookmarks(getBookmarksMock(items), DEFAULT_BKMRK_FOLDER) + expect(result).toEqual(expected) + }) + it('case 5 - both w/ indicator, matching path wins', () => { + const items = consumeBkMock({ + "sortspec": { + "folder l1/folder l2/not relevant for test.md": "file^", // order 1 + "folder l1/folder l2/folder l3/file 1 at level 3.md": "file^", // order 2 -> overwritten by matching path + "\\\\folder l1": { + "folder l2": { // order 3 + "\\\\folder l3": { + "folder l1/folder l2/folder l3/file 1 at level 3.md": "file^", // order 4 -> overwrites item at root path + } + } + }, + } + }) + + const expected = new bkCacheMock() + .add("folder l1/folder l2", 3) + .add("folder l1/folder l2/not relevant for test.md", 1) + .add(" /folder l3/file 1 at level 3.md", 4) + .getExpected() + const result = _unitTests.getOrderedBookmarks(getBookmarksMock(items), DEFAULT_BKMRK_FOLDER) + expect(result).toEqual(expected) + }) + it('case 6 - both w/ indicator and matching path. Ignore the latter', () => { + const items = consumeBkMock({ + "sortspec": { + "\\\\folder l1": { + "folder l2": { // order 1 + "\\\\folder l3": { + "1:folder l1/folder l2/folder l3/file 1 at level 3.md": "file^", // order 2 -> taken + "2:folder l1/folder l2/folder l3/file 1 at level 3.md": "file^", // rejected as duplicate + }, + "folder l1/folder l2/not relevant for test.md": "file", // order 3 + } + }, + } + }) + + const expected = new bkCacheMock() + .add("folder l1/folder l2", 1) + .add("folder l1/folder l2/not relevant for test.md", 3) + .add(" /folder l3/file 1 at level 3.md", 2) + .getExpected() + const result = _unitTests.getOrderedBookmarks(getBookmarksMock(items), DEFAULT_BKMRK_FOLDER) + expect(result).toEqual(expected) + }) + it('case 7 - w/ indicator wins', () => { + const items = consumeBkMock({ + "sortspec": { + "folder l1/folder l2/folder l3/file 1 at level 3.md": "file", // order 1 -> overwritten by w/ indicator + "folder l1": { // order 2 + "\\\\folder l2": { + "folder l3": { // order 3 + "1:folder l1/folder l2/folder l3/file 1 at level 3.md": "file^", // order 4 -> taken, overwrites w/o indicator + }, + "folder l1/folder l2/not relevant for test.md": "file", // order 5 + } + }, + } + }) + + const expected = new bkCacheMock() + .add("folder l1", 2) + .add("folder l1/folder l2/folder l3", 3) + .add("folder l1/folder l2/not relevant for test.md", 5) + .add(" /folder l3/file 1 at level 3.md", 4) + .getExpected() + const result = _unitTests.getOrderedBookmarks(getBookmarksMock(items), DEFAULT_BKMRK_FOLDER) + expect(result).toEqual(expected) + }) + it('case 8 - w/ indicator wins', () => { + const items = consumeBkMock({ + "sortspec": { + "folder l1": { // order 1 + "\\\\folder l2": { + "folder l3": { // order 2 + "folder l1/folder l2/folder l3/file 1 at level 3.md": "file", // order 3 -> overwritten by w/ indicator + "1:folder l1/folder l2/folder l3/file 1 at level 3.md": "file^", // order 4 -> taken, overwrites w/o indicator + }, + "folder l1/folder l2/not relevant for test.md": "file", // order 5 + } + }, + } + }) + + const expected = new bkCacheMock() + .add("folder l1", 1) + .add("folder l1/folder l2/folder l3", 2) + .add("folder l1/folder l2/not relevant for test.md", 5) + .add(" /folder l3/file 1 at level 3.md", 4) + .getExpected() + const result = _unitTests.getOrderedBookmarks(getBookmarksMock(items), DEFAULT_BKMRK_FOLDER) + expect(result).toEqual(expected) + }) + it('case 9 - Item w/o indicator never overwrites the one with the indicator', () => { + const items = consumeBkMock({ + "sortspec": { + "folder l1": { // order 1 + "\\\\folder l2": { + "folder l3": { // order 2 + }, + "folder l1/folder l2/not relevant for test.md": "file", // order 3 + } + }, + "1:folder l1/folder l2/folder l3/file 1 at level 3.md": "file^", // order 4 -> taken, + "2:folder l1/folder l2/folder l3/file 1 at level 3.md": "file", // ignored as duplicate w/o indicator + } + }) + + const expected = new bkCacheMock() + .add("folder l1", 1) + .add("folder l1/folder l2/folder l3", 2) + .add("folder l1/folder l2/not relevant for test.md", 3) + .add(" /folder l3/file 1 at level 3.md", 4) + .getExpected() + const result = _unitTests.getOrderedBookmarks(getBookmarksMock(items), DEFAULT_BKMRK_FOLDER) + expect(result).toEqual(expected) + }) + it('case 10 - Item w/o indicator never overwrites the one with the indicator', () => { + const items = consumeBkMock({ + "sortspec": { + "folder l1": { // order 1 + "\\\\folder l2": { + "folder l3": { // order 2 + "folder l1/folder l2/folder l3/file 1 at level 3.md": "file^", // order 3 -> taken, + }, + "folder l1/folder l2/not relevant for test.md": "file", // order 4 + } + }, + "folder l1/folder l2/folder l3/file 1 at level 3.md": "file", // ignored as duplicate w/o indicator and not matching path + } + }) + + const expected = new bkCacheMock() + .add("folder l1", 1) + .add("folder l1/folder l2/folder l3", 2) + .add("folder l1/folder l2/not relevant for test.md", 4) + .add(" /folder l3/file 1 at level 3.md", 3) + .getExpected() + const result = _unitTests.getOrderedBookmarks(getBookmarksMock(items), DEFAULT_BKMRK_FOLDER) + expect(result).toEqual(expected) + }) + it('case 11 - both w/o indicator and not matching path. Ignore the latter.', () => { + const items = consumeBkMock({ + "sortspec": { + "folder l1": { // order 1 + "\\\\folder l2": { + "folder l3": { // order 2 + }, + }, + }, + "1:folder l1/folder l2/folder l3/file 1 at level 3.md": "file", // order 3 -> taken + "2:folder l1/folder l2/folder l3/file 1 at level 3.md": "file", // ignored as duplicate w/o indicator and not matching path + "folder l1/folder l2/not relevant for test.md": "folder", // order 4 + } + }) + + const expected = new bkCacheMock() + .add("folder l1", 1) + .add("folder l1/folder l2/folder l3", 2) + .add(" /folder l3/file 1 at level 3.md", 3) + .add("folder l1/folder l2/not relevant for test.md", 4) + .getExpected() + const result = _unitTests.getOrderedBookmarks(getBookmarksMock(items), DEFAULT_BKMRK_FOLDER) + expect(result).toEqual(expected) + }) + it('case 12 - both w/o indicator, matching path is not overwritten by not matching', () => { + const items = consumeBkMock({ + "sortspec": { + "folder l1": { // order 1 + "\\\\folder l2": { + "folder l3": { // order 2 + "folder l1/folder l2/folder l3/file 1 at level 3.md": "file", // order 3 -> taken + }, + }, + }, + "1:folder l1/folder l2/folder l3/file 1 at level 3.md": "file", // ignored as duplicate w/o indicator and not matching path + "folder l1/folder l2/not relevant for test.md": "folder", // order 4 + } + }) + + const expected = new bkCacheMock() + .add("folder l1", 1) + .add("folder l1/folder l2/folder l3", 2) + .add(" /folder l3/file 1 at level 3.md", 3) + .add("folder l1/folder l2/not relevant for test.md", 4) + .getExpected() + const result = _unitTests.getOrderedBookmarks(getBookmarksMock(items), DEFAULT_BKMRK_FOLDER) + expect(result).toEqual(expected) + }) + it('case 13 - w/ indicator wins', () => { + const items = consumeBkMock({ + "sortspec": { + "folder l1": { // order 1 + "\\\\folder l2": { + "folder l3": { // order 2 + "folder l1/folder l2/folder l3/file 1 at level 3.md": "file", // order 3 -> overwritten by w/ indicator + }, + }, + }, + "folder l1/folder l2/folder l3/file 1 at level 3.md": "file^", // order 4 -> w/ indicator overwrites w/o indicator + "folder l1/folder l2/not relevant for test.md": "folder", // order 5 + } + }) + + const expected = new bkCacheMock() + .add("folder l1", 1) + .add("folder l1/folder l2/folder l3", 2) + .add(" /folder l3/file 1 at level 3.md", 4) + .add("folder l1/folder l2/not relevant for test.md", 5) + .getExpected() + const result = _unitTests.getOrderedBookmarks(getBookmarksMock(items), DEFAULT_BKMRK_FOLDER) + expect(result).toEqual(expected) + }) + it('case 14 - w/ indicator wins', () => { + const items = consumeBkMock({ + "sortspec": { + "folder l1": { // order 1 + "\\\\folder l2": { + "folder l3": { // order 2 + "1:folder l1/folder l2/folder l3/file 1 at level 3.md": "file^", // order 3 + "2:folder l1/folder l2/folder l3/file 1 at level 3.md": "file", // duplicate, ignored regardless of w/ indicator and matching path + }, + }, + }, + + "folder l1/folder l2/not relevant for test.md": "folder", // order 5 + } + }) + + const expected = new bkCacheMock() + .add("folder l1", 1) + .add(" /folder l2/folder l3", 2) + .add(" /file 1 at level 3.md", 3) + .add(" /not relevant for test.md", 4) + .getExpected() + const result = _unitTests.getOrderedBookmarks(getBookmarksMock(items), DEFAULT_BKMRK_FOLDER) + expect(result).toEqual(expected) + }) + it('case 15 - both w/o indicator, matching path wins', () => { + const items = consumeBkMock({ + "sortspec": { + "folder l1/folder l2/folder l3/file 1 at level 3.md": "file", // order 1 -> overwritten by matching path + "folder l1": { // order 2 + "\\\\folder l2": { + "folder l3": { // order 3 + "folder l1/folder l2/folder l3/file 1 at level 3.md": "file", // order 4 -> overwrites not matching path + }, + }, + }, + "folder l1/folder l2/not relevant for test.md": "folder", // order 5 + } + }) + + const expected = new bkCacheMock() + .add("folder l1", 2) + .add("folder l1/folder l2/folder l3", 3) + .add(" /file 1 at level 3.md", 4) + .add(" /not relevant for test.md", 5) + .getExpected() + const result = _unitTests.getOrderedBookmarks(getBookmarksMock(items), DEFAULT_BKMRK_FOLDER) + expect(result).toEqual(expected) + }) + it('case 15 - both w/o indicator and matching path. Ignore the latter.', () => { + const items = consumeBkMock({ + "sortspec": { + "folder l1": { // order 1 + "folder l2": { // order 2 + "folder l3": { // order 3 + }, + "1:folder l1/folder l2/file 1 at level 2.md": "file", // order 4 -> overwrites not matching path + "2:folder l1/folder l2/file 1 at level 2.md": "file", // ignored as duplicate + }, + }, + "folder l1/folder l2/not relevant for test.md": "folder", // order 5 + } + }) + + const expected = new bkCacheMock() + .add("folder l1", 1) + .add(" /folder l2", 2) + .add(" /folder l3", 3) + .add(" /file 1 at level 2.md", 4) + .add(" /not relevant for test.md", 5) + .getExpected() + const result = _unitTests.getOrderedBookmarks(getBookmarksMock(items), DEFAULT_BKMRK_FOLDER) + expect(result).toEqual(expected) + }) + it('case 16 and 17 - ignore invalid locations', () => { + const items = consumeBkMock({ + "sortspec": { + "folder l1": { // order 1 + "folder l1/folder l2/file 1 at level 2.md": "file^", + "folder l2": { // order 2 + "folder l3": { // order 3 + "folder l1/folder l2/file 1 at level 2.md": "file^", + }, + "folder l1/file 1 at level 1.md": "file", + }, + }, + "folder l1/folder l2/not relevant for test.md": "folder", // order 4 + } + }) + + const expected = new bkCacheMock() + .add("folder l1", 1) + .add(" /folder l2", 2) + .add(" /folder l3", 3) + .add(" /not relevant for test.md", 4) + .getExpected() + const result = _unitTests.getOrderedBookmarks(getBookmarksMock(items), DEFAULT_BKMRK_FOLDER) + expect(result).toEqual(expected) + }) + }) +}) + +/* +Duplicate elimination logic matrix + +Originally table created in Excel, then copied & pasted toObsidian, which creates a good md format +Then converted from md to reStructuredText grid format via https://tableconvert.com/markdown-to-restructuredtext +(a great tables generator/converter in various formats) + ++---------+--------+---------------------+---------------------+----------------+--+---------------------+---------------------+--------+--+-------------+----------------------------------------------------------------------+ +| Case id | | alreadyConsumed | | | | new | | | | reject new? | Comment / scenario | +| | Object | hasSortingIndicator | bookmarkPathMatches | path=/ | | hasSortingIndicator | bookmarkPathMatches | path=/ | | | | ++=========+========+=====================+=====================+================+==+=====================+=====================+========+==+=============+======================================================================+ +| 1 | file | yes | no | yes | | yes | no | yes | | reject | both w/ indicator and not matching path. Ignore the latter. | +| 2 | file | yes | yes | N/R | | yes | no | yes | | reject | both w/ indicator, matching path wins | +| 3 | file | no | no | yes | | yes | no | yes | | take | w/ indicator wins | +| 4 | file | no | yes | N/R | | yes | no | yes | | take | w/ indicator wins | +| 5 | file | yes | no | yes | | yes | yes | N/R | | take | both w/ indicator, matching path wins | +| 6 | file | yes | yes | N/R | | yes | yes | N/R | | reject | both w/ indicator and matching path. Ignore the latter | +| 7 | file | no | no | yes | | yes | yes | N/R | | take | w/ indicator wins | +| 8 | file | no | yes | N/R | | yes | yes | N/R | | take | w/ indicator wins | +| 9 | file | yes | no | yes | | no | no | yes | | reject | Item w/o indicator never overwrites the one with the indicator | +| 10 | file | yes | yes | N/R | | no | no | yes | | reject | Item w/o indicator never overwrites the one with the indicator | +| 11 | file | no | no | yes | | no | no | yes | | reject | both w/o indicator and not matching path. Ignore the latter. | +| 12 | file | no | yes | N/R | | no | no | yes | | reject | both w/o indicator, matching path is not overwritten by not matching | +| 13 | file | yes | no | yes | | no | yes | N/R | | reject | w/ indicator wins | +| 14 | file | yes | yes | N/R | | no | yes | N/R | | reject | w/ indicator wins | +| 15 | file | no | no | yes | | no | yes | N/R | | take | both w/o indicator, matching path wins | +| 16 | file | no | yes | N/R | | no | yes | N/R | | reject | both w/o indicator and matching path. Ignore the latter. | +| | | | | | | | | | | | | +| 17 | file | Doesn't matter | Doesn't matter | Doesn't matter | | no | no | no | | reject | An item in neither of correct bookmarks locations. | +| 18 | file | Doesn't matter | Doesn't matter | Doesn't matter | | yes | no | no | | reject | An item in neither of correct bookmarks locations. | ++---------+--------+---------------------+---------------------+----------------+--+---------------------+---------------------+--------+--+-------------+----------------------------------------------------------------------+ + + +- N/R = Not Relevant +- file and folder bookmark of the same path ==> not distinguished. Folders are a subset of the above matrix. + +edge cases when path of group or folder or file overlaps + - group vs group - the first one wins, to have consistent rules (should never occur, defensive programming) + - group always wins over folder or a file w/o indicator + - group vs file w/ indicator - w/ indicator always wins + - folder vs file - not distinguished, folder treated as a file w/o indicator + +Group name with double backslash prefix: \\groupName represent a bookmark not used for sorting, only used for the structure + */ + +describe('bookmarkedGroupEmptyOrOnlyTransparentForSortingDescendants', () => { + it('empty (undefined)', () => { + const sortspecGroup = { + type: 'group', + title: 'sortspec' + } + + const result = _unitTests.bookmarkedGroupEmptyOrOnlyTransparentForSortingDescendants(sortspecGroup as any) + expect(result).toBeTruthy() + }) + it('empty (zero length array)', () => { + const items = consumeBkMock({ + "sortspec": { + } + }) + + const result = _unitTests.bookmarkedGroupEmptyOrOnlyTransparentForSortingDescendants(items[0]) + expect(result).toBeTruthy() + }) + it('complex empty structure', () => { + const items = consumeBkMock({ + "sortspec": { + "\\\\group l1.1": { + "\\\\group l2.1": { + "\\\\group l3.1": {} + }, + "\\\\group l2.2": {} + }, + "\\\\group l1.2": { + "\\\\group l2.1": {} + } + } + }) + + const result = _unitTests.bookmarkedGroupEmptyOrOnlyTransparentForSortingDescendants(items[0]) + expect(result).toBeTruthy() + }) + it('complex structure - one nested not transparent group', () => { + const items = consumeBkMock({ + "sortspec": { + "\\\\group l1.1": { + "\\\\group l2.1": { + "group l3.1": {} + }, + "\\\\group l2.2": {} + }, + "\\\\group l1.2": { + "\\\\group l2.1": {} + } + } + }) + + const result = _unitTests.bookmarkedGroupEmptyOrOnlyTransparentForSortingDescendants(items[0]) + expect(result).toBeFalsy() + }) + it('complex structure - one nested file', () => { + const items = consumeBkMock({ + "sortspec": { + "\\\\group l1.1": { + "\\\\group l2.1": { + "\\\\group l3.1": {} + }, + "\\\\group l2.2": { + "some file (invalid location, yet anyways a file)": "file" + } + }, + "\\\\group l1.2": { + "\\\\group l2.1": {} + } + } + }) + + const result = _unitTests.bookmarkedGroupEmptyOrOnlyTransparentForSortingDescendants(items[0]) + expect(result).toBeFalsy() + }) + it('complex structure - one not nested folder', () => { + const items = consumeBkMock({ + "sortspec": { + "\\\\group l1.1": { + "\\\\group l2.1": { + "\\\\group l3.1": {} + }, + "\\\\group l2.2": {} + }, + "\\\\group l1.2": { + "\\\\group l2.1": {} + }, + "a folder (must be manual)": "folder" + } + }) + + const result = _unitTests.bookmarkedGroupEmptyOrOnlyTransparentForSortingDescendants(items[0]) + expect(result).toBeFalsy() + }) +}) + +describe('cleanupBookmarkTreeFromTransparentEmptyGroups', () => { + it('should delete the structure up to the bookmark container group (starting point deep in hierarchy)', () => { + const items = consumeBkMock({ + "sortspec": { + "\\\\group l1.1": { + "\\\\group l2.1": { + "\\\\group l3.1": { + "\\\\deepest": {} + } + }, + "\\\\group l2.2": {} + }, + "\\\\group l1.2": { + "\\\\group l2.1": {} + } + } + }) + const plugin = getBookmarksMock(items) + const parentFolder: BookmarkedParentFolder|undefined = _unitTests.findGroupForItemPathInBookmarks( + 'group l1.1/group l2.1/group l3.1/deepest', + false, + plugin, + 'sortspec' + ) + _unitTests.cleanupBookmarkTreeFromTransparentEmptyGroups( + parentFolder, + plugin, + 'sortspec' + ) + expect(JSON.parse(JSON.stringify(items))).toEqual(JSON.parse(JSON.stringify(consumeBkMock({ + "sortspec": {} + })))) + }) + it('should delete 1st level group and not delete the bookmark container group (starting point flat)', () => { + const items = consumeBkMock({ + "sortspec": { + "\\\\group l1.2": {} + } + }) + const plugin = getBookmarksMock(items) + const parentFolder: BookmarkedParentFolder|undefined = _unitTests.findGroupForItemPathInBookmarks( + 'group l1.2', + false, + plugin, + 'sortspec' + ) + _unitTests.cleanupBookmarkTreeFromTransparentEmptyGroups( + parentFolder, + plugin, + 'sortspec' + ) + expect(JSON.parse(JSON.stringify(items))).toEqual(JSON.parse(JSON.stringify(consumeBkMock({ + "sortspec": {} + })))) + }) + it('should delete the structure up to the non-empty group', () => { + const items = consumeBkMock({ + "sortspec": { + "\\\\group l1.1": { + "\\\\group l2.1": { + "\\\\group l3.1": { + "\\\\deepest": {} + } + }, + "\\\\group l2.2": {} + }, + "\\\\group l1.2": { + "\\\\group l2.1": {}, + "group l2.2": {} + } + } + }) + const plugin = getBookmarksMock(items) + const parentFolder: BookmarkedParentFolder|undefined = _unitTests.findGroupForItemPathInBookmarks( + 'group l1.1/group l2.1/group l3.1/deepest', + false, + plugin, + 'sortspec' + ) + _unitTests.cleanupBookmarkTreeFromTransparentEmptyGroups( + parentFolder, + plugin, + 'sortspec' + ) + expect(JSON.parse(JSON.stringify(items))).toEqual(JSON.parse(JSON.stringify(consumeBkMock({ + "sortspec": { + "\\\\group l1.2": { + "\\\\group l2.1": {}, + "group l2.2": {} + } + } + })))) + }) + it('should not delete the structure because of sibling', () => { + const items = consumeBkMock({ + "sortspec": { + "\\\\group l1.1": { + "\\\\group l2.1": { + "\\\\group l3.1": { + "\\\\deepest 1": {}, + "deepest 2": "file^" + + } + }, + "\\\\group l2.2": { + "\\\\deepest 1": {} + } + }, + "\\\\group l1.2": { + "\\\\group l2.1": { + "\\\\deepest 1": {} + } + } + } + }) + const plugin = getBookmarksMock(items) + const parentFolder: BookmarkedParentFolder|undefined = _unitTests.findGroupForItemPathInBookmarks( + 'group l1.1/group l2.1/group l3.1/deepest', + false, + plugin, + 'sortspec' + ) + _unitTests.cleanupBookmarkTreeFromTransparentEmptyGroups( + parentFolder, + plugin, + 'sortspec' + ) + expect(JSON.parse(JSON.stringify(items))).toEqual(JSON.parse(JSON.stringify(consumeBkMock({ + "sortspec": { + "\\\\group l1.1": { + "\\\\group l2.1": { + "\\\\group l3.1": { + "\\\\deepest 1": {}, + "deepest 2": "file^" + + } + }, + "\\\\group l2.2": { + "\\\\deepest 1": {} + } + }, + "\\\\group l1.2": { + "\\\\group l2.1": { + "\\\\deepest 1": {} + } + } + } + })))) + }) + it('should delete the structure after multi-clean invocation', () => { + const items = consumeBkMock({ + "sortspec": { + "\\\\group l1.1": { + "\\\\group l2.1": { + "\\\\group l3.1": { + "\\\\deepest 1": {} + } + }, + "\\\\group l2.2": { + "\\\\deepest 2": {} + } + }, + "\\\\group l1.2": { + "\\\\group l2.1": { + "\\\\deepest 3": {} + } + } + } + }) + const plugin = getBookmarksMock(items) + const parentFolder1: BookmarkedParentFolder|undefined = _unitTests.findGroupForItemPathInBookmarks( + 'group l1.1/group l2.1/group l3.1/deepest 1', + false, + plugin, + 'sortspec' + ) + _unitTests.cleanupBookmarkTreeFromTransparentEmptyGroups( + parentFolder1, + plugin, + 'sortspec' + ) + const parentFolder2: BookmarkedParentFolder|undefined = _unitTests.findGroupForItemPathInBookmarks( + 'group l1.2/group l2.1/deepest 3', + false, + plugin, + 'sortspec' + ) + _unitTests.cleanupBookmarkTreeFromTransparentEmptyGroups( + parentFolder2, + plugin, + 'sortspec' + ) + expect(JSON.parse(JSON.stringify(items))).toEqual(JSON.parse(JSON.stringify(consumeBkMock({ + "sortspec": {} + })))) + }) +}) diff --git a/src/utils/BookmarksCorePluginSignature.ts b/src/utils/BookmarksCorePluginSignature.ts index 9bc0ccde..a2fff745 100644 --- a/src/utils/BookmarksCorePluginSignature.ts +++ b/src/utils/BookmarksCorePluginSignature.ts @@ -9,6 +9,8 @@ import { extractParentFolderPath, lastPathComponent } from "./utils"; +import {Arr} from "tern"; +import * as process from "process"; const BookmarksPlugin_getBookmarks_methodName = 'getBookmarks' @@ -21,45 +23,48 @@ type Path = string type BookmarkedItem = BookmarkedFile | BookmarkedFolder | BookmarkedGroup // Either a file, a folder or header/block inside a file -interface BookmarkWithPath { +interface BookmarkItemSuperset { path: Path + title?: string + ctime: number + subpath?: string // Anchor within the file (heading and/or block ref) } -interface BookmarkedFile { +interface BookmarkWithPath extends Pick { +} + +interface BookmarkedFile extends BookmarkItemSuperset { type: 'file' - path: Path - subpath?: string // Anchor within the file (heading and/or block ref) - title?: string - ctime: number } -interface BookmarkedFolder { +interface BookmarkedFolder extends Omit { type: 'folder' - path: Path - title?: string - ctime: number } -interface BookmarkedGroup { +interface BookmarkedGroup extends Omit { type: 'group' items: Array - title?: string - ctime: number } export type BookmarkedItemPath = string -export interface OrderedBookmarkedItem { - file: boolean - folder: boolean - group: boolean +export interface OrderedBookmarkedItemWithMetadata { + isGroup?: boolean path: BookmarkedItemPath + hasSortingIndicator?: boolean order: number - bookmarkPathOverlap: number|true // how much the location in bookmarks hierarchy matches the actual file/folder path + bookmarkPathMatches?: boolean +} + +export type OrderedBookmarkedItem = Pick +export type Order = number + +export interface OrderedBookmarks { + [key: BookmarkedItemPath]: Order } -interface OrderedBookmarks { - [key: BookmarkedItemPath]: OrderedBookmarkedItem +export interface OrderedBookmarksWithMetadata { + [key: BookmarkedItemPath]: OrderedBookmarkedItemWithMetadata } interface Bookmarks_PluginInstance extends PluginInstance { @@ -72,8 +77,10 @@ interface Bookmarks_PluginInstance extends PluginInstance { export interface BookmarksPluginInterface { determineBookmarkOrder(path: string): number|undefined bookmarkFolderItem(item: TAbstractFile): void + unbookmarkFolderItem(item: TAbstractFile): void saveDataAndUpdateBookmarkViews(updateBookmarkViews: boolean): void bookmarkSiblings(siblings: Array, inTheTop?: boolean): void + unbookmarkSiblings(siblings: Array): void updateSortingBookmarksAfterItemRenamed(renamedItem: TAbstractFile, oldPath: string): void updateSortingBookmarksAfterItemDeleted(deletedItem: TAbstractFile): void isBookmarkedForSorting(item: TAbstractFile): boolean @@ -82,6 +89,25 @@ export interface BookmarksPluginInterface { bookmarksIncludeItemsInFolder(folderPath: string): boolean } +const checkSubtreeForOnlyTransparentGroups = (items: Array): boolean => { + if (!items || items?.length === 0) return true + for (let it of items) { + if (it.type !== 'group' || !it.title || !isGroupTransparentForSorting(it.title)) { + return false + } + // it is a group transparent for sorting + const isEmptyOrTransparent: boolean = checkSubtreeForOnlyTransparentGroups(it.items) + if (!isEmptyOrTransparent) { + return false + } + } + return true +} + +const bookmarkedGroupEmptyOrOnlyTransparentForSortingDescendants = (group: BookmarkedGroup): boolean => { + return checkSubtreeForOnlyTransparentGroups(group.items) +} + class BookmarksPluginWrapper implements BookmarksPluginInterface { plugin: Bookmarks_PluginInstance|undefined @@ -96,16 +122,16 @@ class BookmarksPluginWrapper implements BookmarksPluginInterface { // Intentionally not returning 0 to allow simple syntax of processing the result // // Parameterless invocation enforces cache population, if empty - determineBookmarkOrder = (path?: string): number | undefined => { + determineBookmarkOrder = (path?: string): Order | undefined => { if (!bookmarksCache) { [bookmarksCache, bookmarksFoldersCoverage] = getOrderedBookmarks(this.plugin!, this.groupNameForSorting) bookmarksCacheTimestamp = Date.now() } if (path && path.length > 0) { - const bookmarkedItemPosition: number | undefined = bookmarksCache?.[path]?.order + const bookmarkedItemPosition: Order | undefined = bookmarksCache?.[path] - return (bookmarkedItemPosition !== undefined && bookmarkedItemPosition >= 0) ? (bookmarkedItemPosition + 1) : undefined + return (bookmarkedItemPosition && bookmarkedItemPosition > 0) ? bookmarkedItemPosition : undefined } else { return undefined } @@ -115,6 +141,10 @@ class BookmarksPluginWrapper implements BookmarksPluginInterface { this.bookmarkSiblings([item], true) } + unbookmarkFolderItem = (item: TAbstractFile) => { + this.unbookmarkSiblings([item]) + } + saveDataAndUpdateBookmarkViews = (updateBookmarkViews: boolean = true) => { this.plugin!.onItemsChanged(true) if (updateBookmarkViews) { @@ -138,7 +168,13 @@ class BookmarksPluginWrapper implements BookmarksPluginInterface { if (bookmarksContainer) { // for sanity, the group should be always created if missing siblings.forEach((aSibling) => { const siblingName = lastPathComponent(aSibling.path) - if (!bookmarksContainer.items.find((it) => + const groupTransparentForSorting = bookmarksContainer.items.find((it) => ( + it.type === 'group' && groupNameForPath(it.title||'') === siblingName && isGroupTransparentForSorting(it.title) + )) + if (groupTransparentForSorting) { + // got a group transparent for sorting + groupTransparentForSorting.title = groupNameForPath(groupTransparentForSorting.title||'') + } else if (!bookmarksContainer.items.find((it) => ((it.type === 'folder' || it.type === 'file') && it.path === aSibling.path) || (it.type === 'group' && it.title === siblingName))) { const newEntry: BookmarkedItem = (aSibling instanceof TFolder) ? createBookmarkGroupEntry(siblingName) : createBookmarkFileEntry(aSibling.path); @@ -152,6 +188,46 @@ class BookmarksPluginWrapper implements BookmarksPluginInterface { } } + unbookmarkSiblings = (siblings: Array) => { + if (siblings.length === 0) return // for sanity + + const bookmarksContainer: BookmarkedParentFolder|undefined = findGroupForItemPathInBookmarks( + siblings[0].path, + DontCreateIfMissing, + this.plugin!, + this.groupNameForSorting + ) + + if (bookmarksContainer) { // for sanity + const bookmarkedItemsToRemove: Array = [] + siblings.forEach((aSibling) => { + const siblingName = lastPathComponent(aSibling.path) + const aGroup = bookmarksContainer.items.find( + (it) => (it.type === 'group' && groupNameForPath(it.title||'') === siblingName) + ) + if (aGroup) { + const canBeRemoved = bookmarkedGroupEmptyOrOnlyTransparentForSortingDescendants(aGroup as BookmarkedGroup) + if (canBeRemoved) { + bookmarksContainer.items.remove(aGroup) + cleanupBookmarkTreeFromTransparentEmptyGroups(bookmarksContainer, this.plugin!, this.groupNameForSorting) + } else { + if (!isGroupTransparentForSorting(aGroup.title)) { + aGroup.title = groupNameTransparentForSorting(aGroup.title||'') + } + } + } else { + const aFileOrFolder = bookmarksContainer.items.find( + (it) => ((it.type === 'folder' || it.type === 'file') && it.path === aSibling.path) + ) + if (aFileOrFolder) { + bookmarksContainer.items.remove(aFileOrFolder) + cleanupBookmarkTreeFromTransparentEmptyGroups(bookmarksContainer, this.plugin!, this.groupNameForSorting) + } + } + }); + } + } + updateSortingBookmarksAfterItemRenamed = (renamedItem: TAbstractFile, oldPath: string): void => { updateSortingBookmarksAfterItemRenamed(this.plugin!, renamedItem, oldPath, this.groupNameForSorting) } @@ -177,7 +253,6 @@ class BookmarksPluginWrapper implements BookmarksPluginInterface { } bookmarksIncludeItemsInFolder = (folderPath: string): boolean => { - console.error(`C: for ${folderPath} is ${bookmarksFoldersCoverage?.[folderPath]}`) return !! bookmarksFoldersCoverage?.[folderPath] } } @@ -190,7 +265,8 @@ export const getBookmarksPlugin = (bookmarksGroupName?: string, forceFlushCache? if (installedBookmarksPlugin && installedBookmarksPlugin.enabled && installedBookmarksPlugin.instance) { const bookmarksPluginInstance: Bookmarks_PluginInstance = installedBookmarksPlugin.instance as Bookmarks_PluginInstance // defensive programming, in case Obsidian changes its internal APIs - if (typeof bookmarksPluginInstance?.[BookmarksPlugin_getBookmarks_methodName] === 'function') { + if (typeof bookmarksPluginInstance?.[BookmarksPlugin_getBookmarks_methodName] === 'function' && + Array.isArray(bookmarksPluginInstance?.[BookmarksPlugin_items_collectionName])) { bookmarksPlugin.plugin = bookmarksPluginInstance bookmarksPlugin.groupNameForSorting = bookmarksGroupName if (ensureCachePopulated && !bookmarksCache) { @@ -231,11 +307,16 @@ const invalidateExpiredBookmarksCache = (force?: boolean): void => { type TraverseCallback = (item: BookmarkedItem, parentsGroupsPath: string) => boolean | void const traverseBookmarksCollection = (items: Array, callbackConsumeItem: TraverseCallback) => { + if (!Array.isArray(items)) return const recursiveTraversal = (collection: Array, groupsPath: string) => { + if (!Array.isArray(collection)) return for (let idx = 0, collectionRef = collection; idx < collectionRef.length; idx++) { const item = collectionRef[idx]; if (callbackConsumeItem(item, groupsPath)) return; - if ('group' === item.type) recursiveTraversal(item.items, `${groupsPath}${groupsPath?'/':''}${item.title}`); + if ('group' === item.type) { + const groupNameToUseInPath: string = groupNameForPath(item.title || '') + recursiveTraversal(item.items, `${groupsPath}${groupsPath?'/':''}${groupNameToUseInPath}`); + } } }; recursiveTraversal(items, ''); @@ -243,69 +324,94 @@ const traverseBookmarksCollection = (items: Array, callbackConsu const ARTIFICIAL_ANCHOR_SORTING_BOOKMARK_INDICATOR = '#^-' -const bookmarkLocationAndPathOverlap = (bookmarkParentGroupPath: string, fileOrFolderPath: string): number => { - return fileOrFolderPath?.startsWith(bookmarkParentGroupPath) ? bookmarkParentGroupPath.length : 0 +const ROOT_FOLDER_PATH = '/' + +const TRANSPARENT_FOR_SORTING_PREFIX = '\\\\' + +const isGroupTransparentForSorting = (name?: string): boolean => { + return !!name?.startsWith(TRANSPARENT_FOR_SORTING_PREFIX) } -const ROOT_FOLDER_PATH = '/' +const groupNameTransparentForSorting = (name: string): string => { + return isGroupTransparentForSorting(name) ? name : `${TRANSPARENT_FOR_SORTING_PREFIX}${name}` +} + +export const groupNameForPath = (name: string): string => { + return isGroupTransparentForSorting(name) ? name.substring(TRANSPARENT_FOR_SORTING_PREFIX.length) : name +} const getOrderedBookmarks = (plugin: Bookmarks_PluginInstance, bookmarksGroupName?: string): [OrderedBookmarks, FoldersCoverage] | [undefined, undefined] => { - console.log(`getOrderedBookmarks()`) let bookmarksItems: Array | undefined = plugin?.[BookmarksPlugin_items_collectionName] let bookmarksCoveredFolders: FoldersCoverage = {} - if (bookmarksItems) { + if (bookmarksItems && Array.isArray(bookmarksItems)) { if (bookmarksGroupName) { - // scanning only top level items because by desing the bookmarks group for sorting is a top level item + // scanning only top level items because by design the bookmarks group for sorting is a top level item const bookmarksGroup: BookmarkedGroup|undefined = bookmarksItems.find( (item) => item.type === 'group' && item.title === bookmarksGroupName ) as BookmarkedGroup bookmarksItems = bookmarksGroup ? bookmarksGroup.items : undefined } if (bookmarksItems) { - const orderedBookmarks: OrderedBookmarks = {} - let order: number = 0 + const orderedBookmarksWithMetadata: OrderedBookmarksWithMetadata = {} + let order: number = 1 // Intentionally start > 0 to allow easy check: if (order) ... const consumeItem = (item: BookmarkedItem, parentGroupsPath: string) => { - const isFile: boolean = item.type === 'file' - const hasSortspecAnchor: boolean = isFile && (item as BookmarkedFile).subpath === ARTIFICIAL_ANCHOR_SORTING_BOOKMARK_INDICATOR - const isFolder: boolean = item.type === 'folder' - const isGroup: boolean = item.type === 'group' - if ((isFile && hasSortspecAnchor) || isFolder || isGroup) { - const pathOfGroup: string = `${parentGroupsPath}${parentGroupsPath?'/':''}${item.title}` - const path = isGroup ? pathOfGroup : (item as BookmarkWithPath).path - const alreadyConsumed = orderedBookmarks[path] - - const parentFolderPathOfBookmarkedItem = isGroup ? parentGroupsPath : extractParentFolderPath(path) - console.log(`Add ${path}`) - bookmarksCoveredFolders[parentFolderPathOfBookmarkedItem.length > 0 ? parentFolderPathOfBookmarkedItem : ROOT_FOLDER_PATH] = true - console.log(bookmarksCoveredFolders) - // for groups (they represent folders from sorting perspective) bookmark them unconditionally - // the idea of better match is not applicable - if (alreadyConsumed && isGroup && alreadyConsumed.group) { - return - } + if ('group' === item.type) { + if (!isGroupTransparentForSorting(item.title)) { + const path: string = `${parentGroupsPath}${parentGroupsPath ? '/' : ''}${item.title}` + const alreadyConsumed = orderedBookmarksWithMetadata[path] + if (alreadyConsumed) { + if (alreadyConsumed.isGroup) return // Defensive programming + if (alreadyConsumed.hasSortingIndicator) return + } - // for files and folders (folder can be only manually bookmarked, the plugin uses groups to represent folders) - // the most closely matching location in bookmarks hierarchy is preferred - let pathOverlapLength: number|undefined - if (alreadyConsumed && (isFile || isFolder)) { - pathOverlapLength = bookmarkLocationAndPathOverlap(parentGroupsPath, path) - if (pathOverlapLength <= alreadyConsumed.bookmarkPathOverlap) { - return + orderedBookmarksWithMetadata[path] = { + path: path, + order: order++, + isGroup: true + } + } + } else if ('file' === item.type || 'folder' === item.type) { + const itemWithPath = (item as BookmarkWithPath) + const itemFile = 'file' === item.type ? (item as BookmarkedFile) : undefined + const alreadyConsumed = orderedBookmarksWithMetadata[itemWithPath.path] + const hasSortingIndicator: boolean|undefined = itemFile ? itemFile.subpath === ARTIFICIAL_ANCHOR_SORTING_BOOKMARK_INDICATOR : undefined + const parentFolderPathOfBookmarkedItem = extractParentFolderPath(itemWithPath.path) + const bookmarkPathMatches: boolean = parentGroupsPath === parentFolderPathOfBookmarkedItem + const bookmarkPathIsRoot: boolean = !(parentGroupsPath?.length > 0) + + // Bookmarks not in root (group) or in matching path are ignored + if (!bookmarkPathMatches && !bookmarkPathIsRoot) return + + // For bookmarks in root or in matching path, apply the prioritized duplicate elimination logic + if (alreadyConsumed) { + if (hasSortingIndicator) { + if (alreadyConsumed.hasSortingIndicator && alreadyConsumed.bookmarkPathMatches) return + if (alreadyConsumed.hasSortingIndicator && !bookmarkPathMatches) return + } else { // no sorting indicator on new + if (alreadyConsumed.hasSortingIndicator) return + if (!bookmarkPathMatches || alreadyConsumed.bookmarkPathMatches || alreadyConsumed.isGroup) return } } - orderedBookmarks[path] = { - path: path, + orderedBookmarksWithMetadata[itemWithPath.path] = { + path: itemWithPath.path, order: order++, - file: isFile, - folder: isFile, - group: isGroup, - bookmarkPathOverlap: isGroup || (pathOverlapLength ?? bookmarkLocationAndPathOverlap(parentGroupsPath, path)) + isGroup: false, + bookmarkPathMatches: bookmarkPathMatches, + hasSortingIndicator: hasSortingIndicator } } } traverseBookmarksCollection(bookmarksItems, consumeItem) + + const orderedBookmarks: OrderedBookmarks = {} + + for (let path in orderedBookmarksWithMetadata) { + orderedBookmarks[path] = orderedBookmarksWithMetadata[path].order + const parentFolderPath: Path = extractParentFolderPath(path) + bookmarksCoveredFolders[parentFolderPath.length > 0 ? parentFolderPath : ROOT_FOLDER_PATH] = true + } return [orderedBookmarks, bookmarksCoveredFolders] } } @@ -322,18 +428,16 @@ const createBookmarkGroupEntry = (title: string): BookmarkedGroup => { return { type: "group", ctime: Date.now(), items: [], title: title } } -interface BookmarkedParentFolder { +export interface BookmarkedParentFolder { + pathOfGroup?: Path // undefined when the container is the root of bookmarks group?: BookmarkedGroup // undefined when the item is at root level of bookmarks items: Array // reference to group.items or to root collection of bookmarks } -interface ItemInBookmarks { - parentItemsCollection: Array - item: BookmarkedItem -} - const findGroupForItemPathInBookmarks = (itemPath: string, createIfMissing: boolean, plugin: Bookmarks_PluginInstance, bookmarksGroup?: string): BookmarkedParentFolder|undefined => { - let items = plugin[BookmarksPlugin_items_collectionName] + let items = plugin?.[BookmarksPlugin_items_collectionName] + + if (!Array.isArray(items)) return undefined if (!itemPath || !itemPath.trim()) return undefined // for sanity @@ -347,11 +451,13 @@ const findGroupForItemPathInBookmarks = (itemPath: string, createIfMissing: bool let group: BookmarkedGroup|undefined = undefined - parentPathComponents.forEach((pathSegment) => { - let group: BookmarkedGroup|undefined = items.find((it) => it.type === 'group' && it.title === pathSegment) as BookmarkedGroup + parentPathComponents.forEach((pathSegment, index) => { + group = items.find((it) => it.type === 'group' && groupNameForPath(it.title||'') === pathSegment) as BookmarkedGroup if (!group) { if (createIfMissing) { - group = createBookmarkGroupEntry(pathSegment) + const theSortingBookmarksContainerGroup = (bookmarksGroup && index === 0) + const groupName: string = theSortingBookmarksContainerGroup ? pathSegment : groupNameTransparentForSorting(pathSegment) + group = createBookmarkGroupEntry(groupName) items.push(group) } else { return undefined @@ -363,19 +469,50 @@ const findGroupForItemPathInBookmarks = (itemPath: string, createIfMissing: bool return { items: items, - group: group + group: group, + pathOfGroup: parentPath } } const CreateIfMissing = true const DontCreateIfMissing = false +const renameGroup = (group: BookmarkedGroup, newName: string, makeTransparentForSorting: boolean|undefined) => { + if (makeTransparentForSorting === true) { + group.title = groupNameTransparentForSorting(newName) + } else if (makeTransparentForSorting === false) { + group.title = newName + } else { // no transparency status, retain the status as-is + group.title = isGroupTransparentForSorting(group.title) ? groupNameTransparentForSorting(newName) : newName + } +} + +const cleanupBookmarkTreeFromTransparentEmptyGroups = (parentGroup: BookmarkedParentFolder|undefined, plugin: Bookmarks_PluginInstance, bookmarksGroup?: string) => { + + if (!parentGroup) return // invalid invocation - exit + if (!parentGroup.group) return // root folder of the bookmarks - do not touch items in root folder + + if (checkSubtreeForOnlyTransparentGroups(parentGroup.items)) { + parentGroup.group.items = [] + + const parentContainerOfGroup = findGroupForItemPathInBookmarks( + parentGroup.pathOfGroup || '', + DontCreateIfMissing, + plugin, + bookmarksGroup + ) + if (parentContainerOfGroup) { + parentContainerOfGroup.group?.items?.remove(parentGroup.group) + cleanupBookmarkTreeFromTransparentEmptyGroups(parentContainerOfGroup, plugin, bookmarksGroup) + } + } +} + const updateSortingBookmarksAfterItemRenamed = (plugin: Bookmarks_PluginInstance, renamedItem: TAbstractFile, oldPath: string, bookmarksGroup?: string) => { if (renamedItem.path === oldPath) return; // sanity const aFolder: boolean = renamedItem instanceof TFolder - const aFolderWithChildren: boolean = aFolder && (renamedItem as TFolder).children.length > 0 const aFile: boolean = !aFolder const oldParentPath: string = extractParentFolderPath(oldPath) const oldName: string = lastPathComponent(oldPath) @@ -384,35 +521,58 @@ const updateSortingBookmarksAfterItemRenamed = (plugin: Bookmarks_PluginInstance const moved: boolean = oldParentPath !== newParentPath const renamed: boolean = oldName !== newName + // file renames are handled automatically by Obsidian in bookmarks, no need for additional actions + if (aFile && renamed) return + const originalContainer: BookmarkedParentFolder|undefined = findGroupForItemPathInBookmarks(oldPath, DontCreateIfMissing, plugin, bookmarksGroup) if (!originalContainer) return; - const item: BookmarkedItem|undefined = originalContainer.items.find((it) => { - if (aFolder && it.type === 'group' && it.title === oldName) return true; - if (aFile && it.type === 'file' && it.path === oldPath) return true; - }) + const item: BookmarkedItem|undefined = aFolder ? + originalContainer.items.find((it) => ( + it.type === 'group' && groupNameForPath(it.title||'') === oldName + )) + : // aFile + originalContainer.items.find((it) => ( + it.type === 'file' && it.path === renamedItem.path + )) if (!item) return; - // The renamed/moved item was located in bookmarks, apply the necessary bookmarks updates - - let itemRemovedFromBookmarks: boolean = false - if (moved) { - originalContainer.items.remove(item) - const createTargetLocation: boolean = aFolderWithChildren - const targetContainer: BookmarkedParentFolder|undefined = findGroupForItemPathInBookmarks(renamedItem.path, createTargetLocation, plugin, bookmarksGroup) - if (targetContainer) { - targetContainer.items.push(item) - } else { - itemRemovedFromBookmarks = true // open question: remove from bookmarks indeed, if target location was not under bookmarks control? + // The renamed/moved item was located in bookmarks, actions depend on item type + if (aFile) { + if (moved) { // sanity + originalContainer.group?.items.remove(item) + cleanupBookmarkTreeFromTransparentEmptyGroups(originalContainer, plugin, bookmarksGroup) } - } + } else { // a group + const aGroup: BookmarkedGroup = item as BookmarkedGroup + + if (bookmarkedGroupEmptyOrOnlyTransparentForSortingDescendants(aGroup)) { + if (moved) { // sanity + originalContainer.group?.items.remove(aGroup) + cleanupBookmarkTreeFromTransparentEmptyGroups(originalContainer, plugin, bookmarksGroup) + } else if (renamed) { + renameGroup(aGroup, newName, undefined) + } + } else { // group has some descendants not transparent for sorting + if (moved) { + originalContainer.group?.items.remove(aGroup) + const targetContainer: BookmarkedParentFolder | undefined = findGroupForItemPathInBookmarks(renamedItem.path, CreateIfMissing, plugin, bookmarksGroup) + if (targetContainer) { + targetContainer.group?.items.push(aGroup) + // the group in new location becomes by design transparent for sorting. + // The sorting order is a property of the parent folder, not the item itself + renameGroup(aGroup, groupNameForPath(aGroup.title||''), true) + } + cleanupBookmarkTreeFromTransparentEmptyGroups(originalContainer, plugin, bookmarksGroup) + } - if (aFolder && renamed && !itemRemovedFromBookmarks) { - // Renames of files are handled automatically by Bookmarks core plugin, only need to handle folder rename - // because folders are represented (for sorting purposes) by groups with exact name - (item as BookmarkedGroup).title = newName + if (renamed) { + // unrealistic scenario when a folder is moved and renamed at the same time + renameGroup(aGroup, newName, undefined) + } + } } } @@ -422,19 +582,42 @@ const updateSortingBookmarksAfterItemDeleted = (plugin: Bookmarks_PluginInstance if (deletedItem instanceof TFile) return; let items = plugin[BookmarksPlugin_items_collectionName] - const aFolder: boolean = deletedItem instanceof TFolder - const aFile: boolean = !aFolder - const originalContainer: BookmarkedParentFolder|undefined = findGroupForItemPathInBookmarks(deletedItem.path, DontCreateIfMissing, plugin, bookmarksGroup) + if (!Array.isArray(items)) return - if (!originalContainer) return; + const aFolder: boolean = deletedItem instanceof TFolder + const aFile: boolean = !aFolder - const item: BookmarkedItem|undefined = originalContainer.items.find((it) => { - if (aFolder && it.type === 'group' && it.title === deletedItem.name) return true; - if (aFile && it.type === 'file' && it.path === deletedItem.path) return true; + // Delete all instances of deleted item from two handled locations: + // - in bookmark groups hierarchy matching the item path in file explorer + // - in the bookmark group designated as container for bookmarks (immediate children) + const bookmarksContainer: BookmarkedParentFolder|undefined = findGroupForItemPathInBookmarks(deletedItem.path, DontCreateIfMissing, plugin, bookmarksGroup) + const itemInRootFolder = !!extractParentFolderPath(deletedItem.path) + const bookmarksRootContainer: BookmarkedParentFolder|undefined = + (bookmarksGroup && !itemInRootFolder) ? findGroupForItemPathInBookmarks('intentionally-in-root-path', DontCreateIfMissing, plugin, bookmarksGroup) : undefined + + if (!bookmarksContainer && !bookmarksRootContainer) return; + + [bookmarksContainer, bookmarksRootContainer].forEach((container) => { + const bookmarkEntriesToRemove: Array = [] + container?.items.forEach((it) => { + if (aFolder && it.type === 'group' && groupNameForPath(it.title||'') === deletedItem.name) { + bookmarkEntriesToRemove.push(it) + } + if (aFile && it.type === 'file' && it.path === deletedItem.path) { + bookmarkEntriesToRemove.push(it) + } + }) + bookmarkEntriesToRemove.forEach((itemToRemove) =>{ + container?.group?.items.remove(itemToRemove) + }) + cleanupBookmarkTreeFromTransparentEmptyGroups(container, plugin, bookmarksGroup) }) +} - if (!item) return; - - originalContainer.items.remove(item) +export const _unitTests = { + getOrderedBookmarks: getOrderedBookmarks, + bookmarkedGroupEmptyOrOnlyTransparentForSortingDescendants: bookmarkedGroupEmptyOrOnlyTransparentForSortingDescendants, + cleanupBookmarkTreeFromTransparentEmptyGroups: cleanupBookmarkTreeFromTransparentEmptyGroups, + findGroupForItemPathInBookmarks: findGroupForItemPathInBookmarks } diff --git a/yarn.lock b/yarn.lock index 4a78e366..ba84ad8a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -763,6 +763,13 @@ dependencies: "@types/tern" "*" +"@types/codemirror@5.60.8": + version "5.60.8" + resolved "https://registry.yarnpkg.com/@types/codemirror/-/codemirror-5.60.8.tgz#b647d04b470e8e1836dd84b2879988fc55c9de68" + integrity sha512-VjFgDF/eB+Aklcy15TtOTLQeMjTo07k7KAjql8OK5Dirr7a6sJY4T1uVBDuTVG9VEmn1uUsohOpYnVfgC6/jyw== + dependencies: + "@types/tern" "*" + "@types/estree@*": version "1.0.0" resolved "https://registry.yarnpkg.com/@types/estree/-/estree-1.0.0.tgz#5fb2e536c1ae9bf35366eed879e827fa59ca41c2" @@ -2396,6 +2403,14 @@ npm-run-path@^4.0.1: dependencies: path-key "^3.0.0" +"obsidian-1.4.11@npm:obsidian@1.4.11": + version "1.4.11" + resolved "https://registry.yarnpkg.com/obsidian/-/obsidian-1.4.11.tgz#5cba594c83a74ebad58b630c610265018abdadaa" + integrity sha512-BCVYTvaXxElJMl6MMbDdY/CGK+aq18SdtDY/7vH8v6BxCBQ6KF4kKxL0vG9UZ0o5qh139KpUoJHNm+6O5dllKA== + dependencies: + "@types/codemirror" "5.60.8" + moment "2.29.4" + obsidian@^0.15.4: version "0.15.9" resolved "https://registry.yarnpkg.com/obsidian/-/obsidian-0.15.9.tgz#b6e0b566952643db6b55f7e74fbba6d7140fe4a2"