perf: hide main window from alt tab program switcher #3241
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 pull request introduces enhancements for managing window visibility in the Alt+Tab interface. In the 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 (1)
Flow.Launcher/Helper/WindowsInteropHelper.cs (1)
184-223: Add error handling for window style operations.While the implementation is correct, consider adding error handling for failed window style operations. The SetWindowLong API can fail, and it's important to handle such cases gracefully.
public static void HideFromAltTab(Window window) { var helper = new WindowInteropHelper(window); var exStyle = GetWindowLong(helper.Handle, GWL_EXSTYLE).ToInt32(); + if (exStyle == 0) + { + throw new Win32Exception(Marshal.GetLastPInvokeError()); + } // Add TOOLWINDOW style, remove APPWINDOW style exStyle = (exStyle | WS_EX_TOOLWINDOW) & ~WS_EX_APPWINDOW; - SetWindowLong(helper.Handle, GWL_EXSTYLE, new IntPtr(exStyle)); + var result = SetWindowLong(helper.Handle, GWL_EXSTYLE, new IntPtr(exStyle)); + if (result == IntPtr.Zero) + { + throw new Win32Exception(Marshal.GetLastPInvokeError()); + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Flow.Launcher/Helper/WindowsInteropHelper.cs(1 hunks)Flow.Launcher/MainWindow.xaml(1 hunks)Flow.Launcher/MainWindow.xaml.cs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: gitStream.cm
🔇 Additional comments (4)
Flow.Launcher/Helper/WindowsInteropHelper.cs (2)
152-169: LGTM! Constants and PInvoke declarations are well-defined.The constants and PInvoke declarations are correctly implemented with proper values and attributes.
170-182: LGTM! Architecture-aware implementation.The SetWindowLong helper method correctly handles both 32-bit and 64-bit architectures with proper error handling.
Flow.Launcher/MainWindow.xaml (1)
23-23: LGTM! Event handler is properly integrated.The SourceInitialized event handler is correctly added and follows the existing pattern of window event handlers.
Flow.Launcher/MainWindow.xaml.cs (1)
174-177: LGTM! Event handler is correctly implemented.The OnSourceInitialized event handler is properly implemented to hide the window from Alt+Tab during initialization.
ZGGSONG
changed the title
This comment has been minimized.
This comment has been minimized.
|
LGTM. And I have improved code quality with CSWin32. |
|
🥷 Code experts: Jack251970, onesounds Jack251970 has 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:
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: 2
♻️ Duplicate comments (1)
Flow.Launcher/Helper/WindowsInteropHelper.cs (1)
181-194: 🛠️ Refactor suggestionAdd error handling for SetWindowLong failure.
Similar to HideFromAltTab, this method should handle potential failures when modifying window styles.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Flow.Launcher/Helper/WindowsInteropHelper.cs(1 hunks)Flow.Launcher/NativeMethods.txt(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
🔇 Additional comments (3)
Flow.Launcher/Helper/WindowsInteropHelper.cs (2)
154-160: LGTM! Good error handling practice.The method properly clears any existing error before calling SetWindowLong, following best practices for P/Invoke error handling.
161-164: Verify potential data loss on 64-bit systems.The unchecked conversion from IntPtr to Int32 could potentially lose data on 64-bit systems if the pointer value exceeds Int32.MaxValue.
Consider adding a check to ensure the value fits within Int32 range:
private static int IntPtrToInt32(IntPtr intPtr) { + var int64Value = intPtr.ToInt64(); + if (int64Value > int.MaxValue || int64Value < int.MinValue) + { + throw new OverflowException("IntPtr value is outside the range of Int32"); + } return unchecked((int)intPtr.ToInt64()); }Flow.Launcher/NativeMethods.txt (1)
17-20: LGTM! Required constants and functions are properly declared.The added constants and functions align with the implementation in WindowsInteropHelper.cs.
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, or 📝 job summary for details.
See ❌ Event descriptions for more information. Forbidden patterns 🙅 (1)In order to address this, you could change the content to not match the forbidden patterns (comments before forbidden patterns may help explain why they're forbidden), add patterns for acceptable instances, or adjust the forbidden patterns themselves. These forbidden patterns matched content: s.b. workaround(s)If the flagged items are 🤯 false positivesIf items relate to a ...
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
Flow.Launcher/Helper/WindowsInteropHelper.cs (4)
171-179: Add parameter validation.The method should validate the input parameter to prevent potential null reference exceptions.
Apply this diff to add parameter validation:
public static void HideFromAltTab(Window window) { + if (window == null) + { + throw new ArgumentNullException(nameof(window)); + } + var exStyle = GetCurrentWindowStyle(window); // Add TOOLWINDOW style, remove APPWINDOW style var newExStyle = ((uint)exStyle | (uint)WINDOW_EX_STYLE.WS_EX_TOOLWINDOW) & ~(uint)WINDOW_EX_STYLE.WS_EX_APPWINDOW; SetWindowLong(new(new WindowInteropHelper(window).Handle), WINDOW_LONG_PTR_INDEX.GWL_EXSTYLE, (int)newExStyle); }
185-193: Add parameter validation.The method should validate the input parameter to prevent potential null reference exceptions.
Apply this diff to add parameter validation:
public static void ShowInAltTab(Window window) { + if (window == null) + { + throw new ArgumentNullException(nameof(window)); + } + var exStyle = GetCurrentWindowStyle(window); // Remove the TOOLWINDOW style and add the APPWINDOW style. var newExStyle = ((uint)exStyle & ~(uint)WINDOW_EX_STYLE.WS_EX_TOOLWINDOW) | (uint)WINDOW_EX_STYLE.WS_EX_APPWINDOW; SetWindowLong(new(new WindowInteropHelper(window).Handle), WINDOW_LONG_PTR_INDEX.GWL_EXSTYLE, (int)newExStyle); }
200-208: Add parameter validation.The method should validate the input parameter to prevent potential null reference exceptions.
Apply this diff to add parameter validation:
private static int GetCurrentWindowStyle(Window window) { + if (window == null) + { + throw new ArgumentNullException(nameof(window)); + } + var style = PInvoke.GetWindowLong(new(new WindowInteropHelper(window).Handle), WINDOW_LONG_PTR_INDEX.GWL_EXSTYLE); if (style == 0 && Marshal.GetLastPInvokeError() != 0) { throw new Win32Exception(Marshal.GetLastPInvokeError()); } return style; }
152-210: Enhance documentation and add unit tests.The implementation is solid, but consider the following improvements:
- Document potential exceptions in the XML comments.
- Add unit tests to verify the behavior of these methods.
Would you like me to:
- Generate enhanced XML documentation with exception details?
- Create unit tests for these methods?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Flow.Launcher/Helper/WindowsInteropHelper.cs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
🔇 Additional comments (1)
Flow.Launcher/Helper/WindowsInteropHelper.cs (1)
154-165: LGTM! Robust error handling implementation.The method follows Win32 API best practices by clearing any existing error before making the call and properly checking both the return value and the last error code.
As a tool, used as needed and simply come away, one indeed need not utilize Alt-Tab to switch pages, which also affects user usage of Alt-Tab for switching between applications.
This issue was also raised in issue #2356.