Преглед изворни кода

Bring back open files and git results in mention suggestions (#1946)

Matt Rubens пре 9 месеци
родитељ
комит
15338ed76a

+ 205 - 3
webview-ui/src/utils/__tests__/context-mentions.test.ts

@@ -56,6 +56,12 @@ describe("getContextMenuOptions", () => {
 			label: "test.ts",
 			description: "Source file",
 		},
+		{
+			type: ContextMenuOptionType.OpenedFile,
+			value: "src/opened.ts",
+			label: "opened.ts",
+			description: "Currently opened file",
+		},
 		{
 			type: ContextMenuOptionType.Git,
 			value: "abc1234",
@@ -71,6 +77,18 @@ describe("getContextMenuOptions", () => {
 		},
 	]
 
+	const mockDynamicSearchResults = [
+		{
+			path: "search/result1.ts",
+			type: "file" as const,
+			label: "result1.ts",
+		},
+		{
+			path: "search/folder",
+			type: "folder" as const,
+		},
+	]
+
 	it("should return all option types for empty query", () => {
 		const result = getContextMenuOptions("", null, [])
 		expect(result).toHaveLength(6)
@@ -86,9 +104,11 @@ describe("getContextMenuOptions", () => {
 
 	it("should filter by selected type when query is empty", () => {
 		const result = getContextMenuOptions("", ContextMenuOptionType.File, mockQueryItems)
-		expect(result).toHaveLength(1)
-		expect(result[0].type).toBe(ContextMenuOptionType.File)
-		expect(result[0].value).toBe("src/test.ts")
+		expect(result).toHaveLength(2)
+		expect(result.map((item) => item.type)).toContain(ContextMenuOptionType.File)
+		expect(result.map((item) => item.type)).toContain(ContextMenuOptionType.OpenedFile)
+		expect(result.map((item) => item.value)).toContain("src/test.ts")
+		expect(result.map((item) => item.value)).toContain("src/opened.ts")
 	})
 
 	it("should match git commands", () => {
@@ -108,6 +128,188 @@ describe("getContextMenuOptions", () => {
 		expect(result).toHaveLength(1)
 		expect(result[0].type).toBe(ContextMenuOptionType.NoResults)
 	})
+
+	/**
+	 * Tests for the combined handling of open files, git results, and search results
+	 * Added for commit 3cd7dec78faf786e468ae4f66cef0b81a76d9075
+	 */
+	it("should include dynamic search results along with other matches", () => {
+		// Add an opened file that will match the query
+		const testItems = [
+			...mockQueryItems,
+			{
+				type: ContextMenuOptionType.OpenedFile,
+				value: "src/test-opened.ts",
+				label: "test-opened.ts",
+				description: "Opened test file for search test",
+			},
+		]
+
+		const result = getContextMenuOptions("test", null, testItems, mockDynamicSearchResults)
+
+		// Check if opened files and dynamic search results are included
+		expect(result.some((item) => item.type === ContextMenuOptionType.OpenedFile)).toBe(true)
+		expect(result.some((item) => item.value === "/search/result1.ts")).toBe(true)
+	})
+
+	it("should maintain correct result ordering according to implementation", () => {
+		// Add multiple item types to test ordering
+		const result = getContextMenuOptions("t", null, mockQueryItems, mockDynamicSearchResults)
+
+		// Find the different result types
+		const fileResults = result.filter(
+			(item) =>
+				item.type === ContextMenuOptionType.File ||
+				item.type === ContextMenuOptionType.OpenedFile ||
+				item.type === ContextMenuOptionType.Folder,
+		)
+
+		const searchResults = result.filter(
+			(item) => item.type === ContextMenuOptionType.File && item.value?.includes("/search/"),
+		)
+
+		const gitResults = result.filter((item) => item.type === ContextMenuOptionType.Git)
+
+		// Find the indexes of the first item of each type in the results array
+		const firstFileIndex = result.findIndex((item) => fileResults.some((f) => f === item))
+
+		const firstSearchResultIndex = result.findIndex((item) => searchResults.some((s) => s === item))
+
+		const firstGitResultIndex = result.findIndex((item) => gitResults.some((g) => g === item))
+
+		// Verify file results come before search results
+		expect(firstFileIndex).toBeLessThan(firstSearchResultIndex)
+
+		// Verify search results appear before git results
+		expect(firstSearchResultIndex).toBeLessThan(firstGitResultIndex)
+	})
+
+	it("should include opened files when dynamic search results exist", () => {
+		const result = getContextMenuOptions("open", null, mockQueryItems, mockDynamicSearchResults)
+
+		// Verify opened files are included
+		expect(result.some((item) => item.type === ContextMenuOptionType.OpenedFile)).toBe(true)
+		// Verify dynamic search results are also present
+		expect(result.some((item) => item.value === "/search/result1.ts")).toBe(true)
+	})
+
+	it("should include git results when dynamic search results exist", () => {
+		const result = getContextMenuOptions("commit", null, mockQueryItems, mockDynamicSearchResults)
+
+		// Verify git results are included
+		expect(result.some((item) => item.type === ContextMenuOptionType.Git)).toBe(true)
+		// Verify dynamic search results are also present
+		expect(result.some((item) => item.value === "/search/result1.ts")).toBe(true)
+	})
+
+	it("should deduplicate items correctly when combining different result types", () => {
+		// Create duplicate search result with same path as an existing file
+		const duplicateSearchResults = [
+			{
+				path: "src/test.ts", // Duplicate of existing file in mockQueryItems
+				type: "file" as const,
+			},
+			{
+				path: "unique/path.ts",
+				type: "file" as const,
+			},
+		]
+
+		const result = getContextMenuOptions("test", null, mockQueryItems, duplicateSearchResults)
+
+		// Count occurrences of src/test.ts in results
+		const duplicateCount = result.filter(
+			(item) =>
+				(item.value === "src/test.ts" || item.value === "/src/test.ts") &&
+				item.type === ContextMenuOptionType.File,
+		).length
+		// With path normalization, these should be treated as duplicates
+		expect(duplicateCount).toBe(1)
+
+		// Verify the unique item was included (check both path formats)
+		expect(result.some((item) => item.value === "/unique/path.ts" || item.value === "unique/path.ts")).toBe(true)
+	})
+
+	it("should return NoResults when all combined results are empty with dynamic search", () => {
+		// Use a query that won't match anything
+		const result = getContextMenuOptions(
+			"nonexistentquery123456",
+			null,
+			mockQueryItems,
+			[], // Empty dynamic search results
+		)
+
+		expect(result).toHaveLength(1)
+		expect(result[0].type).toBe(ContextMenuOptionType.NoResults)
+	})
+
+	/**
+	 * Tests that opened files appear first in the results, according to the updated implementation
+	 * This test validates the updated ordering where opened files have the highest priority
+	 */
+	it("should place opened files first in result order", () => {
+		// Create test data with multiple types that should match the query
+		const testQuery = "test" // Using "test" as the query to match all items
+
+		const testItems: ContextMenuQueryItem[] = [
+			{
+				type: ContextMenuOptionType.File,
+				value: "src/test-file.ts",
+				label: "test-file.ts",
+				description: "Regular test file",
+			},
+			{
+				type: ContextMenuOptionType.OpenedFile,
+				value: "src/test-opened.ts",
+				label: "test-opened.ts",
+				description: "Opened test file",
+			},
+			{
+				type: ContextMenuOptionType.Git,
+				value: "abctest",
+				label: "Test commit",
+				description: "Git test commit",
+			},
+		]
+
+		const testSearchResults = [
+			{
+				path: "search/test-result.ts",
+				type: "file" as const,
+				label: "test-result.ts",
+			},
+		]
+
+		// Get results for "test" query
+		const result = getContextMenuOptions(testQuery, null, testItems, testSearchResults)
+
+		// Verify we have results
+		expect(result.length).toBeGreaterThan(0)
+
+		// Verify the first item is an opened file type
+		expect(result[0].type).toBe(ContextMenuOptionType.OpenedFile)
+
+		// Verify the remaining items are in the correct order:
+		// suggestions -> openedFiles -> searchResults -> gitResults
+
+		// Get index of first item of each type
+		const firstOpenedFileIndex = result.findIndex((item) => item.type === ContextMenuOptionType.OpenedFile)
+		const firstSearchResultIndex = result.findIndex(
+			(item) => item.type === ContextMenuOptionType.File && item.value?.includes("/search/"),
+		)
+		const firstGitResultIndex = result.findIndex((item) => item.type === ContextMenuOptionType.Git)
+
+		// Verify opened files come first
+		expect(firstOpenedFileIndex).toBe(0)
+
+		// Verify search results come after opened files but before git results
+		expect(firstSearchResultIndex).toBeGreaterThan(firstOpenedFileIndex)
+
+		// Verify git results come after search results
+		if (firstGitResultIndex !== -1 && firstSearchResultIndex !== -1) {
+			expect(firstGitResultIndex).toBeGreaterThan(firstSearchResultIndex)
+		}
+	})
 })
 
 describe("shouldShowContextMenu", () => {

+ 27 - 57
webview-ui/src/utils/context-mentions.ts

@@ -210,34 +210,6 @@ export function getContextMenuOptions(
 		}
 	}
 
-	if (dynamicSearchResults.length > 0) {
-		// Convert search results to queryItems format
-		const searchResultItems = dynamicSearchResults.map((result) => {
-			const formattedPath = result.path.startsWith("/") ? result.path : `/${result.path}`
-
-			return {
-				type: result.type === "folder" ? ContextMenuOptionType.Folder : ContextMenuOptionType.File,
-				value: formattedPath,
-				label: result.label || path.basename(result.path),
-				description: formattedPath,
-			}
-		})
-
-		const allItems = [...suggestions, ...searchResultItems]
-
-		// Remove duplicates
-		const seen = new Set()
-		const deduped = allItems.filter((item) => {
-			const key = `${item.type}-${item.value}`
-			if (seen.has(key)) return false
-			seen.add(key)
-			return true
-		})
-
-		return deduped
-	}
-
-	// Fallback to original static filtering if no dynamic results
 	const searchableItems = queryItems.map((item) => ({
 		original: item,
 		searchStr: [item.value, item.label, item.description].filter(Boolean).join(" "),
@@ -252,38 +224,36 @@ export function getContextMenuOptions(
 	const matchingItems = query ? fzf.find(query).map((result) => result.item.original) : []
 
 	// Separate matches by type
-	const fileMatches = matchingItems.filter(
-		(item) =>
-			item.type === ContextMenuOptionType.File ||
-			item.type === ContextMenuOptionType.OpenedFile ||
-			item.type === ContextMenuOptionType.Folder,
-	)
+	const openedFileMatches = matchingItems.filter((item) => item.type === ContextMenuOptionType.OpenedFile)
+
 	const gitMatches = matchingItems.filter((item) => item.type === ContextMenuOptionType.Git)
-	const otherMatches = matchingItems.filter(
-		(item) =>
-			item.type !== ContextMenuOptionType.File &&
-			item.type !== ContextMenuOptionType.OpenedFile &&
-			item.type !== ContextMenuOptionType.Folder &&
-			item.type !== ContextMenuOptionType.Git,
-	)
-
-	// Combine suggestions with matching items in the desired order
-	if (suggestions.length > 0 || matchingItems.length > 0) {
-		const allItems = [...suggestions, ...fileMatches, ...gitMatches, ...otherMatches]
-
-		// Remove duplicates based on type and value
-		const seen = new Set()
-		const deduped = allItems.filter((item) => {
-			const key = `${item.type}-${item.value}`
-			if (seen.has(key)) return false
-			seen.add(key)
-			return true
-		})
 
-		return deduped
-	}
+	// Convert search results to queryItems format
+	const searchResultItems = dynamicSearchResults.map((result) => {
+		const formattedPath = result.path.startsWith("/") ? result.path : `/${result.path}`
+
+		return {
+			type: result.type === "folder" ? ContextMenuOptionType.Folder : ContextMenuOptionType.File,
+			value: formattedPath,
+			label: result.label || path.basename(result.path),
+			description: formattedPath,
+		}
+	})
+
+	const allItems = [...suggestions, ...openedFileMatches, ...searchResultItems, ...gitMatches]
+
+	// Remove duplicates - normalize paths by ensuring all have leading slashes
+	const seen = new Set()
+	const deduped = allItems.filter((item) => {
+		// Normalize paths for deduplication by ensuring leading slashes
+		const normalizedValue = item.value && !item.value.startsWith("/") ? `/${item.value}` : item.value
+		const key = `${item.type}-${normalizedValue}`
+		if (seen.has(key)) return false
+		seen.add(key)
+		return true
+	})
 
-	return [{ type: ContextMenuOptionType.NoResults }]
+	return deduped.length > 0 ? deduped : [{ type: ContextMenuOptionType.NoResults }]
 }
 
 export function shouldShowContextMenu(text: string, position: number): boolean {