Reveal Parent Directories in External Applications#1791
Conversation
This only adds the options to the individual rows and the frame. It still needs to be added to the header.
Importing PLATFORM in util.ts broke the app. I am now passing it as an argument to avoid this.
Importing PLATFORM in util.ts broke the app. I am now passing it as an argument to avoid this.
This allows revealing directories and other files with external applications.
|
Warning Rate limit exceeded@oneirocosm has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 20 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThe pull request introduces enhancements to the file and directory preview functionality across multiple frontend files. Key modifications include the addition of a new utility function Changes are made in three primary files: These updates enhance the application's capability to manage native path operations effectively across different operating systems. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
While parent is trivial to compute for non-directories, it is slightly more involved for children that are also directories. This resolves that case using the RemoteFileJoinCommand
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
frontend/util/util.ts (1)
305-326: Add input validation and use platform constants.The implementation looks good but could be improved with:
- Input validation for the platform parameter
- Constants for platform strings to avoid magic strings
+const PLATFORM_DARWIN = 'darwin'; +const PLATFORM_WIN32 = 'win32'; + function makeNativeLabel(platform: string, isDirectory: boolean, isParent: boolean) { + if (!platform) { + platform = 'unknown'; + } let managerName: string; if (!isDirectory && !isParent) { managerName = "Default Application"; - } else if (platform == "darwin") { + } else if (platform === PLATFORM_DARWIN) { managerName = "Finder"; - } else if (platform == "win32") { + } else if (platform === PLATFORM_WIN32) { managerName = "Explorer"; } else { managerName = "File Manager"; }frontend/app/view/preview/directorypreview.tsx (1)
514-521: Add error type and improve error handling.The error handling for parent file info retrieval could be more robust:
- The error type should be specified
- The error message could be more descriptive
- Consider logging the actual error
try { parentFileInfo = await RpcApi.RemoteFileJoinCommand(TabRpcClient, [normPath, ".."], { route: makeConnRoute(conn), }); -} catch (e) { - console.log("could not get parent file info. using child file info as fallback"); +} catch (error: unknown) { + console.error("Failed to get parent file info, using child file info as fallback:", error); parentFileInfo = finfo; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/app/view/preview/directorypreview.tsx(4 hunks)frontend/util/util.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
- GitHub Check: Build for TestDriver.ai
🔇 Additional comments (3)
frontend/app/view/preview/directorypreview.tsx (3)
575-585: LGTM: Context menu items for native path operations.The implementation correctly uses the
makeNativeLabelutility to generate platform-specific labels for both the current item and its parent directory.
865-869: LGTM: Directory context menu implementation.The implementation correctly uses the
makeNativeLabelutility for the directory context menu.
13-13: Verify all imports of makeNativeLabel.Let's verify that makeNativeLabel is imported and used consistently across the codebase.
✅ Verification successful
makeNativeLabel imports and usage verified successfully
The function is properly imported and consistently used across the preview components for generating platform-specific labels.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for all imports of makeNativeLabel echo "Checking imports of makeNativeLabel:" rg "import.*makeNativeLabel.*from.*util" # Search for all usages of makeNativeLabel echo -e "\nChecking usages of makeNativeLabel:" rg "makeNativeLabel\("Length of output: 1043
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/app/view/preview/directorypreview.tsx (1)
520-526: Consider enhancing error handling with specific error types.The error handling for parent directory info retrieval is good but could be more specific. Consider catching specific error types to handle different failure scenarios appropriately.
- } catch (e) { + } catch (error) { + if (error instanceof TypeError) { + console.error("Network or API error:", error); + } else { + console.error("Unexpected error:", error); + } console.log("could not get parent file info. using child file info as fallback"); parentFileInfo = finfo; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/app/view/preview/directorypreview.tsx(4 hunks)frontend/app/view/preview/preview.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/app/view/preview/preview.tsx
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
- GitHub Check: Build for TestDriver.ai
🔇 Additional comments (3)
frontend/app/view/preview/directorypreview.tsx (3)
12-12: LGTM!The import statement for
makeNativeLabelis correctly added and follows the project's import style.
889-893: LGTM!The context menu implementation for opening directories in native file manager is consistent with the earlier changes and uses the makeNativeLabel function appropriately.
580-590: Address the TODO comment about local files.The TODO comment indicates that these options should only be shown for local files. This should be implemented to prevent showing these options for remote files.
Let's verify the current behavior:
Would you like me to help implement the connection type check for these menu items?
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/app/view/preview/directorypreview.tsx(7 hunks)frontend/app/view/preview/preview.tsx(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (go)
- GitHub Check: Build for TestDriver.ai
🔇 Additional comments (4)
frontend/app/view/preview/directorypreview.tsx (2)
12-12: LGTM!The import statement correctly adds the
makeNativeLabelutility function while maintaining alphabetical order.
580-590: Consider performance optimization and WSL support.
- The click handlers could be memoized using
useCallbackto prevent unnecessary re-renders.- The TODO comment about WSL support should be addressed.
Let's verify if WSL support is implemented elsewhere:
frontend/app/view/preview/preview.tsx (2)
690-709: Ensure consistent connection handling.The connection handling in the terminal block creation is consistent with the rest of the codebase.
711-717: LGTM!The implementation of native file opening is clean and follows the established patterns.
| async (e: any, finfo: FileInfo) => { | ||
| e.preventDefault(); | ||
| e.stopPropagation(); | ||
| if (finfo == null) { | ||
| return; | ||
| } | ||
| const normPath = getNormFilePath(finfo); | ||
| const fileName = finfo.path.split("/").pop(); | ||
| let openNativeLabel = "Open File"; | ||
| if (finfo.isdir) { | ||
| openNativeLabel = "Open Directory in File Manager"; | ||
| if (PLATFORM == "darwin") { | ||
| openNativeLabel = "Open Directory in Finder"; | ||
| } else if (PLATFORM == "win32") { | ||
| openNativeLabel = "Open Directory in Explorer"; | ||
| } | ||
| } else { | ||
| openNativeLabel = "Open File in Default Application"; | ||
| let parentFileInfo: FileInfo; | ||
| try { | ||
| parentFileInfo = await RpcApi.RemoteFileJoinCommand(TabRpcClient, [normPath, ".."], { | ||
| route: makeConnRoute(conn), | ||
| }); | ||
| } catch (e) { | ||
| console.log("could not get parent file info. using child file info as fallback"); | ||
| parentFileInfo = finfo; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling and parent directory fallback logic.
The error handling for parent directory info could be improved:
- The error log message is too generic and doesn't include the actual error.
- Using the child file info as a fallback might lead to incorrect behavior.
Consider this improved implementation:
try {
parentFileInfo = await RpcApi.RemoteFileJoinCommand(TabRpcClient, [normPath, ".."], {
route: makeConnRoute(conn),
});
} catch (e) {
- console.log("could not get parent file info. using child file info as fallback");
- parentFileInfo = finfo;
+ console.error(`Failed to get parent info for ${normPath}:`, e);
+ throw new Error(`Unable to access parent directory: ${e.message}`);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async (e: any, finfo: FileInfo) => { | |
| e.preventDefault(); | |
| e.stopPropagation(); | |
| if (finfo == null) { | |
| return; | |
| } | |
| const normPath = getNormFilePath(finfo); | |
| const fileName = finfo.path.split("/").pop(); | |
| let openNativeLabel = "Open File"; | |
| if (finfo.isdir) { | |
| openNativeLabel = "Open Directory in File Manager"; | |
| if (PLATFORM == "darwin") { | |
| openNativeLabel = "Open Directory in Finder"; | |
| } else if (PLATFORM == "win32") { | |
| openNativeLabel = "Open Directory in Explorer"; | |
| } | |
| } else { | |
| openNativeLabel = "Open File in Default Application"; | |
| let parentFileInfo: FileInfo; | |
| try { | |
| parentFileInfo = await RpcApi.RemoteFileJoinCommand(TabRpcClient, [normPath, ".."], { | |
| route: makeConnRoute(conn), | |
| }); | |
| } catch (e) { | |
| console.log("could not get parent file info. using child file info as fallback"); | |
| parentFileInfo = finfo; | |
| async (e: any, finfo: FileInfo) => { | |
| e.preventDefault(); | |
| e.stopPropagation(); | |
| if (finfo == null) { | |
| return; | |
| } | |
| const normPath = getNormFilePath(finfo); | |
| const fileName = finfo.path.split("/").pop(); | |
| let parentFileInfo: FileInfo; | |
| try { | |
| parentFileInfo = await RpcApi.RemoteFileJoinCommand(TabRpcClient, [normPath, ".."], { | |
| route: makeConnRoute(conn), | |
| }); | |
| } catch (e) { | |
| console.error(`Failed to get parent info for ${normPath}:`, e); | |
| throw new Error(`Unable to access parent directory: ${e.message}`); | |
| } |
Adds context menu options to the directory preview to open the parent directory in the native file viewer. Additionally, it adds context menu options in the block header to open either a parent directory or a different type of file in an external default application. These context menu items are only available for local directory previews.
1 participant
Adds context menu options to the directory preview to open the parent directory in the native file viewer. Additionally, it adds context menu options in the block header to open either a parent directory or a different type of file in an external default application.