Repository navigation
fix(v3/windows): pass HWND to AllowDarkModeForWindow - #5877
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Walkthrough
ChangesWindows dark-mode syscall
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" 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 |
There was a problem hiding this comment.
Pull request overview
Fixes the Windows dark-mode uxtheme call ABI by passing the missing HWND argument to ordinal 133 (AllowDarkModeForWindow), and adds a Windows-only regression test to ensure the native argument order is preserved.
Changes:
- Pass both
HWNDand the allow flag tosyscall.SyscallN(procAllowDarkModeForWindow, ...). - Add a Windows-only AST-based test that asserts the
SyscallNinvocation uses(proc, hwnd, allowInt).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| v3/pkg/w32/theme.go | Fixes the AllowDarkModeForWindow syscall ABI by adding the missing HWND argument. |
| v3/pkg/w32/theme_test.go | Adds a Windows-only AST regression test verifying SyscallN argument count and order. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import ( | ||
| "go/ast" | ||
| "go/parser" | ||
| "go/token" | ||
| "testing" | ||
| ) | ||
|
|
||
| func TestAllowDarkModeForWindowPassesHWNDAndAllowFlag(t *testing.T) { | ||
| file, err := parser.ParseFile(token.NewFileSet(), "theme.go", nil, 0) | ||
| if err != nil { | ||
| t.Fatalf("parse theme.go: %v", err) | ||
| } |
|
I was just about to file this. I have another few related issues PRs that I will open too. |
|
Reviewed for Beta 3. The native call fix is correct and should be included. The Windows CI failure is isolated to the new test: importing |
a6f1cab to
6063058
Compare
6063058 to
bd3e702
Compare
… HWND to AllowDarkModeForWindow
Summary
Follow-up to #5873.
HWNDand the allow flagSyscallNcall preserves the native ABI argument orderWhy
The wrapper was declared as
AllowDarkModeForWindow(HWND, bool)but invokedSyscallNwith only the Boolean value. Lowering the uxtheme gate in #5873 expanded that broken call path to Windows build 17763, so this corrects the ABI immediately while keeping the original fix independently merged.Testing
CI should exercise the Windows package and the new regression test.
Summary by CodeRabbit
Bug Fixes
Tests