Skip to content

Commit 15338ed

Browse files
mrubensmrubens
authored
Bring back open files and git results in mention suggestions (#1946)
1 parent 30f9d8f commit 15338ed

File tree

2 files changed

+232
-60
lines changed

2 files changed

+232
-60
lines changed

‎webview-ui/src/utils/__tests__/context-mentions.test.ts‎

Lines changed: 205 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,12 @@ describe("getContextMenuOptions", () => {
5656
label: "test.ts",
5757
description: "Source file",
5858
},
59+
{
60+
type: ContextMenuOptionType.OpenedFile,
61+
value: "src/opened.ts",
62+
label: "opened.ts",
63+
description: "Currently opened file",
64+
},
5965
{
6066
type: ContextMenuOptionType.Git,
6167
value: "abc1234",
@@ -71,6 +77,18 @@ describe("getContextMenuOptions", () => {
7177
},
7278
]
7379

80+
const mockDynamicSearchResults = [
81+
{
82+
path: "search/result1.ts",
83+
type: "file" as const,
84+
label: "result1.ts",
85+
},
86+
{
87+
path: "search/folder",
88+
type: "folder" as const,
89+
},
90+
]
91+
7492
it("should return all option types for empty query", () => {
7593
const result = getContextMenuOptions("", null, [])
7694
expect(result).toHaveLength(6)
@@ -86,9 +104,11 @@ describe("getContextMenuOptions", () => {
86104

87105
it("should filter by selected type when query is empty", () => {
88106
const result = getContextMenuOptions("", ContextMenuOptionType.File, mockQueryItems)
89-
expect(result).toHaveLength(1)
90-
expect(result[0].type).toBe(ContextMenuOptionType.File)
91-
expect(result[0].value).toBe("src/test.ts")
107+
expect(result).toHaveLength(2)
108+
expect(result.map((item) => item.type)).toContain(ContextMenuOptionType.File)
109+
expect(result.map((item) => item.type)).toContain(ContextMenuOptionType.OpenedFile)
110+
expect(result.map((item) => item.value)).toContain("src/test.ts")
111+
expect(result.map((item) => item.value)).toContain("src/opened.ts")
92112
})
93113

94114
it("should match git commands", () => {
@@ -108,6 +128,188 @@ describe("getContextMenuOptions", () => {
108128
expect(result).toHaveLength(1)
109129
expect(result[0].type).toBe(ContextMenuOptionType.NoResults)
110130
})
131+
132+
/**
133+
* Tests for the combined handling of open files, git results, and search results
134+
* Added for commit 3cd7dec78faf786e468ae4f66cef0b81a76d9075
135+
*/
136+
it("should include dynamic search results along with other matches", () => {
137+
// Add an opened file that will match the query
138+
const testItems = [
139+
...mockQueryItems,
140+
{
141+
type: ContextMenuOptionType.OpenedFile,
142+
value: "src/test-opened.ts",
143+
label: "test-opened.ts",
144+
description: "Opened test file for search test",
145+
},
146+
]
147+
148+
const result = getContextMenuOptions("test", null, testItems, mockDynamicSearchResults)
149+
150+
// Check if opened files and dynamic search results are included
151+
expect(result.some((item) => item.type === ContextMenuOptionType.OpenedFile)).toBe(true)
152+
expect(result.some((item) => item.value === "/search/result1.ts")).toBe(true)
153+
})
154+
155+
it("should maintain correct result ordering according to implementation", () => {
156+
// Add multiple item types to test ordering
157+
const result = getContextMenuOptions("t", null, mockQueryItems, mockDynamicSearchResults)
158+
159+
// Find the different result types
160+
const fileResults = result.filter(
161+
(item) =>
162+
item.type === ContextMenuOptionType.File ||
163+
item.type === ContextMenuOptionType.OpenedFile ||
164+
item.type === ContextMenuOptionType.Folder,
165+
)
166+
167+
const searchResults = result.filter(
168+
(item) => item.type === ContextMenuOptionType.File && item.value?.includes("/search/"),
169+
)
170+
171+
const gitResults = result.filter((item) => item.type === ContextMenuOptionType.Git)
172+
173+
// Find the indexes of the first item of each type in the results array
174+
const firstFileIndex = result.findIndex((item) => fileResults.some((f) => f === item))
175+
176+
const firstSearchResultIndex = result.findIndex((item) => searchResults.some((s) => s === item))
177+
178+
const firstGitResultIndex = result.findIndex((item) => gitResults.some((g) => g === item))
179+
180+
// Verify file results come before search results
181+
expect(firstFileIndex).toBeLessThan(firstSearchResultIndex)
182+
183+
// Verify search results appear before git results
184+
expect(firstSearchResultIndex).toBeLessThan(firstGitResultIndex)
185+
})
186+
187+
it("should include opened files when dynamic search results exist", () => {
188+
const result = getContextMenuOptions("open", null, mockQueryItems, mockDynamicSearchResults)
189+
190+
// Verify opened files are included
191+
expect(result.some((item) => item.type === ContextMenuOptionType.OpenedFile)).toBe(true)
192+
// Verify dynamic search results are also present
193+
expect(result.some((item) => item.value === "/search/result1.ts")).toBe(true)
194+
})
195+
196+
it("should include git results when dynamic search results exist", () => {
197+
const result = getContextMenuOptions("commit", null, mockQueryItems, mockDynamicSearchResults)
198+
199+
// Verify git results are included
200+
expect(result.some((item) => item.type === ContextMenuOptionType.Git)).toBe(true)
201+
// Verify dynamic search results are also present
202+
expect(result.some((item) => item.value === "/search/result1.ts")).toBe(true)
203+
})
204+
205+
it("should deduplicate items correctly when combining different result types", () => {
206+
// Create duplicate search result with same path as an existing file
207+
const duplicateSearchResults = [
208+
{
209+
path: "src/test.ts", // Duplicate of existing file in mockQueryItems
210+
type: "file" as const,
211+
},
212+
{
213+
path: "unique/path.ts",
214+
type: "file" as const,
215+
},
216+
]
217+
218+
const result = getContextMenuOptions("test", null, mockQueryItems, duplicateSearchResults)
219+
220+
// Count occurrences of src/test.ts in results
221+
const duplicateCount = result.filter(
222+
(item) =>
223+
(item.value === "src/test.ts" || item.value === "/src/test.ts") &&
224+
item.type === ContextMenuOptionType.File,
225+
).length
226+
// With path normalization, these should be treated as duplicates
227+
expect(duplicateCount).toBe(1)
228+
229+
// Verify the unique item was included (check both path formats)
230+
expect(result.some((item) => item.value === "/unique/path.ts" || item.value === "unique/path.ts")).toBe(true)
231+
})
232+
233+
it("should return NoResults when all combined results are empty with dynamic search", () => {
234+
// Use a query that won't match anything
235+
const result = getContextMenuOptions(
236+
"nonexistentquery123456",
237+
null,
238+
mockQueryItems,
239+
[], // Empty dynamic search results
240+
)
241+
242+
expect(result).toHaveLength(1)
243+
expect(result[0].type).toBe(ContextMenuOptionType.NoResults)
244+
})
245+
246+
/**
247+
* Tests that opened files appear first in the results, according to the updated implementation
248+
* This test validates the updated ordering where opened files have the highest priority
249+
*/
250+
it("should place opened files first in result order", () => {
251+
// Create test data with multiple types that should match the query
252+
const testQuery = "test" // Using "test" as the query to match all items
253+
254+
const testItems: ContextMenuQueryItem[] = [
255+
{
256+
type: ContextMenuOptionType.File,
257+
value: "src/test-file.ts",
258+
label: "test-file.ts",
259+
description: "Regular test file",
260+
},
261+
{
262+
type: ContextMenuOptionType.OpenedFile,
263+
value: "src/test-opened.ts",
264+
label: "test-opened.ts",
265+
description: "Opened test file",
266+
},
267+
{
268+
type: ContextMenuOptionType.Git,
269+
value: "abctest",
270+
label: "Test commit",
271+
description: "Git test commit",
272+
},
273+
]
274+
275+
const testSearchResults = [
276+
{
277+
path: "search/test-result.ts",
278+
type: "file" as const,
279+
label: "test-result.ts",
280+
},
281+
]
282+
283+
// Get results for "test" query
284+
const result = getContextMenuOptions(testQuery, null, testItems, testSearchResults)
285+
286+
// Verify we have results
287+
expect(result.length).toBeGreaterThan(0)
288+
289+
// Verify the first item is an opened file type
290+
expect(result[0].type).toBe(ContextMenuOptionType.OpenedFile)
291+
292+
// Verify the remaining items are in the correct order:
293+
// suggestions -> openedFiles -> searchResults -> gitResults
294+
295+
// Get index of first item of each type
296+
const firstOpenedFileIndex = result.findIndex((item) => item.type === ContextMenuOptionType.OpenedFile)
297+
const firstSearchResultIndex = result.findIndex(
298+
(item) => item.type === ContextMenuOptionType.File && item.value?.includes("/search/"),
299+
)
300+
const firstGitResultIndex = result.findIndex((item) => item.type === ContextMenuOptionType.Git)
301+
302+
// Verify opened files come first
303+
expect(firstOpenedFileIndex).toBe(0)
304+
305+
// Verify search results come after opened files but before git results
306+
expect(firstSearchResultIndex).toBeGreaterThan(firstOpenedFileIndex)
307+
308+
// Verify git results come after search results
309+
if (firstGitResultIndex !== -1 && firstSearchResultIndex !== -1) {
310+
expect(firstGitResultIndex).toBeGreaterThan(firstSearchResultIndex)
311+
}
312+
})
111313
})
112314

113315
describe("shouldShowContextMenu", () => {

‎webview-ui/src/utils/context-mentions.ts‎

Lines changed: 27 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -210,34 +210,6 @@ export function getContextMenuOptions(
210210
}
211211
}
212212

213-
if (dynamicSearchResults.length > 0) {
214-
// Convert search results to queryItems format
215-
const searchResultItems = dynamicSearchResults.map((result) => {
216-
const formattedPath = result.path.startsWith("/") ? result.path : `/${result.path}`
217-
218-
return {
219-
type: result.type === "folder" ? ContextMenuOptionType.Folder : ContextMenuOptionType.File,
220-
value: formattedPath,
221-
label: result.label || path.basename(result.path),
222-
description: formattedPath,
223-
}
224-
})
225-
226-
const allItems = [...suggestions, ...searchResultItems]
227-
228-
// Remove duplicates
229-
const seen = new Set()
230-
const deduped = allItems.filter((item) => {
231-
const key = `${item.type}-${item.value}`
232-
if (seen.has(key)) return false
233-
seen.add(key)
234-
return true
235-
})
236-
237-
return deduped
238-
}
239-
240-
// Fallback to original static filtering if no dynamic results
241213
const searchableItems = queryItems.map((item) => ({
242214
original: item,
243215
searchStr: [item.value, item.label, item.description].filter(Boolean).join(" "),
@@ -252,38 +224,36 @@ export function getContextMenuOptions(
252224
const matchingItems = query ? fzf.find(query).map((result) => result.item.original) : []
253225

254226
// Separate matches by type
255-
const fileMatches = matchingItems.filter(
256-
(item) =>
257-
item.type === ContextMenuOptionType.File ||
258-
item.type === ContextMenuOptionType.OpenedFile ||
259-
item.type === ContextMenuOptionType.Folder,
260-
)
227+
const openedFileMatches = matchingItems.filter((item) => item.type === ContextMenuOptionType.OpenedFile)
228+
261229
const gitMatches = matchingItems.filter((item) => item.type === ContextMenuOptionType.Git)
262-
const otherMatches = matchingItems.filter(
263-
(item) =>
264-
item.type !== ContextMenuOptionType.File &&
265-
item.type !== ContextMenuOptionType.OpenedFile &&
266-
item.type !== ContextMenuOptionType.Folder &&
267-
item.type !== ContextMenuOptionType.Git,
268-
)
269-
270-
// Combine suggestions with matching items in the desired order
271-
if (suggestions.length > 0 || matchingItems.length > 0) {
272-
const allItems = [...suggestions, ...fileMatches, ...gitMatches, ...otherMatches]
273-
274-
// Remove duplicates based on type and value
275-
const seen = new Set()
276-
const deduped = allItems.filter((item) => {
277-
const key = `${item.type}-${item.value}`
278-
if (seen.has(key)) return false
279-
seen.add(key)
280-
return true
281-
})
282230

283-
return deduped
284-
}
231+
// Convert search results to queryItems format
232+
const searchResultItems = dynamicSearchResults.map((result) => {
233+
const formattedPath = result.path.startsWith("/") ? result.path : `/${result.path}`
234+
235+
return {
236+
type: result.type === "folder" ? ContextMenuOptionType.Folder : ContextMenuOptionType.File,
237+
value: formattedPath,
238+
label: result.label || path.basename(result.path),
239+
description: formattedPath,
240+
}
241+
})
242+
243+
const allItems = [...suggestions, ...openedFileMatches, ...searchResultItems, ...gitMatches]
244+
245+
// Remove duplicates - normalize paths by ensuring all have leading slashes
246+
const seen = new Set()
247+
const deduped = allItems.filter((item) => {
248+
// Normalize paths for deduplication by ensuring leading slashes
249+
const normalizedValue = item.value && !item.value.startsWith("/") ? `/${item.value}` : item.value
250+
const key = `${item.type}-${normalizedValue}`
251+
if (seen.has(key)) return false
252+
seen.add(key)
253+
return true
254+
})
285255

286-
return [{ type: ContextMenuOptionType.NoResults }]
256+
return deduped.length > 0 ? deduped : [{ type: ContextMenuOptionType.NoResults }]
287257
}
288258

289259
export function shouldShowContextMenu(text: string, position: number): boolean {

0 commit comments

Comments
 (0)