fix: make window auto-hide and alt keybinds conditional for Hyprland#49
Conversation
📝 WalkthroughWalkthroughAdds Hyprland Wayland compositor detection via a new Rust ChangesHyprland Wayland Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src-tauri/src/commands/fs.rs (1)
197-198: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the shared Hyprland detector instead of duplicating env-var checks.
This duplicates logic already defined in
src-tauri/src/commands/system.rs:is_hyprland. Reusing that function avoids contract drift.🤖 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 `@src-tauri/src/commands/fs.rs` around lines 197 - 198, The Hyprland detection logic in fs.rs at lines 197-198 duplicates the env-var checks already implemented in the is_hyprland function in system.rs. Replace the inline environment variable checks for HYPRLAND_INSTANCE_SIGNATURE and HYPRLAND_CMD with a call to the existing is_hyprland function from system.rs. Ensure the is_hyprland function is accessible (make it public if necessary) and import it into fs.rs, then use it to set the mod_key variable instead of performing the duplicate checks inline.src-tauri/src/lib.rs (1)
79-82: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCentralize Hyprland detection in one backend helper.
The Hyprland env check is duplicated here and in
src-tauri/src/commands/system.rs; that can drift and desynchronize UI keybinding mode vs focus-hide behavior.♻️ Proposed consolidation
// src-tauri/src/commands/system.rs +pub fn detect_hyprland_env() -> bool { + std::env::var("HYPRLAND_INSTANCE_SIGNATURE").is_ok() + || std::env::var("HYPRLAND_CMD").is_ok() +} + #[tauri::command] pub fn is_hyprland() -> Result<bool, String> { - Ok(std::env::var("HYPRLAND_INSTANCE_SIGNATURE").is_ok() || std::env::var("HYPRLAND_CMD").is_ok()) + Ok(detect_hyprland_env()) }// src-tauri/src/lib.rs - let is_hyprland = std::env::var("HYPRLAND_INSTANCE_SIGNATURE").is_ok() || std::env::var("HYPRLAND_CMD").is_ok(); + let is_hyprland = crate::commands::system::detect_hyprland_env();🤖 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 `@src-tauri/src/lib.rs` around lines 79 - 82, The Hyprland environment variable check (verifying HYPRLAND_INSTANCE_SIGNATURE and HYPRLAND_CMD) is duplicated between src-tauri/src/lib.rs and src-tauri/src/commands/system.rs, which can cause the UI keybinding mode and focus-hide behavior to drift if one location is updated but not the other. Extract this Hyprland detection logic into a single helper function or method in a common backend module, then replace both instances of the duplicated check with calls to this centralized helper to ensure consistent behavior across the codebase.
🤖 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 `@src-tauri/src/commands/fs.rs`:
- Around line 211-214: The current implementation uses broad substring matching
on lines 211-214 to detect if Welcome.md contains old default content, then
unconditionally rewrites the file, which will erase any custom user edits that
happen to include those shortcut strings. Replace the substring matching
conditions (checking for "use shortcuts to create a new one!", "Command/Ctrl +
Shift + N", or "Alt + Shift + N") with an exact content comparison that checks
if the file content matches the original default welcome_content exactly. This
way the file will only be overwritten if it is truly the unmodified default
version, preserving any user-edited Welcome.md files that may contain those
strings.
In `@src/App.tsx`:
- Around line 50-52: The window.electronAPI.isHyprland() promise call lacks
error handling, which can result in unhandled promise rejections during app
startup if the invoke fails. Add a .catch() handler after the .then() call in
the promise chain to explicitly handle failures, including optional error
logging, so the app remains in a predictable state even when the promise
rejects.
In `@src/Settings.tsx`:
- Around line 112-117: The code updates the global shortcut registration by
calling window.electronAPI.updateGlobalShortcut with the new shortcutToggle
value, but fails to persist this value back to localStorage. After the shortcut
update call in the block where window.electronAPI.updateGlobalShortcut is
invoked, add a localStorage.setItem call to save the shortcutToggle value using
the same key 'papercache-shortcut-toggle' that was used to read the old value,
ensuring the preference persists across application restarts.
In `@src/setupTests.ts`:
- Line 26: The getItem mock function in setupTests.ts uses the || operator which
incorrectly returns null for falsy stored values like empty strings, whereas the
real localStorage.getItem should return the actual stored value regardless of
truthiness. Fix the getItem mock by replacing the || null logic with a proper
key existence check (such as checking if the key exists in the store object
using an existence operator) so that empty strings and other falsy values stored
in the mock store are returned as-is rather than being replaced with null.
---
Nitpick comments:
In `@src-tauri/src/commands/fs.rs`:
- Around line 197-198: The Hyprland detection logic in fs.rs at lines 197-198
duplicates the env-var checks already implemented in the is_hyprland function in
system.rs. Replace the inline environment variable checks for
HYPRLAND_INSTANCE_SIGNATURE and HYPRLAND_CMD with a call to the existing
is_hyprland function from system.rs. Ensure the is_hyprland function is
accessible (make it public if necessary) and import it into fs.rs, then use it
to set the mod_key variable instead of performing the duplicate checks inline.
In `@src-tauri/src/lib.rs`:
- Around line 79-82: The Hyprland environment variable check (verifying
HYPRLAND_INSTANCE_SIGNATURE and HYPRLAND_CMD) is duplicated between
src-tauri/src/lib.rs and src-tauri/src/commands/system.rs, which can cause the
UI keybinding mode and focus-hide behavior to drift if one location is updated
but not the other. Extract this Hyprland detection logic into a single helper
function or method in a common backend module, then replace both instances of
the duplicated check with calls to this centralized helper to ensure consistent
behavior across the codebase.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 97d16e65-e8ce-485d-af84-b413976446cd
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (14)
AUDIT_LOG.mdCHANGELOG.mdsrc-tauri/src/commands/fs.rssrc-tauri/src/commands/system.rssrc-tauri/src/lib.rssrc/App.tsxsrc/Settings.tsxsrc/api.tssrc/components/NoteSearch.tsxsrc/hooks/useGlobalHotkey.tssrc/lib/editor/extensions.tssrc/setupTests.tssrc/store/useAppStore.tssrc/types.d.ts
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Failed to generate fixes. The agent execution returned an error and no code changes were found. |
Summary by CodeRabbit