Improve Arch Linux support and fix video export on Hyprland#484
Improve Arch Linux support and fix video export on Hyprland#484psychosomat wants to merge 2 commits intosiddharthvaddem:mainfrom
Conversation
- Add pacman package build target for Arch Linux in electron-builder.json5 - Update build:linux script in package.json to include pacman target - Fix dialog window issues on Wayland/Hyprland: * Pass mainWindow reference to dialog.showSaveDialog and dialog.showOpenDialog in electron/ipc/handlers.ts * Required for proper dialog functionality on Wayland compositors * Previously dialogs opened without parent window attachment causing issues on Hyprland Changes ensure: - Correct video export on Arch Linux + Hyprland systems - Ability to install via pacman package manager - Improved compatibility with Wayland compositors
📝 WalkthroughWalkthroughDetect Wayland sessions on Linux and add corresponding Electron command-line flags; make file dialogs attach to the main BrowserWindow when available; and extend Linux packaging targets to include Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 31f0483c65
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "build:mac": "tsc && vite build && electron-builder --mac", | ||
| "build:win": "tsc && vite build && electron-builder --win", | ||
| "build:linux": "tsc && vite build && electron-builder --linux AppImage deb", | ||
| "build:linux": "tsc && vite build && electron-builder --linux AppImage deb pacman", |
There was a problem hiding this comment.
This change adds pacman to the Linux build targets, but the build-linux workflow still uploads only AppImage/zsync/deb artifacts (.github/workflows/build.yml, upload step at lines 245-253). That means the new pacman package is built and then discarded, so Arch users cannot access the package from CI outputs and release automation tied to these artifacts. Please include the pacman output pattern in the upload paths.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@electron/ipc/handlers.ts`:
- Around line 839-856: The dialog calls duplicate the options and pass
getMainWindow() directly which may return a stale/destroyed BrowserWindow;
create a small helper (e.g., buildDialogOptions or attachParentIfAlive) that
accepts the base options object and will attach a valid parent only when
getMainWindow() exists and !mainWindow.isDestroyed(), then use that helper for
dialog.showSaveDialog and the other dialog callers so you build the options once
(title, defaultPath, filters, properties) and only add a parent: mainWindow when
safe instead of passing getMainWindow() directly in
showSaveDialog/showOpenDialog call sites referenced here.
- Around line 362-364: The supportsWindowOpacity flag incorrectly treats Wayland
the same as non-Linux platforms; update the assignment of supportsWindowOpacity
(which currently references isWayland) to exclude Wayland so Linux always
follows the no-opacity codepath: remove the "|| isWayland" portion where
supportsWindowOpacity is computed, leaving isWayland as a separate boolean and
ensuring subsequent logic in the code that calls BrowserWindow.setOpacity(0) and
win.hide() (refer to supportsWindowOpacity, isWayland, BrowserWindow.setOpacity,
and win.hide) will run the Linux fallback path.
In `@electron/main.ts`:
- Line 41: Replace the incorrect Chromium feature name "PipeWire" used in
app.commandLine.appendSwitch("enable-features", "...") with the correct feature
"WebRTCPipeWireCapturer" so the call to app.commandLine.appendSwitch (in
electron/main.ts) enables WaylandWindowDrag and WebRTCPipeWireCapturer; update
the feature list string passed to app.commandLine.appendSwitch accordingly.
🪄 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: 66c50af3-1976-48d7-9764-ef5b99972944
📒 Files selected for processing (4)
electron-builder.json5electron/ipc/handlers.tselectron/main.tspackage.json
- Add buildDialogOptions helper function to safely attach parent window only when valid and not destroyed - Update all dialog calls (save-exported-video, open-video-file-picker, save-project-file, load-project-file) to use the helper - Fix supportsWindowOpacity logic by removing || isWayland so Linux always follows no-opacity codepath - Change incorrect Chromium feature name 'PipeWire' to 'WebRTCPipeWireCapturer' in main.ts - Remove unused isWayland variable in handlers.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
electron/main.ts (1)
33-43: wayland detection + pipewire flag — looks good 👍pipewire feature name is now correct (
WebRTCPipeWireCapturer), matching the previous round of feedback. detection viaXDG_SESSION_TYPE/WAYLAND_DISPLAYis the standard heuristic.two tiny nits, both optional:
process.env.WAYLAND_DISPLAY !== undefinedtreats an empty string as "wayland present". vanishingly rare in practice but a truthy check is slightly safer:- electron also supports
--ozone-platform-hint=auto, which lets chromium pick wayland when available and gracefully fall back to x11 otherwise. since you're already gating on a wayland check, hard-settingwaylandis fine — just flagging in case you want the softer landing for weird sessions (e.g. XWayland-only apps or broken compositors).optional tweak
- const isWayland = - process.env.XDG_SESSION_TYPE === "wayland" || process.env.WAYLAND_DISPLAY !== undefined; + const isWayland = + process.env.XDG_SESSION_TYPE === "wayland" || Boolean(process.env.WAYLAND_DISPLAY);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@electron/main.ts` around lines 33 - 43, Change the WAYLAND detection to treat an empty string as absent by replacing the strict undefined check with a truthiness check (e.g., use process.env.WAYLAND_DISPLAY instead of process.env.WAYLAND_DISPLAY !== undefined) in the isWayland calculation so isWayland is false for empty strings; optionally, if you prefer a softer fallback instead of forcing Wayland, switch the call to app.commandLine.appendSwitch("ozone-platform", "wayland") to use the hint form app.commandLine.appendSwitch("ozone-platform-hint", "auto") (or use that variant when you want Chromium to prefer Wayland but fall back to X11) while keeping the WebRTCPipeWireCapturer feature appendSwitch unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@electron/main.ts`:
- Around line 33-43: Change the WAYLAND detection to treat an empty string as
absent by replacing the strict undefined check with a truthiness check (e.g.,
use process.env.WAYLAND_DISPLAY instead of process.env.WAYLAND_DISPLAY !==
undefined) in the isWayland calculation so isWayland is false for empty strings;
optionally, if you prefer a softer fallback instead of forcing Wayland, switch
the call to app.commandLine.appendSwitch("ozone-platform", "wayland") to use the
hint form app.commandLine.appendSwitch("ozone-platform-hint", "auto") (or use
that variant when you want Chromium to prefer Wayland but fall back to X11)
while keeping the WebRTCPipeWireCapturer feature appendSwitch unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f43e5ca0-dc0b-4710-a618-29485c3f4ea7
📒 Files selected for processing (2)
electron/ipc/handlers.tselectron/main.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- electron/ipc/handlers.ts
|
Hi, The new helper adds a Severity: action required | Category: correctness How to fix: Pass parent as first arg Agent prompt to fix - you can give this to your LLM of choice:
Spotted by Qodo code review - free for open-source projects. |
|
Hi, Severity: remediation recommended | Category: maintainability How to fix: Import dialog option types Agent prompt to fix - you can give this to your LLM of choice:
Found by Qodo code review. FYI, Qodo is free for open-source. |
|
@psychosomat I will not be able to validate this/ test this. Waiting on someone who can help validate this before it can be merged |
No problem - I totally get that you already have enough of PR. Either way, I'd be happy to help. |
3 participants
Changes ensure:
Summary by CodeRabbit
New Features
Improvements