-
Notifications
You must be signed in to change notification settings - Fork 30
feat: Add search filter to sidebar #562
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
bab6c21
3e340bf
9dd25a8
8f103e5
2d186c7
448d947
a5c0ceb
de2dd41
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -684,6 +684,46 @@ export class Commands { | |
}); | ||
} | ||
|
||
/** | ||
* Search/filter workspaces in the All Workspaces view. | ||
* This method will be called from the view title menu. | ||
*/ | ||
public async searchWorkspaces(): Promise<void> { | ||
const quickPick = vscode.window.createQuickPick(); | ||
quickPick.placeholder = | ||
"Type to search workspaces by name, owner, template, status, or agent"; | ||
quickPick.title = "Search All Workspaces"; | ||
quickPick.value = ""; | ||
|
||
// Get current search filter to show in the input | ||
const currentFilter = (await vscode.commands.executeCommand( | ||
"coder.getWorkspaceSearchFilter", | ||
)) as string; | ||
if (currentFilter) { | ||
quickPick.value = currentFilter; | ||
} | ||
|
||
quickPick.ignoreFocusOut = true; // Keep open when clicking elsewhere | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this seems like an odd thing to set. can you explain your rationale? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, here was my thought process on user behavior - When a user clicks outside the Quickpick (like on the workspace list or elsewhere in VS Code), the quickpick automatically closes
With the current approach it solves the above scenarios. The context is still preserved in state (private searchFilter) so the filtered results will not change if the input closes - its more so just a means for the user to not have to click on the search button again if they are not satisfied or have not completed their search |
||
quickPick.canSelectMany = false; // Don't show selection list | ||
|
||
quickPick.onDidChangeValue((value) => { | ||
// Update the search filter in real-time as user types | ||
vscode.commands.executeCommand("coder.setWorkspaceSearchFilter", value); | ||
}); | ||
|
||
quickPick.onDidAccept(() => { | ||
// When user presses Enter, close the search | ||
quickPick.hide(); | ||
}); | ||
|
||
quickPick.onDidHide(() => { | ||
// Don't clear the search when closed - keep the filter active | ||
quickPick.dispose(); | ||
}); | ||
|
||
quickPick.show(); | ||
} | ||
|
||
/** | ||
* Return agents from the workspace. | ||
* | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -42,6 +42,9 @@ export class WorkspaceProvider | |
private timeout: NodeJS.Timeout | undefined; | ||
private fetching = false; | ||
private visible = false; | ||
private searchFilter = ""; | ||
private metadataCache: Record<string, string> = {}; | ||
private searchDebounceTimer: NodeJS.Timeout | undefined; | ||
|
||
constructor( | ||
private readonly getWorkspacesQuery: WorkspaceQuery, | ||
|
@@ -52,6 +55,43 @@ export class WorkspaceProvider | |
// No initialization. | ||
} | ||
|
||
setSearchFilter(filter: string) { | ||
// Validate search term length to prevent performance issues | ||
if (filter.length > 200) { | ||
filter = filter.substring(0, 200); | ||
} | ||
Comment on lines
+59
to
+62
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what performance issue? why 200 characters? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, i'd like to note:
The 200 char limit was just an arbitrary number to limit the length of the search input We could do the following -
Let me know how we should proceed or what you suggest |
||
|
||
// Clear any existing debounce timer | ||
if (this.searchDebounceTimer) { | ||
clearTimeout(this.searchDebounceTimer); | ||
} | ||
|
||
// Debounce the search operation to improve performance | ||
this.searchDebounceTimer = setTimeout(() => { | ||
this.searchFilter = filter; | ||
this.refresh(undefined); | ||
this.searchDebounceTimer = undefined; | ||
}, 150); // 150ms debounce delay - good balance between responsiveness and performance | ||
} | ||
|
||
getSearchFilter(): string { | ||
return this.searchFilter; | ||
} | ||
|
||
/** | ||
* Clear the search filter immediately without debouncing. | ||
* Use this when the user explicitly clears the search. | ||
*/ | ||
clearSearchFilter(): void { | ||
// Clear any pending debounce timer | ||
if (this.searchDebounceTimer) { | ||
clearTimeout(this.searchDebounceTimer); | ||
this.searchDebounceTimer = undefined; | ||
} | ||
this.searchFilter = ""; | ||
this.refresh(undefined); | ||
} | ||
|
||
// fetchAndRefresh fetches new workspaces, re-renders the entire tree, then | ||
// keeps refreshing (if a timer length was provided) as long as the user is | ||
// still logged in and no errors were encountered fetching workspaces. | ||
|
@@ -63,6 +103,9 @@ export class WorkspaceProvider | |
} | ||
this.fetching = true; | ||
|
||
// Clear metadata cache when refreshing to ensure data consistency | ||
this.clearMetadataCache(); | ||
|
||
// It is possible we called fetchAndRefresh() manually (through the button | ||
// for example), in which case we might still have a pending refresh that | ||
// needs to be cleared. | ||
|
@@ -201,6 +244,11 @@ export class WorkspaceProvider | |
clearTimeout(this.timeout); | ||
this.timeout = undefined; | ||
} | ||
// clear search debounce timer | ||
if (this.searchDebounceTimer) { | ||
clearTimeout(this.searchDebounceTimer); | ||
this.searchDebounceTimer = undefined; | ||
} | ||
} | ||
|
||
/** | ||
|
@@ -300,7 +348,157 @@ export class WorkspaceProvider | |
|
||
return Promise.resolve([]); | ||
} | ||
return Promise.resolve(this.workspaces || []); | ||
|
||
// Filter workspaces based on search term | ||
let filteredWorkspaces = this.workspaces || []; | ||
const trimmedFilter = this.searchFilter.trim(); | ||
if (trimmedFilter) { | ||
const searchTerm = trimmedFilter.toLowerCase(); | ||
filteredWorkspaces = filteredWorkspaces.filter((workspace) => | ||
this.matchesSearchTerm(workspace, searchTerm), | ||
); | ||
} | ||
|
||
return Promise.resolve(filteredWorkspaces); | ||
} | ||
|
||
/** | ||
* Extract and normalize searchable text fields from a workspace. | ||
* This helper method reduces code duplication between exact word and substring matching. | ||
*/ | ||
private extractSearchableFields(workspace: WorkspaceTreeItem): { | ||
workspaceName: string; | ||
ownerName: string; | ||
templateName: string; | ||
status: string; | ||
agentNames: string[]; | ||
agentMetadataText: string; | ||
} { | ||
// Handle null/undefined workspace data safely | ||
const workspaceName = workspace.workspace.name.toLowerCase(); | ||
const ownerName = workspace.workspace.owner_name.toLowerCase(); | ||
const templateName = ( | ||
workspace.workspace.template_display_name || | ||
workspace.workspace.template_name | ||
).toLowerCase(); | ||
const status = ( | ||
workspace.workspace.latest_build.status || "" | ||
).toLowerCase(); | ||
|
||
// Extract agent names with null safety | ||
const agents = extractAgents(workspace.workspace.latest_build.resources); | ||
const agentNames = agents | ||
.map((agent) => agent.name.toLowerCase()) | ||
.filter((name) => name.length > 0); | ||
|
||
// Extract and cache agent metadata with error handling | ||
let agentMetadataText = ""; | ||
const metadataCacheKey = agents.map((agent) => agent.id).join(","); | ||
|
||
if (this.metadataCache[metadataCacheKey]) { | ||
agentMetadataText = this.metadataCache[metadataCacheKey]; | ||
} else { | ||
const metadataStrings: string[] = []; | ||
let hasSerializationErrors = false; | ||
|
||
agents.forEach((agent) => { | ||
const watcher = this.agentWatchers[agent.id]; | ||
if (watcher?.metadata) { | ||
watcher.metadata.forEach((metadata) => { | ||
try { | ||
metadataStrings.push(JSON.stringify(metadata).toLowerCase()); | ||
} catch (error) { | ||
hasSerializationErrors = true; | ||
// Handle JSON serialization errors gracefully | ||
this.storage.output.warn( | ||
`Failed to serialize metadata for agent ${agent.id}: ${error}`, | ||
); | ||
Comment on lines
+412
to
+415
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. but now the metadata still gets cached even with a missing entry, right? this seems like a cache corruption bug There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed this - de2dd41. We now only cache the data if all metadata is serialized successfully |
||
} | ||
}); | ||
} | ||
}); | ||
|
||
agentMetadataText = metadataStrings.join(" "); | ||
|
||
// Only cache if all metadata serialized successfully | ||
if (!hasSerializationErrors) { | ||
this.metadataCache[metadataCacheKey] = agentMetadataText; | ||
} | ||
} | ||
|
||
return { | ||
workspaceName, | ||
ownerName, | ||
templateName, | ||
status, | ||
agentNames, | ||
agentMetadataText, | ||
}; | ||
} | ||
|
||
/** | ||
* Check if a workspace matches the given search term using smart search logic. | ||
* Prioritizes exact word matches over substring matches. | ||
*/ | ||
private matchesSearchTerm( | ||
workspace: WorkspaceTreeItem, | ||
searchTerm: string, | ||
): boolean { | ||
// Early return for empty search terms | ||
if (!searchTerm || searchTerm.trim().length === 0) { | ||
return true; | ||
} | ||
|
||
// Extract all searchable fields once | ||
const fields = this.extractSearchableFields(workspace); | ||
|
||
// Pre-compile regex patterns for exact word matching | ||
const searchWords = searchTerm | ||
.split(/\s+/) | ||
.filter((word) => word.length > 0); | ||
|
||
const regexPatterns: RegExp[] = []; | ||
for (const word of searchWords) { | ||
// Simple word boundary search | ||
regexPatterns.push(new RegExp(`\\b${word}\\b`, "i")); | ||
} | ||
|
||
// Combine all text for exact word matching | ||
const allText = [ | ||
fields.workspaceName, | ||
fields.ownerName, | ||
fields.templateName, | ||
fields.status, | ||
...fields.agentNames, | ||
fields.agentMetadataText, | ||
].join(" "); | ||
|
||
// Check for exact word matches (higher priority) | ||
const hasExactWordMatch = | ||
regexPatterns.length > 0 && | ||
regexPatterns.some((pattern) => pattern.test(allText)); | ||
|
||
// Check for substring matches (lower priority) - only if no exact word match | ||
const hasSubstringMatch = | ||
yelnatscoding marked this conversation as resolved.
Show resolved
Hide resolved
|
||
!hasExactWordMatch && allText.includes(searchTerm); | ||
|
||
// Return true if either exact word match or substring match | ||
return hasExactWordMatch || hasSubstringMatch; | ||
} | ||
|
||
/** | ||
* Clear the metadata cache when workspaces are refreshed to ensure data consistency. | ||
* Also clears cache if it grows too large to prevent memory issues. | ||
*/ | ||
private clearMetadataCache(): void { | ||
// Clear cache if it grows too large (prevent memory issues) | ||
const cacheSize = Object.keys(this.metadataCache).length; | ||
if (cacheSize > 1000) { | ||
this.storage.output.info( | ||
`Clearing metadata cache due to size (${cacheSize} entries)`, | ||
); | ||
} | ||
this.metadataCache = {}; | ||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
feels a little weird to put this in the middle
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where do you suggest we move it to? or should we remove it entirely?