Fix file explorer invocation to ensure correct file selection behavior #3590
Conversation
This comment has been minimized.
This comment has been minimized.
|
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
📝 WalkthroughWalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant PublicAPIInstance
participant WindowsShell
participant CustomFileManager
Caller->>PublicAPIInstance: OpenDirectory(directoryPath, fileNameOrFilePath)
alt Using default Windows Explorer and file specified
PublicAPIInstance->>WindowsShell: SHParseDisplayName(folderPath)
WindowsShell-->>PublicAPIInstance: folderPIDL
PublicAPIInstance->>WindowsShell: SHParseDisplayName(filePath)
WindowsShell-->>PublicAPIInstance: filePIDL
PublicAPIInstance->>WindowsShell: SHOpenFolderAndSelectItems(folderPIDL, filePIDL)
else Using default Windows Explorer and no file specified
PublicAPIInstance->>WindowsShell: Start explorer.exe with folderPath
else Using custom file manager
PublicAPIInstance->>CustomFileManager: Start process with replaced arguments
end
PublicAPIInstance-->>Caller: Completion or error handling
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ 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 (
|
Jack251970
added
Dev branch only
…tion using SHOpenFolderAndSelectItems
This comment has been minimized.
This comment has been minimized.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
Flow.Launcher/PublicAPIInstance.cs (1)
371-437: 💡 Verification agent🧩 Analysis chain
Replace Korean comments with English and improve code organization.
The method logic is correct and handles both default and custom explorers appropriately. However, there are Korean comments that should be replaced with English for better maintainability.
Apply this diff to replace Korean comments with English:
if (fileNameOrFilePath is null) { - // 폴더만 열기 + // Open folder only Process.Start(new ProcessStartInfo { FileName = directoryPath, UseShellExecute = true })?.Dispose(); } else { - // SHOpenFolderAndSelectItems 방식 + // Use SHOpenFolderAndSelectItems approach OpenFolderAndSelectItem(targetPath); } } else { - // 커스텀 파일 관리자 + // Custom file manager var shellProcess = new ProcessStartInfoVerify the fix resolves the original issue.
The implementation correctly addresses the original problem by using Shell APIs when the default explorer is used with a file path, which should open the containing folder and select the file instead of launching the file directly.
Let me verify the fix works as intended:
🏁 Script executed:
#!/bin/bash # Description: Verify the fix addresses the original issue by checking the logic flow # Check if the method correctly handles the file vs folder logic ast-grep --pattern $'if (fileNameOrFilePath is null) { $$$ } else { OpenFolderAndSelectItem($$$); }' # Verify the custom explorer handling remains intact ast-grep --pattern $'var shellProcess = new ProcessStartInfo { FileName = $$$, UseShellExecute = true, Arguments = $$$ };'Length of output: 1191
I’d like to confirm that all Korean comments have been removed. Running a quick check on the current snippet:
🏁 Script executed:
#!/bin/bash # Show lines 350–440 to inspect comments sed -n '350,440p' Flow.Launcher/PublicAPIInstance.cs # Search for any remaining Korean (Hangul) in comments grep -nP '//.*[\x{AC00}-\x{D7A3}]' Flow.Launcher/PublicAPIInstance.cs || echo 'No Korean comments found'Length of output: 3833
Replace remaining Korean comments with English
The
OpenDirectorymethod inFlow.Launcher/PublicAPIInstance.csstill contains Korean comments that need replacing. Please apply the following changes:--- a/Flow.Launcher/PublicAPIInstance.cs +++ b/Flow.Launcher/PublicAPIInstance.cs @@ -378,9 +378,9 @@ public void OpenDirectory(string directoryPath, string fileNameOrFilePath = nul if (fileNameOrFilePath is null) { - // 폴더만 열기 + // Open folder only Process.Start(new ProcessStartInfo { FileName = directoryPath, UseShellExecute = true })?.Dispose(); } else { - // SHOpenFolderAndSelectItems 방식 + // Use SHOpenFolderAndSelectItems approach OpenFolderAndSelectItem(targetPath); } @@ -386,7 +386,7 @@ public void OpenDirectory(string directoryPath, string fileNameOrFilePath = nul } else { - // 커스텀 파일 관리자 + // Custom file manager var shellProcess = new ProcessStartInfo { FileName = explorerInfo.Path.Replace("%d", directoryPath),These updates ensure all inline comments are in English for maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Flow.Launcher/PublicAPIInstance.cs(3 hunks)
🧰 Additional context used
🪛 GitHub Actions: Check Spelling
Flow.Launcher/PublicAPIInstance.cs
[warning] 54-54: Spell check warning: Ioc is not a recognized word. (unrecognized-spelling)
[warning] 152-152: Spell check warning: VSTHRD is not a recognized word. (unrecognized-spelling)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: gitStream.cm
- GitHub Check: build
🔇 Additional comments (1)
Flow.Launcher/PublicAPIInstance.cs (1)
11-11: LGTM: Required using statement for P/Invoke declarations.The addition of
System.Runtime.InteropServicesnamespace is necessary for the P/Invoke declarations that follow.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the file explorer behavior so that when “Open Containing Folder” is used on a file, the file’s containing directory is opened and the file gets selected (when supported). Key changes include renaming parameters for consistency, splitting the logic between Windows and custom file managers, and adding a native helper method for folder/file selection.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| Flow.Launcher/PublicAPIInstance.cs | Updated OpenDirectory to correctly handle file vs folder paths and improved parameter naming conventions. |
| Flow.Launcher.Infrastructure/Win32Helper.cs | Introduced OpenFolderAndSelectFile with appropriate COM invocations and added required using directives. |
| Flow.Launcher.Infrastructure/NativeMethods.txt | Registered new native method entries required for file selection in the Windows File Manager. |
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, or 📝 job summary for details.
See ❌ Event descriptions for more information. If the flagged items are 🤯 false positivesIf items relate to a ...
|
|
🥷 Code experts: Jack251970 Jack251970, onesounds have most 👩💻 activity in the files. See details
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame: To learn more about /:\ gitStream - Visit our Docs |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Flow.Launcher.Infrastructure/Win32Helper.cs (1)
768-768:⚠️ Potential issueAddress the validation concern from previous review.
As noted in the previous review,
Path.GetDirectoryName(filePath)can return null if the file path doesn't include a directory component, which would cause issues when passed toSHParseDisplayName.var folderPath = Path.GetDirectoryName(filePath); +if (string.IsNullOrEmpty(folderPath)) + throw new ArgumentException("Unable to determine directory from file path.", nameof(filePath));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Flow.Launcher.Infrastructure/NativeMethods.txt(1 hunks)Flow.Launcher.Infrastructure/Win32Helper.cs(3 hunks)Flow.Launcher/PublicAPIInstance.cs(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- Flow.Launcher.Infrastructure/NativeMethods.txt
🚧 Files skipped from review as they are similar to previous changes (1)
- Flow.Launcher/PublicAPIInstance.cs
🧰 Additional context used
🪛 GitHub Actions: Check Spelling
Flow.Launcher.Infrastructure/Win32Helper.cs
[warning] 19-33: Dwm is not a recognized word. (unrecognized-spelling)
[warning] 43-31: Dwm is not a recognized word. (unrecognized-spelling)
[warning] 43-27: PInvoke is not a recognized word. (unrecognized-spelling)
[warning] 54-60: SYSTEMBACKDROP is not a recognized word. (unrecognized-spelling)
[warning] 55-69: DWMSBT is not a recognized word. (unrecognized-spelling)
[warning] 55-57: SYSTEMBACKDROP is not a recognized word. (unrecognized-spelling)
[warning] 56-72: DWMSBT is not a recognized word. (unrecognized-spelling)
[warning] 56-60: SYSTEMBACKDROP is not a recognized word. (unrecognized-spelling)
[warning] 57-52: DWMSBT is not a recognized word. (unrecognized-spelling)
[warning] 57-40: SYSTEMBACKDROP is not a recognized word. (unrecognized-spelling)
[warning] 60-31: Dwm is not a recognized word. (unrecognized-spelling)
[warning] 60-27: PInvoke is not a recognized word. (unrecognized-spelling)
[warning] 62-41: DWMWA is not a recognized word. (unrecognized-spelling)
[warning] 62-35: DWMWINDOWATTRIBUTE is not a recognized word. (unrecognized-spelling)
[warning] 71-31: Dwm is not a recognized word. (unrecognized-spelling)
[warning] 71-27: PInvoke is not a recognized word. (unrecognized-spelling)
[warning] 73-41: DWMWA is not a recognized word. (unrecognized-spelling)
[warning] 73-35: DWMWINDOWATTRIBUTE is not a recognized word. (unrecognized-spelling)
[warning] 89-63: DWMWCP is not a recognized word. (unrecognized-spelling)
[warning] 90-68: DWMWCP is not a recognized word. (unrecognized-spelling)
[warning] 91-65: DWMWCP is not a recognized word. (unrecognized-spelling)
[warning] 95-27: PInvoke is not a recognized word. (unrecognized-spelling)
[warning] 97-41: DWMWA is not a recognized word. (unrecognized-spelling)
[warning] 97-35: DWMWINDOWATTRIBUTE is not a recognized word. (unrecognized-spelling)
[warning] 163-59: GWL is not a recognized word. (unrecognized-spelling)
[warning] 174-73: GWL is not a recognized word. (unrecognized-spelling)
[warning] 179-78: GWL is not a recognized word. (unrecognized-spelling)
[warning] 190-71: GWL is not a recognized word. (unrecognized-spelling)
[warning] 199-51: Wnd is not a recognized word. (unrecognized-spelling)
[warning] 207-53: Wnd is not a recognized word. (unrecognized-spelling)
[warning] 211-55: Wnd is not a recognized word. (unrecognized-spelling)
[warning] 240-21: Wnd is not a recognized word. (unrecognized-spelling)
[warning] 264-51: WINTAB is not a recognized word. (unrecognized-spelling)
[warning] 277-36: Progman is not a recognized word. (unrecognized-spelling)
[warning] 278-76: WORKERW is not a recognized word. (unrecognized-spelling)
[warning] 518-39: hkl is not a recognized word. (unrecognized-spelling)
[warning] 524-31: hkl is not a recognized word. (unrecognized-spelling)
[warning] 550-35: nqo is not a recognized word. (unrecognized-spelling)
[warning] 623-73: tsf is not a recognized word. (unrecognized-spelling)
[warning] 646-69: tsf is not a recognized word. (unrecognized-spelling)
[warning] 646-73: tsf is not a recognized word. (unrecognized-spelling)
[warning] 647-44: Tsf is not a recognized word. (unrecognized-spelling)
[warning] 683-26: Noto is not a recognized word. (unrecognized-spelling)
[warning] 684-26: Noto is not a recognized word. (unrecognized-spelling)
[warning] 685-29: Noto is not a recognized word. (unrecognized-spelling)
[warning] 686-29: Noto is not a recognized word. (unrecognized-spelling)
[warning] 727-106: noto is not a recognized word. (unrecognized-spelling)
[warning] 730-83: noto is not a recognized word. (unrecognized-spelling)
[warning] 732-40: noto is not a recognized word. (unrecognized-spelling)
[warning] 752-75: noto is not a recognized word. (unrecognized-spelling)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
- GitHub Check: build
🔇 Additional comments (2)
Flow.Launcher.Infrastructure/Win32Helper.cs (2)
6-6: LGTM: Required using statements added correctly.The new using statements are appropriately added to support the new Explorer functionality.
Also applies to: 21-21
770-786: LGTM: Excellent implementation with proper error handling and resource management.The implementation correctly:
- Uses appropriate Windows Shell APIs (
SHParseDisplayName,SHOpenFolderAndSelectItems)- Provides proper error handling with meaningful COM exceptions
- Ensures memory cleanup with
CoTaskMemFreein the finally block- Uses unsafe code appropriately for native pointer operations
This addresses the core issue described in the PR objectives by properly opening the containing folder and selecting the file.
jjw24
added
bug
3 participants
What's the PR
Cause
When using the default explorer setting, the plugin attempted to open the full path directly—even when the path pointed to a file (including the filename itself).
This has been fixed to work correctly. The -select option now functions as expected.