Fix Copy Action Issue #3165
Fix Copy Action Issue #3165
Conversation
@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 ...
|
|
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
📝 WalkthroughWalkthroughThis pull request involves removing Single Threaded Apartment (STA) related methods and simplifying clipboard handling across multiple files in the Flow Launcher infrastructure. The changes primarily focus on eliminating asynchronous task management for COM interactions, specifically removing Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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 (
|
![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 (3)
Flow.Launcher/PublicAPIInstance.cs (3)
125-133: Handle non-existent paths gracefully.
WhendirectCopyis true, the code checksFile.ExistsorDirectory.Existsand proceeds. If both checks fail, there is no fallback path for a malformed or non-existent path string. Consider adding an early return or user notification for invalid paths.+ if (!isFile && !Directory.Exists(stringToCopy)) + { + ShowMsg("Copy Failed", "Invalid file or directory path."); + return; + }
135-139: Retain user orientation in notifications.
The new localized message references “file” or “folder” but does not explicitly mention what was copied. Consider displaying the actual path in the notification to reassure the user.
140-148: Use Clipboard.SetText(...) when copying strings.
Clipboard.SetDataObject(stringToCopy)works, yetClipboard.SetText(stringToCopy)is more direct and descriptive. Using a specialized method may be clearer and can benefit from additional text-specific optimizations in certain environments.- Clipboard.SetDataObject(stringToCopy); + Clipboard.SetText(stringToCopy);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
Flow.Launcher.Infrastructure/NativeMethods.txt(1 hunks)Flow.Launcher.Infrastructure/UserSettings/Settings.cs(1 hunks)Flow.Launcher.Infrastructure/Win32Helper.cs(0 hunks)Flow.Launcher/PublicAPIInstance.cs(1 hunks)
💤 Files with no reviewable changes (1)
- Flow.Launcher.Infrastructure/Win32Helper.cs
✅ Files skipped from review due to trivial changes (1)
- Flow.Launcher.Infrastructure/NativeMethods.txt
🔇 Additional comments (2)
Flow.Launcher/PublicAPIInstance.cs (1)
120-124: Ensure thread context consistency for clipboard operations.
Removingasyncin favor of a synchronous call is acceptable if this method is always called from an STA thread (which WPF typically provides). However, if this method is ever invoked on a non-STA thread, clipboard operations may fail.Would you like to verify or confirm that all code paths invoking
CopyToClipboardoriginate from STA threads?✅ Verification successful
Let me gather more information about the thread context by examining the call sites.
Thread safety for clipboard operations is maintained through WPF's STA context
Based on the analysis of the codebase:
- The application is a WPF application (confirmed by MainWindow.xaml), which means the main UI thread is automatically running in STA mode.
- All direct calls to
CopyToClipboardare made from:
- UI event handlers in MainWindow.xaml.cs (OnCopy event)
- ViewModel commands in MainViewModel.cs
- Plugin actions that are executed through the WPF command system
- The WPF command system ensures that commands are executed on the UI thread, which maintains the STA context required for clipboard operations.
The removal of
asyncis safe because:
- All calls to
CopyToClipboardare made through WPF's command and event system- WPF automatically ensures these operations run on the STA UI thread
- No background thread usage was found in relation to clipboard operations
The code is correctly handling clipboard operations within the appropriate thread context.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # This script will search all references of CopyToClipboard calls to check if they're invoked only from the UI thread or any guaranteed STA context. rg -A 5 "CopyToClipboard"Length of output: 6872
Script:
#!/bin/bash # Check MainWindow.xaml to verify if it's a WPF window cat Flow.Launcher/MainWindow.xaml # Check for any background/async operations in MainViewModel ast-grep --pattern 'Task $_ CopyToClipboard' # Look for any background thread or Task.Run usage near CopyToClipboard calls rg -B 5 "CopyToClipboard"Length of output: 38317
Flow.Launcher.Infrastructure/UserSettings/Settings.cs (1)
233-233: Confirm no asynchronous constraints remain.
Replacing()=> Win32Helper.StartSTATaskAsync(Clipboard.GetText).ResultwithClipboard.GetTextis simpler. Ensure that synchronous retrieval here does not block the UI if large data is present in the clipboard.
jjw24
added
Dev branch only
Revert "Fix clipboard action under sta thread issue" to fix copy action issue in #3122.