-
Notifications
You must be signed in to change notification settings - Fork 99
feature: basic file search #341
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
Conversation
WalkthroughA file search dialog feature was added to the repository browsing interface. This includes state updates, a new command dialog component for file search with hotkey and recent files, backend logic to fetch a flat file list, UI integration in the layout and file tree panel, and a refactor for file/folder icon rendering. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant FileTreePanel
participant BrowseState
participant FileSearchCommandDialog
participant Backend (getFiles)
participant Router
User->>FileTreePanel: Clicks search button or presses Cmd+P
FileTreePanel->>BrowseState: Set isFileSearchOpen = true
BrowseState->>FileSearchCommandDialog: Renders dialog (open)
FileSearchCommandDialog->>Backend (getFiles): Fetch flat file list (repo, revision)
Backend (getFiles)-->>FileSearchCommandDialog: Returns file list
User->>FileSearchCommandDialog: Types search query
FileSearchCommandDialog->>FileSearchCommandDialog: Filters files, shows results
User->>FileSearchCommandDialog: Selects file
FileSearchCommandDialog->>Router: Navigates to file
FileSearchCommandDialog->>BrowseState: Set isFileSearchOpen = false
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/web/src/features/fileTree/actions.ts (1)
268-285
: Consider consolidating the duplicatedgetRepoPath
function.As noted in the TODO comment, this function is duplicated from the backend's
utils.ts
file. Consider moving this to a shared package to reduce code duplication and maintenance overhead.packages/web/src/features/fileTree/components/fileTreeItemIcon.tsx (1)
14-34
: Eliminate code duplication in icon name processing.The icon name processing logic is duplicated between the
tree
andblob
branches. Consider extracting this into a helper function for better maintainability.+const formatIconName = (icon: string): string => { + return `vscode-icons:${icon.substring(0, icon.indexOf('.')).replaceAll('_', '-')}`; +}; export const FileTreeItemIcon = ({ item, className }: FileTreeItemIconProps) => { const iconName = useMemo(() => { if (item.type === 'tree') { const icon = getIconForFolder(item.name); if (icon) { - const iconName = `vscode-icons:${icon.substring(0, icon.indexOf('.')).replaceAll('_', '-')}`; - return iconName; + return formatIconName(icon); } } else if (item.type === 'blob') { const icon = getIconForFile(item.name); if (icon) { - const iconName = `vscode-icons:${icon.substring(0, icon.indexOf('.')).replaceAll('_', '-')}`; - return iconName; + return formatIconName(icon); } } return "vscode-icons:file-type-unknown"; }, [item.name, item.type]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/web/src/app/[domain]/browse/browseStateProvider.tsx
(2 hunks)packages/web/src/app/[domain]/browse/components/fileSearchCommandDialog.tsx
(1 hunks)packages/web/src/app/[domain]/browse/layout.tsx
(2 hunks)packages/web/src/features/fileTree/actions.ts
(1 hunks)packages/web/src/features/fileTree/components/fileTreeItemComponent.tsx
(2 hunks)packages/web/src/features/fileTree/components/fileTreeItemIcon.tsx
(1 hunks)packages/web/src/features/fileTree/components/fileTreePanel.tsx
(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (6)
packages/web/src/app/[domain]/browse/layout.tsx (2)
packages/web/src/app/[domain]/browse/components/fileSearchCommandDialog.tsx (1)
FileSearchCommandDialog
(29-216)packages/web/src/app/[domain]/browse/browseStateProvider.tsx (1)
BrowseStateProvider
(43-81)
packages/web/src/features/fileTree/components/fileTreePanel.tsx (4)
packages/web/src/components/ui/tooltip.tsx (3)
Tooltip
(30-30)TooltipTrigger
(30-30)TooltipContent
(30-30)packages/web/src/components/ui/button.tsx (1)
Button
(56-56)packages/web/src/app/components/keyboardShortcutHint.tsx (1)
KeyboardShortcutHint
(8-22)packages/web/src/components/ui/separator.tsx (1)
Separator
(31-31)
packages/web/src/features/fileTree/components/fileTreeItemComponent.tsx (1)
packages/web/src/features/fileTree/components/fileTreeItemIcon.tsx (1)
FileTreeItemIcon
(14-34)
packages/web/src/app/[domain]/browse/components/fileSearchCommandDialog.tsx (7)
packages/web/src/features/fileTree/actions.ts (2)
FileTreeItem
(14-18)getFiles
(160-209)packages/web/src/app/[domain]/browse/hooks/useBrowseParams.ts (1)
useBrowseParams
(5-48)packages/web/src/app/[domain]/browse/hooks/useBrowseState.ts (1)
useBrowseState
(6-12)packages/web/src/app/[domain]/browse/hooks/useBrowseNavigation.ts (1)
useBrowseNavigation
(25-59)packages/web/src/hooks/usePrefetchFileSource.ts (1)
usePrefetchFileSource
(9-25)packages/web/src/lib/utils.ts (1)
unwrapServiceError
(392-399)packages/web/src/features/fileTree/components/fileTreeItemIcon.tsx (1)
FileTreeItemIcon
(14-34)
packages/web/src/features/fileTree/components/fileTreeItemIcon.tsx (2)
packages/web/src/features/fileTree/actions.ts (1)
FileTreeItem
(14-18)packages/web/src/lib/utils.ts (1)
cn
(18-20)
packages/web/src/features/fileTree/actions.ts (3)
packages/web/src/actions.ts (3)
sew
(53-61)withAuth
(63-116)withOrgMembership
(147-195)packages/web/src/lib/serviceError.ts (2)
notFound
(91-97)unexpectedError
(75-81)packages/backend/src/utils.ts (1)
getRepoPath
(75-90)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (14)
packages/web/src/app/[domain]/browse/browseStateProvider.tsx (1)
15-15
: LGTM! Clean state management extension.The new
isFileSearchOpen
property follows the established pattern for boolean state flags in the browse state. The naming is clear and the default value is appropriate.Also applies to: 24-24
packages/web/src/features/fileTree/actions.ts (1)
160-209
: Well-implemented file listing function.The
getFiles
function follows established patterns in the codebase for authentication, authorization, and git operations. The use ofgit ls-tree -r --name-only
is appropriate for getting a flat list of files, and the error handling is consistent with other functions.packages/web/src/app/[domain]/browse/layout.tsx (1)
11-11
: Appropriate placement of the file search dialog.The
FileSearchCommandDialog
is correctly positioned within theBrowseStateProvider
but outside the main layout structure, making it suitable for use as a modal overlay that can access browse state.Also applies to: 66-66
packages/web/src/features/fileTree/components/fileTreePanel.tsx (1)
24-24
: Excellent UI integration following established patterns.The search button implementation is well-crafted:
- Follows the same pattern as the existing collapse button
- Uses consistent styling and tooltip structure
- Properly integrates with the browse state management
- The
ml-auto
positioning appropriately places it on the right side of the header- Keyboard shortcut hint matches the actual hotkey (⌘ P) from the dialog component
Also applies to: 107-125
packages/web/src/features/fileTree/components/fileTreeItemComponent.tsx (3)
4-4
: LGTM: Clean import refactoring.Removing the
useMemo
import after extracting the icon logic to a separate component is correct.
8-8
: LGTM: Proper component import.Importing the new
FileTreeItemIcon
component follows good modularization practices.
83-83
: LGTM: Simplified icon rendering.Replacing the inline icon computation with the dedicated
FileTreeItemIcon
component improves code separation and maintainability.packages/web/src/app/[domain]/browse/components/fileSearchCommandDialog.tsx (7)
18-18
: LGTM: Reasonable performance limit.Setting
MAX_RESULTS = 100
is a good performance optimization to prevent UI lag with large repositories.
42-51
: LGTM: Well-configured hotkey implementation.The hotkey setup correctly prevents default behavior and includes appropriate form tag enablement for universal access.
132-139
: LGTM: Good workaround for input focus timing.The comment clearly explains the loading state issue and the useEffect provides a proper solution.
154-155
: LGTM: Proper accessibility implementation.Using screen reader-only titles and descriptions ensures the dialog is accessible while maintaining the visual design.
259-267
: Verify highlight range calculation consistency.The
Highlight
component usestext.slice(range.from, range.to + 1)
which treatsrange.to
as inclusive, but this needs to be consistent with how the range is calculated in the search logic.Please verify that the highlight range calculation in the search logic (lines 88-95) produces the expected highlighting behavior. Test with a search query like "test" in a file path "src/test/file.js" to ensure the entire match is highlighted correctly.
269-283
: LGTM: Well-designed loading skeleton.The skeleton provides good visual feedback with appropriate animation and layout structure that matches the search results.
66-99
: 🛠️ Refactor suggestionPotential edge case in search highlighting logic.
The search highlighting calculation may have an off-by-one error when creating the highlight range.
.map(({ file, matchIndex }) => { return { file, match: { from: matchIndex, - to: matchIndex + searchQuery.length - 1, + to: matchIndex + searchQuery.length, }, } }),The current logic subtracts 1 from the end index, but the
Highlight
component on line 263 usestext.slice(range.from, range.to + 1)
, which suggestsrange.to
should be exclusive. This creates a double adjustment that may truncate the last character of the match.Likely an incorrect or invalid review comment.
This PR implements a basic file search into Sourcebot's
/browse
view:file-search.mp4
Note: this is exact substring search, and not fuzzy search. I tried to implement fuzzy search using fuse.js and some other libraries, but the performance wasn't great on larger projects (e.g., UnrealEngine with 100k+ files).
Fuzzy search is pretty useful when searching for files, so we should re-visit this at some point. To solve the perf issue, some ideas are:
git ls-tree
on every request.Why not use zoekt?
I decided not to use zoekt for this file search since that would require an index for the specific revision you are searching. This will currently always be the case (since you cannot browse revisions that aren't indexed), but I'd ideally like to support browsing any revision (regardless if it is indexed or not). This would unlock use-cases such as permalinks to specific commit SHAs.
Summary by CodeRabbit
New Features
Style
Documentation