From b1f36156d19a042d42d6a30ff6b5cd07648138f0 Mon Sep 17 00:00:00 2001 From: SebastianMC <23032356+SebastianMC@users.noreply.github.com> Date: Thu, 24 Aug 2023 00:28:24 +0200 Subject: [PATCH 1/2] Bugfix in sorterByMetadataField - reverse order working correctly now - new unit tests --- src/custom-sort/custom-sort.spec.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/custom-sort/custom-sort.spec.ts b/src/custom-sort/custom-sort.spec.ts index b5ba6c6..7406c57 100644 --- a/src/custom-sort/custom-sort.spec.ts +++ b/src/custom-sort/custom-sort.spec.ts @@ -2254,6 +2254,25 @@ describe('CustomSortOrder.byMetadataFieldAlphabeticalReverse', () => { expect(result1).toBe(SORT_FIRST_GOES_EARLIER) expect(result2).toBe(SORT_FIRST_GOES_LATER) }) + it('should put the item with metadata later if the second one has no metadata (reverse order)', () => { + // given + const itemA: Partial = { + metadataFieldValue: '15', + sortString: 'n123' + } + const itemB: Partial = { + sortString: 'n123' + } + const sorter: SorterFn = Sorters[CustomSortOrder.byMetadataFieldAlphabeticalReverse] + + // when + const result1: number = sorter(itemA as FolderItemForSorting, itemB as FolderItemForSorting) + const result2: number = sorter(itemB as FolderItemForSorting, itemA as FolderItemForSorting) + + // then + expect(result1).toBe(SORT_FIRST_GOES_LATER) + expect(result2).toBe(SORT_FIRST_GOES_EARLIER) + }) it('should correctly fallback to alphabetical reverse if no metadata on both items', () => { // given const itemA: Partial = { From 45f591859889d0e68454cdcdeee68a1c5d1b112a Mon Sep 17 00:00:00 2001 From: SebastianMC <23032356+SebastianMC@users.noreply.github.com> Date: Thu, 24 Aug 2023 01:11:22 +0200 Subject: [PATCH 2/2] Major improvement: added support for determining and applying sort order currently selected in Obsidian UI - the meaning of CustomSortOrder.standardObsidian changes from a fixed one to what is actually selected in Obsidian UI - the CustomSortOrder.standardObsidian can be applied at a folder level (as the default for folder) and at a group level (this is a major addition) - added a mapping of Obsidian UI sorting methods onto internal plugin sorting methods, plus addition of the Obsidian UI logic to push folders to the top unconditionally - !!! NO NEW UNIT TESTS FOR THIS FEATURE - must add later - not tested manually, as the commits extraction and pushing is done as part of #88 github issue --- src/custom-sort/custom-sort.spec.ts | 1 + src/custom-sort/custom-sort.ts | 106 ++++++++++++++---- .../sorting-spec-processor.spec.ts | 13 ++- src/custom-sort/sorting-spec-processor.ts | 6 +- src/main.ts | 36 +++--- src/types/types.d.ts | 2 + src/utils/utils.spec.ts | 24 ++++ src/utils/utils.ts | 10 ++ 8 files changed, 156 insertions(+), 42 deletions(-) create mode 100644 src/utils/utils.spec.ts diff --git a/src/custom-sort/custom-sort.spec.ts b/src/custom-sort/custom-sort.spec.ts index 0fec06a..2890f5f 100644 --- a/src/custom-sort/custom-sort.spec.ts +++ b/src/custom-sort/custom-sort.spec.ts @@ -8,6 +8,7 @@ import { matchGroupRegex, sorterByMetadataField, SorterFn, + getSorterFnFor, Sorters } from './custom-sort'; import {CustomSortGroupType, CustomSortOrder, CustomSortSpec, RegExpSpec} from './custom-sort-types'; diff --git a/src/custom-sort/custom-sort.ts b/src/custom-sort/custom-sort.ts index 98b8b26..0e4847c 100644 --- a/src/custom-sort/custom-sort.ts +++ b/src/custom-sort/custom-sort.ts @@ -64,6 +64,8 @@ export interface FolderItemForSorting { } export type SorterFn = (a: FolderItemForSorting, b: FolderItemForSorting) => number +export type PlainSorterFn = (a: TAbstractFile, b: TAbstractFile) => number +export type PlainFileOnlySorterFn = (a: TFile, b: TFile) => number export type CollatorCompareFn = (a: string, b: string) => number // Syntax sugar @@ -112,29 +114,95 @@ export let Sorters: { [key in CustomSortOrder]: SorterFn } = { [CustomSortOrder.byMetadataFieldAlphabeticalReverse]: sorterByMetadataField(ReverseOrder), [CustomSortOrder.byMetadataFieldTrueAlphabeticalReverse]: sorterByMetadataField(ReverseOrder, TrueAlphabetical), - // This is a fallback entry which should not be used - the plugin code should refrain from custom sorting at all + // This is a fallback entry which should not be used - the getSorterFor() function below should protect against it [CustomSortOrder.standardObsidian]: (a: FolderItemForSorting, b: FolderItemForSorting) => CollatorCompare(a.sortString, b.sortString), }; -function compareTwoItems(itA: FolderItemForSorting, itB: FolderItemForSorting, sortSpec: CustomSortSpec) { - if (itA.groupIdx != undefined && itB.groupIdx != undefined) { - if (itA.groupIdx === itB.groupIdx) { - const group: CustomSortGroup | undefined = sortSpec.groups[itA.groupIdx] - const matchingGroupPresentOnBothSidesAndEqual: boolean = itA.matchGroup !== undefined && itA.matchGroup === itB.matchGroup - if (matchingGroupPresentOnBothSidesAndEqual && group.secondaryOrder) { - return Sorters[group.secondaryOrder ?? CustomSortOrder.default](itA, itB) +// OS - Obsidian Sort +const OS_alphabetical = 'alphabetical' +const OS_alphabeticalReverse = 'alphabeticalReverse' +const OS_byModifiedTime = 'byModifiedTime' +const OS_byModifiedTimeReverse = 'byModifiedTimeReverse' +const OS_byCreatedTime = 'byCreatedTime' +const OS_byCreatedTimeReverse = 'byCreatedTimeReverse' + +export const ObsidianStandardDefaultSortingName = OS_alphabetical + +const StandardObsidianToCustomSort: {[key: string]: CustomSortOrder} = { + [OS_alphabetical]: CustomSortOrder.alphabetical, + [OS_alphabeticalReverse]: CustomSortOrder.alphabeticalReverse, + [OS_byModifiedTime]: CustomSortOrder.byModifiedTimeReverse, // In Obsidian labeled as 'Modified time (new to old)' + [OS_byModifiedTimeReverse]: CustomSortOrder.byModifiedTime, // In Obsidian labeled as 'Modified time (old to new)' + [OS_byCreatedTime]: CustomSortOrder.byCreatedTimeReverse, // In Obsidian labeled as 'Created time (new to old)' + [OS_byCreatedTimeReverse]: CustomSortOrder.byCreatedTime // In Obsidian labeled as 'Created time (old to new)' +} + +const StandardObsidianToPlainSortFn: {[key: string]: PlainFileOnlySorterFn} = { + [OS_alphabetical]: (a: TFile, b: TFile) => CollatorCompare(a.basename, b.basename), + [OS_alphabeticalReverse]: (a: TFile, b: TFile) => -StandardObsidianToPlainSortFn[OS_alphabetical](a,b), + [OS_byModifiedTime]: (a: TFile, b: TFile) => b.stat.mtime - a.stat.mtime, + [OS_byModifiedTimeReverse]: (a: TFile, b: TFile) => -StandardObsidianToPlainSortFn[OS_byModifiedTime](a,b), + [OS_byCreatedTime]: (a: TFile, b: TFile) => b.stat.ctime - a.stat.ctime, + [OS_byCreatedTimeReverse]: (a: TFile, b: TFile) => -StandardObsidianToPlainSortFn[OS_byCreatedTime](a,b) +} + +// Standard Obsidian comparator keeps folders in the top sorted alphabetically +const StandardObsidianComparator = (order: CustomSortOrder): SorterFn => { + const customSorterFn = Sorters[order] + return (a: FolderItemForSorting, b: FolderItemForSorting): number => { + return a.isFolder || b.isFolder + ? + (a.isFolder && !b.isFolder ? -1 : (b.isFolder && !a.isFolder ? 1 : Sorters[CustomSortOrder.alphabetical](a,b))) + : + customSorterFn(a, b); + } +} + +// Equivalent of StandardObsidianComparator working directly on TAbstractFile items +export const StandardPlainObsidianComparator = (order: string): PlainSorterFn => { + const fileSorterFn = StandardObsidianToPlainSortFn[order] || StandardObsidianToCustomSort[OS_alphabetical] + return (a: TAbstractFile, b: TAbstractFile): number => { + const aIsFolder: boolean = a instanceof TFolder + const bIsFolder: boolean = b instanceof TFolder + return aIsFolder || bIsFolder + ? + (aIsFolder && !bIsFolder ? -1 : (bIsFolder && !aIsFolder ? 1 : CollatorCompare(a.name,b.name))) + : + fileSorterFn(a as TFile, b as TFile); + } +} + +export const getSorterFnFor = (sorting: CustomSortOrder, currentUIselectedSorting?: string): SorterFn => { + if (sorting === CustomSortOrder.standardObsidian) { + sorting = StandardObsidianToCustomSort[currentUIselectedSorting ?? 'alphabetical'] ?? CustomSortOrder.alphabetical + return StandardObsidianComparator(sorting) + } else { + return Sorters[sorting] + } +} + +function getComparator(sortSpec: CustomSortSpec, currentUIselectedSorting?: string): SorterFn { + const compareTwoItems = (itA: FolderItemForSorting, itB: FolderItemForSorting) => { + if (itA.groupIdx != undefined && itB.groupIdx != undefined) { + if (itA.groupIdx === itB.groupIdx) { + const group: CustomSortGroup | undefined = sortSpec.groups[itA.groupIdx] + const matchingGroupPresentOnBothSidesAndEqual: boolean = itA.matchGroup !== undefined && itA.matchGroup === itB.matchGroup + if (matchingGroupPresentOnBothSidesAndEqual && group.secondaryOrder) { + return getSorterFnFor(group.secondaryOrder ?? CustomSortOrder.default, currentUIselectedSorting)(itA, itB) + } else { + return getSorterFnFor(group?.order ?? CustomSortOrder.default, currentUIselectedSorting)(itA, itB) + } } else { - return Sorters[group?.order ?? CustomSortOrder.default](itA, itB) + return itA.groupIdx - itB.groupIdx; } } else { - return itA.groupIdx - itB.groupIdx; + // should never happen - groupIdx is not known for at least one of items to compare. + // The logic of determining the index always sets some idx + // Yet for sanity and to satisfy TS code analyzer a fallback to default behavior below + return getSorterFnFor(CustomSortOrder.default, currentUIselectedSorting)(itA, itB) } - } else { - // should never happen - groupIdx is not known for at least one of items to compare. - // The logic of determining the index always sets some idx - // Yet for sanity and to satisfy TS code analyzer a fallback to default behavior below - return Sorters[CustomSortOrder.default](itA, itB) } + return compareTwoItems } const isFolder = (entry: TAbstractFile) => { @@ -270,7 +338,7 @@ export const determineSortingGroup = function (entry: TFile | TFolder, spec: Cus break case CustomSortGroupType.StarredOnly: if (ctx?.starredPluginInstance) { - let starred: boolean = determineStarredStatusOf(entry, aFile, ctx.starredPluginInstance) + const starred: boolean = determineStarredStatusOf(entry, aFile, ctx.starredPluginInstance) if (starred) { determined = true } @@ -458,9 +526,9 @@ export const folderSort = function (sortingSpec: CustomSortSpec, order: string[] // Finally, for advanced sorting by modified date, for some folders the modified date has to be determined determineFolderDatesIfNeeded(folderItems, sortingSpec) - folderItems.sort(function (itA: FolderItemForSorting, itB: FolderItemForSorting) { - return compareTwoItems(itA, itB, sortingSpec); - }); + const comparator: SorterFn = getComparator(sortingSpec, fileExplorer.sortOrder) + + folderItems.sort(comparator) const items = folderItems .map((item: FolderItemForSorting) => fileExplorer.fileItems[item.path]) diff --git a/src/custom-sort/sorting-spec-processor.spec.ts b/src/custom-sort/sorting-spec-processor.spec.ts index f774270..a2785df 100644 --- a/src/custom-sort/sorting-spec-processor.spec.ts +++ b/src/custom-sort/sorting-spec-processor.spec.ts @@ -489,16 +489,23 @@ describe('SortingSpecProcessor', () => { const txtInputStandardObsidianSortAttr: string = ` target-folder: AAA sorting: standard +/ Some folder + sorting: standard ` const expectedSortSpecForObsidianStandardSorting: { [key: string]: CustomSortSpec } = { "AAA": { defaultOrder: CustomSortOrder.standardObsidian, groups: [{ + exactText: 'Some folder', + foldersOnly: true, + order: CustomSortOrder.standardObsidian, + type: CustomSortGroupType.ExactName + }, { order: CustomSortOrder.standardObsidian, type: CustomSortGroupType.Outsiders }], - outsidersGroupIdx: 0, + outsidersGroupIdx: 1, targetFoldersPaths: ['AAA'] } } @@ -1649,11 +1656,13 @@ const txtInputErrorTooManyNumericSortSymbols: string = ` % Chapter\\R+ ... page\\d+ ` +/* No longer applicable const txtInputErrorNestedStandardObsidianSortAttr: string = ` target-folder: AAA / Some folder sorting: standard ` +*/ const txtInputErrorPriorityEmptyFilePattern: string = ` /!! /: @@ -1754,6 +1763,7 @@ describe('SortingSpecProcessor error detection and reporting', () => { `${ERR_PREFIX} 9:TooManySortingSymbols Maximum one sorting symbol allowed per line ${ERR_SUFFIX_IN_LINE(2)}`) expect(errorsLogger).toHaveBeenNthCalledWith(2, ERR_LINE_TXT('% Chapter\\R+ ... page\\d+ ')) }) + /* Problem no longer applicable it('should recognize error: nested standard obsidian sorting attribute', () => { const inputTxtArr: Array = txtInputErrorNestedStandardObsidianSortAttr.split('\n') const result = processor.parseSortSpecFromText(inputTxtArr, 'mock-folder', 'custom-name-note.md') @@ -1763,6 +1773,7 @@ describe('SortingSpecProcessor error detection and reporting', () => { `${ERR_PREFIX} 14:StandardObsidianSortAllowedOnlyAtFolderLevel The standard Obsidian sort order is only allowed at a folder level (not nested syntax) ${ERR_SUFFIX_IN_LINE(4)}`) expect(errorsLogger).toHaveBeenNthCalledWith(2, ERR_LINE_TXT(' sorting: standard')) }) + */ it('should recognize error: priority indicator alone', () => { const inputTxtArr: Array = ` /! diff --git a/src/custom-sort/sorting-spec-processor.ts b/src/custom-sort/sorting-spec-processor.ts index 937f5ba..e5854cf 100644 --- a/src/custom-sort/sorting-spec-processor.ts +++ b/src/custom-sort/sorting-spec-processor.ts @@ -69,7 +69,7 @@ export enum ProblemCode { ItemToHideExactNameWithExtRequired, ItemToHideNoSupportForThreeDots, DuplicateWildcardSortSpecForSameFolder, - StandardObsidianSortAllowedOnlyAtFolderLevel, + ProblemNoLongerApplicable_StandardObsidianSortAllowedOnlyAtFolderLevel, // Placeholder kept to avoid refactoring of many unit tests (hardcoded error codes) PriorityNotAllowedOnOutsidersGroup, TooManyPriorityPrefixes, CombiningNotAllowedOnOutsidersGroup, @@ -971,10 +971,6 @@ export class SortingSpecProcessor { this.problem(ProblemCode.DuplicateOrderAttr, `Duplicate order specification for a sorting rule of folder ${folderPathsForProblemMsg}`) return false; } - if ((attr.value as RecognizedOrderValue).order === CustomSortOrder.standardObsidian) { - this.problem(ProblemCode.StandardObsidianSortAllowedOnlyAtFolderLevel, `The standard Obsidian sort order is only allowed at a folder level (not nested syntax)`) - return false; - } this.ctx.currentSpecGroup.order = (attr.value as RecognizedOrderValue).order this.ctx.currentSpecGroup.byMetadataField = (attr.value as RecognizedOrderValue).applyToMetadataField this.ctx.currentSpecGroup.secondaryOrder = (attr.value as RecognizedOrderValue).secondaryOrder diff --git a/src/main.ts b/src/main.ts index 62074dc..c4f9fc4 100644 --- a/src/main.ts +++ b/src/main.ts @@ -16,7 +16,9 @@ import { Vault } from 'obsidian'; import {around} from 'monkey-around'; -import {folderSort} from './custom-sort/custom-sort'; +import { + folderSort +} from './custom-sort/custom-sort'; import {SortingSpecProcessor, SortSpecsCollection} from './custom-sort/sorting-spec-processor'; import {CustomSortOrder, CustomSortSpec} from './custom-sort/custom-sort-types'; @@ -30,6 +32,8 @@ import { ICON_SORT_SUSPENDED_SYNTAX_ERROR } from "./custom-sort/icons"; +import {lastPathComponent} from "./utils/utils"; + interface CustomSortPluginSettings { additionalSortspecFile: string suspended: boolean @@ -309,6 +313,18 @@ export default class CustomSortPlugin extends Plugin { }) } + determineSortSpecForFolder(folderPath: string, folderName?: string): CustomSortSpec|null|undefined { + folderName = folderName ?? lastPathComponent(folderPath) + let sortSpec: CustomSortSpec | null | undefined = this.sortSpecCache?.sortSpecByPath?.[folderPath] + sortSpec = sortSpec ?? this.sortSpecCache?.sortSpecByName?.[folderName] + + if (!sortSpec && this.sortSpecCache?.sortSpecByWildcard) { + // when no sorting spec found directly by folder path, check for wildcard-based match + sortSpec = this.sortSpecCache?.sortSpecByWildcard.folderMatch(folderPath, folderName) + } + return sortSpec + } + // For the idea of monkey-patching credits go to https://github.com/nothingislost/obsidian-bartender patchFileExplorerFolder(patchableFileExplorer?: FileExplorerView): boolean { let plugin = this; @@ -332,23 +348,10 @@ export default class CustomSortPlugin extends Plugin { setIcon(plugin.ribbonIconEl, ICON_SORT_ENABLED_ACTIVE) } - // if custom sort is not specified, use the UI-selected const folder: TFolder = this.file - let sortSpec: CustomSortSpec | null | undefined = plugin.sortSpecCache?.sortSpecByPath?.[folder.path] - sortSpec = sortSpec ?? plugin.sortSpecCache?.sortSpecByName?.[folder.name] + let sortSpec: CustomSortSpec | null | undefined = plugin.determineSortSpecForFolder(folder.path, folder.name) + if (sortSpec) { - if (sortSpec.defaultOrder === CustomSortOrder.standardObsidian) { - sortSpec = null // A folder is explicitly excluded from custom sorting plugin - } - } else if (plugin.sortSpecCache?.sortSpecByWildcard) { - // when no sorting spec found directly by folder path, check for wildcard-based match - sortSpec = plugin.sortSpecCache?.sortSpecByWildcard.folderMatch(folder.path, folder.name) - if (sortSpec?.defaultOrder === CustomSortOrder.standardObsidian) { - sortSpec = null // A folder is explicitly excluded from custom sorting plugin - } - } - if (sortSpec) { - sortSpec.plugin = plugin return folderSort.call(this, sortSpec, ...args); } else { return old.call(this, ...args); @@ -371,7 +374,6 @@ export default class CustomSortPlugin extends Plugin { } onunload() { - } updateStatusBar() { diff --git a/src/types/types.d.ts b/src/types/types.d.ts index ef295f5..22ea304 100644 --- a/src/types/types.d.ts +++ b/src/types/types.d.ts @@ -53,5 +53,7 @@ declare module 'obsidian' { createFolderDom(folder: TFolder): FileExplorerFolder; requestSort(): void; + + sortOrder: string } } diff --git a/src/utils/utils.spec.ts b/src/utils/utils.spec.ts new file mode 100644 index 0000000..5189d85 --- /dev/null +++ b/src/utils/utils.spec.ts @@ -0,0 +1,24 @@ +import {lastPathComponent, extractParentFolderPath} from "./utils"; + +describe('lastPathComponent and extractParentFolderPath', () => { + it.each([ + ['a folder', '', 'a folder'], + ['a/subfolder', 'a', 'subfolder'], + ['parent/child', 'parent', 'child'], + ['','',''], + [' ','',''], + ['/strange', '', 'strange'], + ['a/b/c/', 'a/b/c', ''], + ['d d d/e e e/f f f/ggg ggg', 'd d d/e e e/f f f', 'ggg ggg'], + ['/','',''], + [' / ','',''], + [' /','',''], + ['/ ','',''] + ])('should from %s extract %s and %s', (path: string, parentPath: string, lastComponent: string) => { + const extractedParentPath: string = extractParentFolderPath(path) + const extractedLastComponent: string = lastPathComponent(path) + expect(extractedParentPath).toBe(parentPath) + expect(extractedLastComponent).toBe(lastComponent) + } + ) +}) diff --git a/src/utils/utils.ts b/src/utils/utils.ts index db8a783..5afece4 100644 --- a/src/utils/utils.ts +++ b/src/utils/utils.ts @@ -6,3 +6,13 @@ export function isDefined(o: any): boolean { export function last(o: Array): T | undefined { return o?.length > 0 ? o[o.length - 1] : undefined } + +export function lastPathComponent(path: string): string { + const lastPathSeparatorIdx = (path ?? '').lastIndexOf('/') + return lastPathSeparatorIdx >= 0 ? path.substring(lastPathSeparatorIdx + 1).trim() : path.trim() +} + +export function extractParentFolderPath(path: string): string { + const lastPathSeparatorIdx = (path ?? '').lastIndexOf('/') + return lastPathSeparatorIdx > 0 ? path.substring(0, lastPathSeparatorIdx).trim() : '' +}