fix windows directory adding etc#43
fix windows directory adding etc#43SuperComboGamer merged 2 commits intoremoving-electron-features-build-apifrom
Conversation
- Introduced a new FileBrowserProvider to manage file browsing state and functionality. - Added FileBrowserDialog component for user interface to navigate and select directories. - Updated Home component to utilize the file browser context and provide global access. - Enhanced HttpApiClient to use the new file browser for directory and file selection. - Implemented server-side route for browsing directories, including drive detection on Windows.
Summary of ChangesHello @SuperComboGamer, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the user experience for selecting directories and files by replacing the rudimentary prompt-based input with a fully interactive file browser dialog. This change involves both frontend and backend development, including new React components, a dedicated context for managing the browser's state, and a server-side API endpoint capable of listing file system contents and handling platform-specific features like Windows drives. The overall impact is a more intuitive and robust method for users to specify file paths within the application. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a new file browser dialog to replace the native prompt for selecting directories, which is a significant improvement for user experience. The implementation is well-structured, using a new React context, a dialog component, and a backend endpoint. However, I've identified a critical security vulnerability in the new backend endpoint that allows for path traversal, which must be addressed. I've also included a recommendation to improve React hooks usage for better code maintainability.
| const browseDirectory = async (dirPath?: string) => { | ||
| setLoading(true); | ||
| setError(""); | ||
|
|
||
| try { | ||
| // Get server URL from environment or default | ||
| const serverUrl = process.env.NEXT_PUBLIC_SERVER_URL || "http://localhost:3008"; | ||
|
|
||
| const response = await fetch(`${serverUrl}/api/fs/browse`, { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify({ dirPath }), | ||
| }); | ||
|
|
||
| const result: BrowseResult = await response.json(); | ||
|
|
||
| if (result.success) { | ||
| setCurrentPath(result.currentPath); | ||
| setParentPath(result.parentPath); | ||
| setDirectories(result.directories); | ||
| setDrives(result.drives || []); | ||
| } else { | ||
| setError(result.error || "Failed to browse directory"); | ||
| } | ||
| } catch (err) { | ||
| setError(err instanceof Error ? err.message : "Failed to load directories"); | ||
| } finally { | ||
| setLoading(false); | ||
| } | ||
| }; | ||
|
|
||
| // Load home directory on mount | ||
| useEffect(() => { | ||
| if (open && !currentPath) { | ||
| browseDirectory(); | ||
| } | ||
| }, [open]); |
There was a problem hiding this comment.
The browseDirectory function is redefined on every render, and the useEffect hook at line 83 has an incomplete dependency array. This violates React hooks rules and can lead to subtle bugs or stale closures if the component is refactored in the future.
To adhere to best practices, you should wrap browseDirectory in useCallback to memoize it, and provide a complete dependency array to the useEffect hook.
const browseDirectory = useCallback(async (dirPath?: string) => {
setLoading(true);
setError("");
try {
// Get server URL from environment or default
const serverUrl = process.env.NEXT_PUBLIC_SERVER_URL || "http://localhost:3008";
const response = await fetch(`${serverUrl}/api/fs/browse`, {
method: "POST",
headers: { "Content-Type": "application/json" },
body: JSON.stringify({ dirPath }),
});
const result: BrowseResult = await response.json();
if (result.success) {
setCurrentPath(result.currentPath);
setParentPath(result.parentPath);
setDirectories(result.directories);
setDrives(result.drives || []);
} else {
setError(result.error || "Failed to browse directory");
}
} catch (err) {
setError(err instanceof Error ? err.message : "Failed to load directories");
} finally {
setLoading(false);
}
}, []);
// Load home directory on mount
useEffect(() => {
if (open && !currentPath) {
browseDirectory();
}
}, [open, currentPath, browseDirectory]);
No description provided.