Skip to content

Commit 0b91930

Browse files
committed
refactor: extract filename via lastIndexOf instead of split/pop
Replace split('/').pop() ?? '' with slice(lastIndexOf('/') + 1) in modelStore and nodeBookmarkStore. Equivalent across no-slash/trailing-slash/empty inputs and drops the nullish fallback branch.
1 parent 3377b8e commit 0b91930

3 files changed

Lines changed: 242 additions & 4 deletions

File tree

src/stores/modelStore.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,9 @@ export class ComfyModelDef {
6969
this.path_index = pathIndex
7070
this.file_name = name
7171
this.normalized_file_name = name.replaceAll('\\', '/')
72-
this.simplified_file_name = this.normalized_file_name.split('/').pop() ?? ''
72+
this.simplified_file_name = this.normalized_file_name.slice(
73+
this.normalized_file_name.lastIndexOf('/') + 1
74+
)
7375
if (this.simplified_file_name.endsWith('.safetensors')) {
7476
this.simplified_file_name = this.simplified_file_name.slice(
7577
0,
Lines changed: 236 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,236 @@
1+
import { createPinia, setActivePinia } from 'pinia'
2+
import { beforeEach, describe, expect, it, vi } from 'vitest'
3+
4+
import { useNodeBookmarkStore } from '@/stores/nodeBookmarkStore'
5+
import type { ComfyNodeDefImpl } from '@/stores/nodeDefStore'
6+
7+
const BOOKMARK_ID = 'Comfy.NodeLibrary.Bookmarks.V2'
8+
const CUSTOMIZATION_ID = 'Comfy.NodeLibrary.BookmarksCustomization'
9+
10+
const { settings, setSpy, nodeDefs } = vi.hoisted(() => ({
11+
settings: {} as Record<string, unknown>,
12+
setSpy: vi.fn(),
13+
nodeDefs: {} as Record<string, unknown>
14+
}))
15+
16+
vi.mock('@/platform/settings/settingStore', async () => {
17+
const { reactive } = await import('vue')
18+
const reactiveSettings = reactive(settings)
19+
setSpy.mockImplementation(async (id: string, value: unknown) => {
20+
reactiveSettings[id] = value
21+
})
22+
return {
23+
useSettingStore: () => ({
24+
get: (id: string) => reactiveSettings[id],
25+
set: setSpy
26+
})
27+
}
28+
})
29+
30+
vi.mock('@/stores/nodeDefStore', () => ({
31+
useNodeDefStore: () => ({ allNodeDefsByName: nodeDefs }),
32+
buildNodeDefTree: (defs: unknown[]) => ({ key: 'root', children: defs }),
33+
createDummyFolderNodeDef: (path: string) => ({
34+
isDummyFolder: true,
35+
nodePath: path,
36+
name: path
37+
})
38+
}))
39+
40+
type BookmarkNodeFixture = Pick<
41+
ComfyNodeDefImpl,
42+
'isDummyFolder' | 'nodePath' | 'category' | 'name'
43+
>
44+
45+
function folderNode(nodePath: string) {
46+
const node = {
47+
isDummyFolder: true,
48+
nodePath,
49+
category: nodePath.replace(/\/$/, ''),
50+
name: nodePath
51+
} satisfies BookmarkNodeFixture
52+
return node as ComfyNodeDefImpl
53+
}
54+
55+
function leafNode(name: string, nodePath = name) {
56+
const node = {
57+
isDummyFolder: false,
58+
name,
59+
nodePath,
60+
category: ''
61+
} satisfies BookmarkNodeFixture
62+
return node as ComfyNodeDefImpl
63+
}
64+
65+
beforeEach(() => {
66+
setActivePinia(createPinia())
67+
for (const key of Object.keys(settings)) delete settings[key]
68+
for (const key of Object.keys(nodeDefs)) delete nodeDefs[key]
69+
settings[BOOKMARK_ID] = []
70+
settings[CUSTOMIZATION_ID] = {}
71+
setSpy.mockClear()
72+
})
73+
74+
describe('nodeBookmarkStore', () => {
75+
it('reports isBookmarked by either nodePath or top-level name', () => {
76+
settings[BOOKMARK_ID] = ['sampling/KSampler', 'LoadImage']
77+
const store = useNodeBookmarkStore()
78+
79+
expect(store.isBookmarked(leafNode('KSampler', 'sampling/KSampler'))).toBe(
80+
true
81+
)
82+
expect(store.isBookmarked(leafNode('LoadImage'))).toBe(true)
83+
expect(store.isBookmarked(leafNode('VAEDecode'))).toBe(false)
84+
})
85+
86+
it('adds a bookmark by appending to the current list', async () => {
87+
settings[BOOKMARK_ID] = ['A']
88+
const store = useNodeBookmarkStore()
89+
90+
await store.addBookmark('B')
91+
92+
expect(setSpy).toHaveBeenCalledWith(BOOKMARK_ID, ['A', 'B'])
93+
})
94+
95+
it('toggles an un-bookmarked node by adding its name', async () => {
96+
const store = useNodeBookmarkStore()
97+
98+
await store.toggleBookmark(leafNode('KSampler'))
99+
100+
expect(setSpy).toHaveBeenCalledWith(BOOKMARK_ID, ['KSampler'])
101+
})
102+
103+
it('toggles a bookmarked node by deleting both nodePath and name', async () => {
104+
settings[BOOKMARK_ID] = ['sampling/KSampler', 'KSampler']
105+
const store = useNodeBookmarkStore()
106+
107+
await store.toggleBookmark(leafNode('KSampler', 'sampling/KSampler'))
108+
109+
expect(setSpy).toHaveBeenCalledWith(BOOKMARK_ID, ['KSampler'])
110+
expect(setSpy).toHaveBeenLastCalledWith(BOOKMARK_ID, [])
111+
expect(store.bookmarks).toEqual([])
112+
})
113+
114+
it('creates a folder under a parent and at the root', async () => {
115+
const store = useNodeBookmarkStore()
116+
117+
const rootPath = await store.addNewBookmarkFolder(undefined, 'Favorites')
118+
expect(rootPath).toBe('Favorites/')
119+
120+
const childPath = await store.addNewBookmarkFolder(
121+
folderNode('Favorites/'),
122+
'Nested'
123+
)
124+
expect(childPath).toBe('Favorites/Nested/')
125+
})
126+
127+
it('builds the bookmark tree, dropping unknown node defs', () => {
128+
nodeDefs['KSampler'] = leafNode('KSampler')
129+
settings[BOOKMARK_ID] = ['sampling/KSampler', 'sampling/Unknown', 'Folder/']
130+
const store = useNodeBookmarkStore()
131+
132+
const children = (store.bookmarkedRoot as { children: unknown[] }).children
133+
expect(children).toHaveLength(2)
134+
})
135+
136+
describe('renameBookmarkFolder', () => {
137+
it('rejects renaming a non-folder node', async () => {
138+
const store = useNodeBookmarkStore()
139+
await expect(
140+
store.renameBookmarkFolder(leafNode('KSampler'), 'New')
141+
).rejects.toThrow('Cannot rename non-folder node')
142+
})
143+
144+
it('rejects a name containing a slash', async () => {
145+
const store = useNodeBookmarkStore()
146+
await expect(
147+
store.renameBookmarkFolder(folderNode('Old/'), 'a/b')
148+
).rejects.toThrow('cannot contain')
149+
})
150+
151+
it('rejects a rename that collides with an existing folder', async () => {
152+
settings[BOOKMARK_ID] = ['Taken/']
153+
const store = useNodeBookmarkStore()
154+
await expect(
155+
store.renameBookmarkFolder(folderNode('Old/'), 'Taken')
156+
).rejects.toThrow('already exists')
157+
})
158+
159+
it('rewrites matching bookmark paths on a valid rename', async () => {
160+
settings[BOOKMARK_ID] = ['Old/', 'Old/KSampler', 'Other/Node']
161+
const store = useNodeBookmarkStore()
162+
163+
await store.renameBookmarkFolder(folderNode('Old/'), 'New')
164+
165+
expect(setSpy).toHaveBeenCalledWith(BOOKMARK_ID, [
166+
'New/',
167+
'New/KSampler',
168+
'Other/Node'
169+
])
170+
})
171+
172+
it('does nothing when the folder keeps the same path', async () => {
173+
const store = useNodeBookmarkStore()
174+
175+
await store.renameBookmarkFolder(folderNode('Old/'), 'Old')
176+
177+
expect(setSpy).not.toHaveBeenCalled()
178+
})
179+
})
180+
181+
it('deletes a folder and all its descendants', async () => {
182+
settings[BOOKMARK_ID] = ['Old/', 'Old/KSampler', 'Keep/Node']
183+
const store = useNodeBookmarkStore()
184+
185+
await store.deleteBookmarkFolder(folderNode('Old/'))
186+
187+
expect(setSpy).toHaveBeenCalledWith(BOOKMARK_ID, ['Keep/Node'])
188+
})
189+
190+
it('rejects deleting a non-folder node', async () => {
191+
const store = useNodeBookmarkStore()
192+
193+
await expect(
194+
store.deleteBookmarkFolder(leafNode('KSampler'))
195+
).rejects.toThrow('Cannot delete non-folder node')
196+
})
197+
198+
describe('updateBookmarkCustomization', () => {
199+
it('persists a non-default customization', async () => {
200+
const store = useNodeBookmarkStore()
201+
202+
await store.updateBookmarkCustomization('Folder/', {
203+
color: '#ff0000',
204+
icon: 'pi-star'
205+
})
206+
207+
expect(setSpy).toHaveBeenCalledWith(CUSTOMIZATION_ID, {
208+
'Folder/': { color: '#ff0000', icon: 'pi-star' }
209+
})
210+
})
211+
212+
it('drops attributes set to their default values', async () => {
213+
const store = useNodeBookmarkStore()
214+
215+
await store.updateBookmarkCustomization('Folder/', {
216+
color: store.defaultBookmarkColor,
217+
icon: store.defaultBookmarkIcon
218+
})
219+
220+
expect(setSpy).toHaveBeenCalledWith(CUSTOMIZATION_ID, {
221+
'Folder/': undefined
222+
})
223+
})
224+
})
225+
226+
it('renames a customization entry, moving the old key to the new one', async () => {
227+
settings[CUSTOMIZATION_ID] = { 'Old/': { color: '#abc' } }
228+
const store = useNodeBookmarkStore()
229+
230+
await store.renameBookmarkCustomization('Old/', 'New/')
231+
232+
expect(setSpy).toHaveBeenCalledWith(CUSTOMIZATION_ID, {
233+
'New/': { color: '#abc' }
234+
})
235+
})
236+
})

src/stores/nodeBookmarkStore.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,9 @@ export const useNodeBookmarkStore = defineStore('nodeBookmark', () => {
5050
.map((bookmark: string) => {
5151
if (bookmark.endsWith('/')) return createDummyFolderNodeDef(bookmark)
5252

53-
const parts = bookmark.split('/')
54-
const name = parts.pop() ?? ''
55-
const category = parts.join('/')
53+
const slashIndex = bookmark.lastIndexOf('/')
54+
const name = bookmark.slice(slashIndex + 1)
55+
const category = bookmark.slice(0, Math.max(0, slashIndex))
5656
const srcNodeDef = nodeDefStore.allNodeDefsByName[name]
5757
if (!srcNodeDef) {
5858
return null

0 commit comments

Comments
 (0)