Expand DevTools adding test windows#4504
Conversation
a01ef43 to
46dbd1e
Compare
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 3 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
📝 WalkthroughWalkthroughAdds devtools test-window functionality to the About settings page: new localized strings for window titles and message-box labels, view model commands to open test report/progress/plugin-update windows and configurable test message boxes, and restructured settings UI with a SettingsExpander exposing these actions. ChangesDevtools Test Windows Feature
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant SettingsPaneAboutViewModel
participant ProgressWindow
User->>SettingsPaneAboutViewModel: OpenTestProgressWindowAsync()
SettingsPaneAboutViewModel->>ProgressWindow: Show progress window
loop progress updates
SettingsPaneAboutViewModel->>ProgressWindow: Update progress value
end
User->>SettingsPaneAboutViewModel: Cancel
SettingsPaneAboutViewModel->>SettingsPaneAboutViewModel: Stop loop and suppress OperationCanceledException
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
fb6b814 to
716dc54
Compare
…ying all the buttons it had as individual SettingsCards
…end result as notification
…ools The old Wizard name is still left as the setting card header
716dc54 to
b39d210
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Flow.Launcher.Infrastructure/Image/ThumbnailReader.cs`:
- Around line 92-95: The HRESULT handling in ThumbnailReader’s shell item
creation path should not throw the result of GetExceptionForHR, because
success-style codes like S_FALSE can return null and lead to an unintended
NullReferenceException. Update the check around SHCreateItemFromParsingName in
ThumbnailReader to use Marshal.ThrowExceptionForHR(retCode) instead of throwing
the value returned by Marshal.GetExceptionForHR, keeping the existing retCode
validation and behavior aligned with the API semantics.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 943b2ace-52d1-41d7-9937-63f43db0c65e
📒 Files selected for processing (12)
Flow.Launcher.Infrastructure/Flow.Launcher.Infrastructure.csprojFlow.Launcher.Infrastructure/Image/ThumbnailReader.csFlow.Launcher.Infrastructure/packages.lock.jsonFlow.Launcher.Plugin/Flow.Launcher.Plugin.csprojFlow.Launcher.Plugin/packages.lock.jsonFlow.Launcher/Languages/en.xamlFlow.Launcher/SettingPages/ViewModels/SettingsPaneAboutViewModel.csFlow.Launcher/SettingPages/Views/SettingsPaneAbout.xamlPlugins/Flow.Launcher.Plugin.ProcessKiller/Flow.Launcher.Plugin.ProcessKiller.csprojPlugins/Flow.Launcher.Plugin.Program/Flow.Launcher.Plugin.Program.csprojPlugins/Flow.Launcher.Plugin.Program/Programs/ShellLocalization.csPlugins/Flow.Launcher.Plugin.Sys/Flow.Launcher.Plugin.Sys.csproj
✅ Files skipped from review due to trivial changes (3)
- Plugins/Flow.Launcher.Plugin.Program/Flow.Launcher.Plugin.Program.csproj
- Flow.Launcher.Plugin/Flow.Launcher.Plugin.csproj
- Flow.Launcher.Infrastructure/packages.lock.json
🚧 Files skipped from review as they are similar to previous changes (3)
- Flow.Launcher/SettingPages/ViewModels/SettingsPaneAboutViewModel.cs
- Flow.Launcher/SettingPages/Views/SettingsPaneAbout.xaml
- Flow.Launcher/Languages/en.xaml
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Flow.Launcher.Infrastructure/Image/ThumbnailReader.cs`:
- Around line 92-95: The HRESULT handling in ThumbnailReader’s shell item
creation path should not throw the result of GetExceptionForHR, because
success-style codes like S_FALSE can return null and lead to an unintended
NullReferenceException. Update the check around SHCreateItemFromParsingName in
ThumbnailReader to use Marshal.ThrowExceptionForHR(retCode) instead of throwing
the value returned by Marshal.GetExceptionForHR, keeping the existing retCode
validation and behavior aligned with the API semantics.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 943b2ace-52d1-41d7-9937-63f43db0c65e
📒 Files selected for processing (12)
Flow.Launcher.Infrastructure/Flow.Launcher.Infrastructure.csprojFlow.Launcher.Infrastructure/Image/ThumbnailReader.csFlow.Launcher.Infrastructure/packages.lock.jsonFlow.Launcher.Plugin/Flow.Launcher.Plugin.csprojFlow.Launcher.Plugin/packages.lock.jsonFlow.Launcher/Languages/en.xamlFlow.Launcher/SettingPages/ViewModels/SettingsPaneAboutViewModel.csFlow.Launcher/SettingPages/Views/SettingsPaneAbout.xamlPlugins/Flow.Launcher.Plugin.ProcessKiller/Flow.Launcher.Plugin.ProcessKiller.csprojPlugins/Flow.Launcher.Plugin.Program/Flow.Launcher.Plugin.Program.csprojPlugins/Flow.Launcher.Plugin.Program/Programs/ShellLocalization.csPlugins/Flow.Launcher.Plugin.Sys/Flow.Launcher.Plugin.Sys.csproj
✅ Files skipped from review due to trivial changes (3)
- Plugins/Flow.Launcher.Plugin.Program/Flow.Launcher.Plugin.Program.csproj
- Flow.Launcher.Plugin/Flow.Launcher.Plugin.csproj
- Flow.Launcher.Infrastructure/packages.lock.json
🚧 Files skipped from review as they are similar to previous changes (3)
- Flow.Launcher/SettingPages/ViewModels/SettingsPaneAboutViewModel.cs
- Flow.Launcher/SettingPages/Views/SettingsPaneAbout.xaml
- Flow.Launcher/Languages/en.xaml
🛑 Comments failed to post (1)
Flow.Launcher.Infrastructure/Image/ThumbnailReader.cs (1)
92-95: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does Microsoft.Windows.CsWin32 0.3.298 generate a generic SHCreateItemFromParsingName<T> PInvoke overload?💡 Result:
No, Microsoft.Windows.CsWin32 version 0.3.298 does not generate a generic SHCreateItemFromParsingName P/Invoke overload [1][2]. The generated P/Invoke signatures for SHCreateItemFromParsingName in CsWin32 typically match the native Win32 API signature, which uses an IntPtr (or void*) for the output parameter and requires an explicit GUID for the requested interface ID (IID) [1][2]. While CsWin32 generates various "friendly overloads" for many APIs to improve ergonomics—such as using Span or SafeHandle types—it does not automatically generate generic T-based wrappers for methods that require manual IID management like SHCreateItemFromParsingName [1][3][4]. Developers needing a more idiomatic C# experience for such APIs often create their own partial class or helper methods to wrap the CsWin32-generated P/Invoke, manually handling the IID_PPV_ARGS pattern or casting to the desired interface [1][5][2].
Citations:
- 1: https://microsoft.github.io/CsWin32/docs/Examples.html
- 2: https://context7.com/microsoft/cswin32/llms.txt
- 3: https://microsoft.github.io/CsWin32/docs/getting-started.html
- 4: https://deepwiki.com/microsoft/CsWin32/2.4-friendly-overloads
- 5: microsoft/CsWin32#434
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the target file around the commented lines and locate related usages. file='Flow.Launcher.Infrastructure/Image/ThumbnailReader.cs' wc -l "$file" sed -n '1,220p' "$file" printf '\n--- references ---\n' rg -n "SHCreateItemFromParsingName|GetExceptionForHR|ThrowExceptionForHR|HRESULT\.S_OK" Flow.Launcher.Infrastructure -SRepository: Flow-Launcher/Flow.Launcher
Length of output: 9216
🌐 Web query:
Marshal.GetExceptionForHR returns null only for S_OK or for any successful HRESULT values; can throwing its result cause null exception for S_FALSE?💡 Result:
Marshal.GetExceptionForHR returns null for any HRESULT that does not represent an error [1]. By definition, an HRESULT is considered an error only if its high-order bit is set (i.e., the value is less than 0) [2]. Because S_FALSE (which has a value of 1) and S_OK (which has a value of 0) are both non-negative, Marshal.GetExceptionForHR treats them as successful return codes and returns null [1][3]. Regarding your second question: throwing the result of Marshal.GetExceptionForHR when it returns null will cause a NullReferenceException. Since the method returns null for S_FALSE, passing the result of such a call directly into a throw statement (e.g., throw Marshal.GetExceptionForHR(S_FALSE);) will result in a NullReferenceException, as the throw keyword cannot throw a null object [2]. It is standard practice to check if the returned exception object is null before attempting to throw it.
Citations:
- 1: https://learn.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.marshal.getexceptionforhr?view=netframework-4.8.1
- 2: dotnet/runtime#117568
- 3: https://github.com/dotnet/runtime/blob/5535e31a712343a63f5d7d796cd874e563e5ac14/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.cs
🌐 Web query:
Microsoft docs Marshal.GetExceptionForHR behavior S_FALSE null exception Marshal.ThrowExceptionForHR💡 Result:
The behavior of Marshal.GetExceptionForHR and Marshal.ThrowExceptionForHR regarding HRESULT values, including S_FALSE, is defined as follows: Marshal.GetExceptionForHR This method returns an Exception object if the HRESULT represents a failure code [1][2]. If the HRESULT value does not represent an error code—specifically if it indicates success (such as S_OK, where HRESULT is 0, or S_FALSE, where HRESULT is 1)—the method returns null [1][2]. Developers should verify that the returned exception is not null before attempting to use it [1][2]. Marshal.ThrowExceptionForHR This method is designed to throw an exception for a failure HRESULT [3][4]. If the HRESULT provided is 0 or positive (indicating success, such as S_OK or S_FALSE), the method returns without creating or throwing an exception [3][4]. In summary, because S_FALSE is considered a success code in COM (HRESULT 0x00000001), it is treated by both methods as a non-error state, resulting in no exception being thrown or returned [1][3].
Citations:
- 1: https://learn.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.marshal.getexceptionforhr?view=net-10.0
- 2: https://learn.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.marshal.getexceptionforhr?view=netframework-4.8.1
- 3: https://learn.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.marshal.throwexceptionforhr?view=netframework-4.8.1
- 4: https://learn.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.marshal.throwexceptionforhr?view=net-10.0
Avoid throwing the result of
GetExceptionForHRhere.
S_FALSEis a success code, butMarshal.GetExceptionForHR(retCode)returnsnullfor it, sothrow nullwould surface as aNullReferenceException. UseMarshal.ThrowExceptionForHR(retCode)instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Flow.Launcher.Infrastructure/Image/ThumbnailReader.cs` around lines 92 - 95, The HRESULT handling in ThumbnailReader’s shell item creation path should not throw the result of GetExceptionForHR, because success-style codes like S_FALSE can return null and lead to an unintended NullReferenceException. Update the check around SHCreateItemFromParsingName in ThumbnailReader to use Marshal.ThrowExceptionForHR(retCode) instead of throwing the value returned by Marshal.GetExceptionForHR, keeping the existing retCode validation and behavior aligned with the API semantics.
2 participants
Add new dev tools to open test windows so their behavior and look can be verified at runtime
These specific windows are usually hard to trigger or unsafe to use if you don't want to mess with your configuration.
The progress window has a simulated timer (1m) thats short enough to see change but long enough to get a look at it.
The message box windows are available in all the supported button types and send the result back as a notification - so you can test its behavior.
UI Changes
To make room for all these new test windows I turned DevTools into an expander
Before
After
(Messsage Box Type Flyout)
Summary by cubic
Replaces the DevTools flyout with a structured expander and adds test windows for Report, Progress (cancelable), Plugin Update, and Message Box. Improves discoverability and lets you verify these UIs safely.
Summary of changes
SettingsCard+ flyout into aSettingsExpanderwith individualSettingsCards; use sharedopenFolderButtonfor settings/log/cache folders; welcome button label is now “Open Welcome Window” (card header still “Wizard”).openFolderButton.Release Note
About > DevTools now uses an expander with clear actions and adds test windows for reports, progress, plugin updates, and message boxes.
Written for commit b39d210. Summary will update on new commits.