mm #12
mm #12
Conversation
|
Unable to perform a code review. You have run out of credits 😔 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
The pull request #12 has too many files changed.
The GitHub API will only let us fetch up to 300 changed files, and this pull request has 507.
|
This pull request introduces several configuration and usability improvements, primarily focused on issue reporting, application behavior, and system integration. The most notable changes include the addition of GitHub issue templates for bug reports, enhancements, and documentation, updates to application launch and visibility settings, and adjustments to user experience in terminal and browser configurations. GitHub Issue Templates and Repository Configuration:
Application Launch and Visibility:
Terminal and Editor Experience:
Browser and System Integration:
General Configuration and Boot Script:
|
There was a problem hiding this comment.
Pull Request Overview
This pull request represents a major refactoring of the Omarchy installation system, introducing significant improvements to code organization, error handling, and user experience. The changes transform the installer from a sequential script-based approach to a modular, logged system with comprehensive error handling.
Key Changes
- Complete restructuring of the installation system with new helper modules for logging, error handling, and presentation
- Migration standardization using utility commands (omarchy-pkg-add, omarchy-cmd-missing) instead of direct pacman calls
- Introduction of Walker/Elephant application launcher configuration replacing the legacy Walker setup
Reviewed Changes
Copilot reviewed 195 out of 507 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| install.sh | Replaced monolithic installation with modular system using helper functions and organized module sourcing |
| install/helpers/*.sh | Added new helper modules for logging, error handling, presentation, and chroot operations |
| install/preflight/*.sh | Refactored preflight checks into modular components with enhanced guard conditions |
| install/packaging/*.sh | Introduced base package installation and restructured packaging modules |
| install/config/*.sh | Reorganized configuration scripts with new hardware detection and setup modules |
| install/login/*.sh | Simplified Plymouth and login configuration, removed seamless login implementation |
| migrations/*.sh | Standardized migration scripts to use utility commands instead of direct system calls |
| config/walker/*.toml | Migrated to new Elephant-based Walker configuration system |
| config/hypr/*.conf | Updated Hyprland configuration with new bindings, group support, and improved window management |
| default/walker/themes/* | Replaced legacy Walker theme files with new XML-based layout system |
Comments suppressed due to low confidence (1)
config/hypr/looknfeel.conf:1
- Blur passes increased from 1 to 3 will significantly impact performance, especially on lower-end hardware. Consider documenting this change and providing a way for users to adjust this setting based on their hardware capabilities.
# Change the default Omarchy look'n'feel
| touch ~/.local/state/omarchy/migrations/1751134560.sh && | ||
| uwsm stop | ||
| sudo systemctl restart systemd-timesyncd | ||
| bash omarchy-update-perform |
There was a problem hiding this comment.
The command is missing proper quoting and should use the absolute path or ensure PATH is set. Should be 'bash "$OMARCHY_PATH/bin/omarchy-update-perform"' or verify PATH includes the bin directory.
| bash omarchy-update-perform | |
| bash "$OMARCHY_PATH/bin/omarchy-update-perform" |
| if [[ ! -f /etc/arch-release ]]; then | ||
| abort "Vanilla Arch" | ||
| fi |
There was a problem hiding this comment.
[nitpick] The conditional structure has been changed from a one-liner to multi-line format. While this is more explicit, all guards follow this pattern now. Consider documenting why this change improves the code (e.g., better readability, easier debugging) to justify the increased verbosity.
| # Disable [email protected] only if not already disabled | ||
| if ! systemctl is-enabled [email protected] | grep -q disabled; then | ||
| sudo systemctl disable [email protected] | ||
| sudo plymouth-set-default-theme omarchy |
There was a problem hiding this comment.
The removed '-R' flag from plymouth-set-default-theme means the initramfs will not be regenerated. This could leave the system with mismatched theme configuration. Either add back the '-R' flag or ensure mkinitcpio regeneration happens elsewhere.
| sudo plymouth-set-default-theme omarchy | |
| sudo plymouth-set-default-theme -R omarchy |
| @@ -1,4 +1,3 @@ | |||
| # Editor used by CLI | |||
There was a problem hiding this comment.
The EDITOR variable is no longer set in this file but SUDO_EDITOR still references it. This will result in SUDO_EDITOR being empty unless EDITOR is set elsewhere. Should either set EDITOR here or source the file that sets it before this line.
| # Editor used by CLI | |
| # Editor used by CLI | |
| export EDITOR="${EDITOR:-vim}" |
config/walker/config.toml
Outdated
| close_when_open = true # close walker when invoking while already opened | ||
| selection_wrap = true # wrap list if at bottom or top | ||
| click_to_close = true # closes walker if clicking outside of the main content area | ||
| global_argument_delimiter = "#" # query: firefox#https://benz.dev => part after delimiter will be ignored when querying. this should be the same as in the elephant config |
There was a problem hiding this comment.
The comment states 'this should be the same as in the elephant config' but there's no reference to where that elephant config is or how to keep them in sync. Add a comment with the file path or a reference to ensure maintainability.
| global_argument_delimiter = "#" # query: firefox#https://benz.dev => part after delimiter will be ignored when querying. this should be the same as in the elephant config | |
| global_argument_delimiter = "#" # query: firefox#https://benz.dev => part after delimiter will be ignored when querying. this should be the same as in the elephant config (see config/elephant/config.toml) |
| EOF | ||
|
|
||
| [[ -f /boot/EFI/limine/limine.conf ]] && EFI=true | ||
| [[ -f /boot/EFI/limine/limine.conf ]] || [[ -f /boot/EFI/BOOT/limine.conf ]] && EFI=true |
There was a problem hiding this comment.
Operator precedence issue: the && binds tighter than ||, so this evaluates as [[ -f /boot/EFI/limine/limine.conf ]] || ([[ -f /boot/EFI/BOOT/limine.conf ]] && EFI=true). Should be ([[ -f /boot/EFI/limine/limine.conf ]] || [[ -f /boot/EFI/BOOT/limine.conf ]]) && EFI=true
| [[ -f /boot/EFI/limine/limine.conf ]] || [[ -f /boot/EFI/BOOT/limine.conf ]] && EFI=true | |
| ([[ -f /boot/EFI/limine/limine.conf ]] || [[ -f /boot/EFI/BOOT/limine.conf ]]) && EFI=true |
| --ozone-platform-hint=wayland | ||
| --enable-features=TouchpadOverscrollHistoryNavigation | ||
| --load-extension=~/.local/share/omarchy/default/chromium/extensions/copy-url | ||
| # Chromium crash workaround for Wayland color management on Hyprland - see https://github.com/hyprwm/Hyprland/issues/11957 |
There was a problem hiding this comment.
While the comment references issue #11957, it would be helpful to also note that this is a temporary workaround and should be removed when the upstream issue is resolved. Add a TODO or NOTE about checking if this workaround is still needed in future updates.
| # Chromium crash workaround for Wayland color management on Hyprland - see https://github.com/hyprwm/Hyprland/issues/11957 | |
| # Chromium crash workaround for Wayland color management on Hyprland - see https://github.com/hyprwm/Hyprland/issues/11957 | |
| # TODO: Remove this workaround when https://github.com/hyprwm/Hyprland/issues/11957 is resolved. |
|
This pull request introduces several configuration and usability improvements, mainly focused on editor behavior, application visibility, issue reporting, and platform compatibility. The changes include new configuration files, updates to existing application launchers, enhancements to issue templates, and workarounds for platform-specific issues. Editor and Application Configuration:
Platform Compatibility and Workarounds:
Issue Reporting and Documentation:
Usability Improvements:
Application Behavior and Preferences: |
| command -v limine &>/dev/null || abort "Limine bootloader" | ||
|
|
||
| # Must have btrfs root filesystem | ||
| [ "$(findmnt -n -o FSTYPE /)" = "btrfs" ] || abort "Btrfs root filesystem" |
There was a problem hiding this comment.
Extra trailing whitespace at end of line 43
| [ "$(findmnt -n -o FSTYPE /)" = "btrfs" ] || abort "Btrfs root filesystem" | |
| [ "$(findmnt -n -o FSTYPE /)" = "btrfs" ] || abort "Btrfs root filesystem" |
| @@ -1,18 +1,22 @@ | |||
| # This is now a deprecated file meant for those who did not wish to learn the latest Omarchy hotkeys. | |||
There was a problem hiding this comment.
Grammar issue: 'did not wish' should be 'do not wish' (present tense) since the file continues to serve this purpose
| # This is now a deprecated file meant for those who did not wish to learn the latest Omarchy hotkeys. | |
| # This is now a deprecated file meant for those who do not wish to learn the latest Omarchy hotkeys. |
Ghostty will only use the last `shell-integration-features` line found in the config, so multiple options need to be presented as a comma-separated list. Including them on separate lines meant that only the `ssh-env` option was getting set, causing the `no-cursor` option to be ignored. This prevented the cursor from going into the `block` style.
…stics") by adjusting JetBrains window rules (#2899) * fix(jetbrains.conf): allow focus for Android Studio’s first dialog by unsetting nofocus rules for empty-title windows * Refactor window rules for JetBrains apps Updated window rules for JetBrains applications to allow focus and interaction with dialogs. --------- Co-authored-by: David Heinemeier Hansson <[email protected]>
Omarchy 3.1.4
* Fix: Low Battery notification returning empty value on reboot * Simplify the check * Double paste --------- Co-authored-by: David Heinemeier Hansson <[email protected]>
As @abenz1267 pointed out, the previous solution wasn't the greatest. This is the proper solution.
* Apply Omarchy theme; Install Neovim extension (automatically scoops up LazyVim config). * Just set the Omarchy theme Removed installation of Neovim extension from script. --------- Co-authored-by: David Heinemeier Hansson <[email protected]>
Co-authored-by: David Heinemeier Hansson <[email protected]>
* Add copy-url support for Brave browser * Add required migration * Excess CR --------- Co-authored-by: David Heinemeier Hansson <[email protected]>
…ervice after running omarchy-refresh-walker Closes #3609
…and allow a second to save state)
Omarchy 3.2.1
* Update version * feat: show link to changelog in update confirm window * Link to scrollable changelog You may be updating more than one version at the time. --------- Co-authored-by: Ryan Hughes <[email protected]> Co-authored-by: David Heinemeier Hansson <[email protected]>
…into other menus Closes #3604
* feat: restore the last notification. * Stick with comma --------- Co-authored-by: David Heinemeier Hansson <[email protected]>
* feat: silently move active window to target workspace Adds a quality-of-life keybind: SUPER + ALT + SHIFT + [1-9, 0] This silently moves the active window from the active workspace to the chosen destination workspace. The destination workspace re-tiles correctly upon receiving the window. The currently active workspace remains active, and the moved window retains focus throughout the operation (no workspace switch, no animation, no extra UI noise). Why: - Provides fast, silent window reshuffling during multitasking. - Avoids disruptive workspace switching. - Matches Omarchy’s focus on efficient Hyprland ergonomics. - Keeps the user anchored to their current workflow while reorganizing. Testing: 1. Open multiple windows across several workspaces. 2. Focus a window on the active workspace. 3. Press SUPER + ALT + [1–9, 0]. 4. Verify the window appears on the target workspace and re-tiles. 5. Confirm the **source workspace stays active**. 6. Confirm **focus remains on the moved window**. 7. Repeat with multiple applications and workspace combinations. * We've frozen this old tiling config * Put on Super + Shift + Alt This is an alternative version of an existing move * Fix description --------- Co-authored-by: David Heinemeier Hansson <[email protected]>
Omarchy 3.2.2
59 participants
This pull request introduces several improvements to project configuration, user experience, and application behavior. Key changes include the addition of standardized issue templates, enhanced application launch behavior, and various configuration tweaks to improve usability and compatibility.
Project configuration and issue templates:
.editorconfigto enforce consistent coding styles across the project.Application launch and usability improvements:
nvim.desktoplauncher to useomarchy-launch-editorfor opening files, improving editor integration..desktopfiles to hide several system applications from application menus for a cleaner user experience.Configuration and compatibility tweaks:
boot.shand improved branch selection messaging for clarity during installations. [1] [2]btopand adjustedalacrittykeybindings for improved terminal usability. [1] [2]Minor configuration updates:
elephantandfastfetchconfiguration files for better performance and appearance. [1] [2] [3]