Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a new zlocal() helper to Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Installer as "install.sh"
participant OS as "OS Detection"
participant Resolver as "Family Resolver"
participant Pkg as "Package Manager"
participant Clipboard as "Clipboard Detection"
participant System
User->>Installer: run install script
Installer->>OS: detect platform (/etc/os-release, uname)
OS-->>Resolver: provide ID / ID_LIKE
Resolver->>Resolver: _resolve_distro_family()
Resolver-->>Installer: DISTRO_FAMILY
alt DISTRO_FAMILY = debian
Installer->>Pkg: use apt-get / apt
else DISTRO_FAMILY = arch
Installer->>Pkg: use pacman
else DISTRO_FAMILY = fedora
Installer->>Pkg: use dnf
else DISTRO_FAMILY = suse
Installer->>Pkg: use zypper
else DISTRO_FAMILY = unknown
Installer->>System: emit error + list required packages
end
Installer->>Clipboard: detect Wayland / X11 / headless
alt Wayland
Clipboard-->>Installer: select wl-clipboard
else X11
Clipboard-->>Installer: select xclip
else Headless
Clipboard-->>Installer: select xclip & wl-clipboard
end
Installer->>Pkg: install COMMON_DEPS + clipboard tools
Pkg->>System: deploy packages
System-->>Installer: report install result
Estimated Code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
install.sh (2)
152-156: Minor: Identical branches can be unified.Both branches call
install_pkg "${COMMON_DEPS[@]}"identically, making the conditional unnecessary.♻️ Proposed simplification
-if [[ "$OS_TYPE" == "macos" ]]; then - install_pkg "${COMMON_DEPS[@]}" -else - install_pkg "${COMMON_DEPS[@]}" -fi +install_pkg "${COMMON_DEPS[@]}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@install.sh` around lines 152 - 156, The conditional checking OS_TYPE around calling install_pkg with COMMON_DEPS is redundant because both branches execute identical code; remove the if/else and call install_pkg "${COMMON_DEPS[@]}" directly (eliminate the OS_TYPE check and associated branching so only a single invocation of install_pkg with COMMON_DEPS remains).
169-191: Simplify redundant case statements.All distro families install identical packages (
wl-clipboard,xclip). The case statements can be collapsed sinceinstall_pkgalready handles distro-specific logic.♻️ Proposed simplification
if [[ -n "${WAYLAND_DISPLAY:-}" ]]; then echo -e "${BLUE}Wayland detected — installing wl-clipboard${NC}" - case "$DISTRO_FAMILY" in - debian) install_pkg wl-clipboard ;; - arch) install_pkg wl-clipboard ;; - fedora) install_pkg wl-clipboard ;; - suse) install_pkg wl-clipboard ;; - esac + install_pkg wl-clipboard elif [[ -n "${DISPLAY:-}" ]]; then echo -e "${BLUE}X11 detected — installing xclip${NC}" - case "$DISTRO_FAMILY" in - debian) install_pkg xclip ;; - arch) install_pkg xclip ;; - fedora) install_pkg xclip ;; - suse) install_pkg xclip ;; - esac + install_pkg xclip else # Headless / TTY-only: install both so the env is ready for either echo -e "${YELLOW}No display server detected (headless/TTY) — installing xclip + wl-clipboard as fallback${NC}" - case "$DISTRO_FAMILY" in - debian) install_pkg xclip wl-clipboard ;; - arch) install_pkg xclip wl-clipboard ;; - fedora) install_pkg xclip wl-clipboard ;; - suse) install_pkg xclip wl-clipboard ;; - esac + install_pkg xclip wl-clipboard fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@install.sh` around lines 169 - 191, Collapse the repeated case branches by calling install_pkg directly for the required packages instead of branching on DISTRO_FAMILY each time; replace the three identical case blocks (the ones that install wl-clipboard when WAYLAND_DETECTED, xclip when DISPLAY is set, and both when headless) with direct calls like install_pkg wl-clipboard, install_pkg xclip, and install_pkg xclip wl-clipboard respectively, keeping the DISPLAY/Wayland detection logic and referencing the existing DISTRO_FAMILY variable only if install_pkg needs it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@install.sh`:
- Around line 68-95: The distro family resolver fails to map macOS; update
_resolve_distro_family so that when id is "macos" (and optionally when id_like
contains "macos" or "darwin") it echoes "macos" instead of falling through to
"unknown". Modify the top-level case in _resolve_distro_family to include macos
in the id matches and add a corresponding branch in the id_like fallback,
ensuring DISTRO_FAMILY="$( _resolve_distro_family "$DISTRO" "$DISTRO_LIKE" )"
will be "macos" so install_pkg uses the Homebrew path correctly.
- Around line 167-175: The script currently references WAYLAND_DISPLAY and
DISPLAY directly while run under set -u, causing failures when those env vars
are unset; update the conditionals to use safe parameter expansion (e.g.,
${WAYLAND_DISPLAY:-} and ${DISPLAY:-}) wherever they are read (the if [[ -n
"$WAYLAND_DISPLAY" ]] and elif [[ -n "$DISPLAY" ]] checks) so they default to
empty strings, preserving behavior of the install_pkg calls (wl-clipboard) and
still respecting DISTRO_FAMILY and install_pkg logic.
---
Nitpick comments:
In `@install.sh`:
- Around line 152-156: The conditional checking OS_TYPE around calling
install_pkg with COMMON_DEPS is redundant because both branches execute
identical code; remove the if/else and call install_pkg "${COMMON_DEPS[@]}"
directly (eliminate the OS_TYPE check and associated branching so only a single
invocation of install_pkg with COMMON_DEPS remains).
- Around line 169-191: Collapse the repeated case branches by calling
install_pkg directly for the required packages instead of branching on
DISTRO_FAMILY each time; replace the three identical case blocks (the ones that
install wl-clipboard when WAYLAND_DETECTED, xclip when DISPLAY is set, and both
when headless) with direct calls like install_pkg wl-clipboard, install_pkg
xclip, and install_pkg xclip wl-clipboard respectively, keeping the
DISPLAY/Wayland detection logic and referencing the existing DISTRO_FAMILY
variable only if install_pkg needs it.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
install.sh (1)
68-95:⚠️ Potential issue | 🔴 Critical
DISTRO_FAMILYstill misses macOS mapping, which breaks package installs on macOS.At Line 73 and Line 84,
_resolve_distro_familynever returnsmacos, so macOS resolves tounknownand hits the unsupported branch ininstall_pkg.🐛 Proposed fix
_resolve_distro_family() { local id="$1" local id_like="$2" case "$id" in + macos|darwin) + echo "macos" ;; ubuntu|debian|pop|kali|linuxmint|raspbian|elementary|zorin) echo "debian" ;; arch|manjaro|endeavouros|garuda|artix) echo "arch" ;; fedora|rhel|centos|rocky|almalinux|nobara) echo "fedora" ;; opensuse*|suse*) echo "suse" ;; *) case "$id_like" in + *macos*|*darwin*) echo "macos" ;; *debian*|*ubuntu*) echo "debian" ;; *arch*) echo "arch" ;; *fedora*|*rhel*) echo "fedora" ;; *suse*) echo "suse" ;; *) echo "unknown" ;; esac ;; esac }Also applies to: 101-123
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@install.sh` around lines 68 - 95, The distro detection routine _resolve_distro_family fails to map macOS, causing DISTRO_FAMILY to be "unknown" and breaking install_pkg; update _resolve_distro_family to return "macos" for macOS identifiers (e.g., match "darwin", "macos", "macosx", "macos*" as appropriate) in the outer case and also include a fallback in the ID_LIKE inner case (match "*macos*" or "*darwin*"); ensure references to DISTRO_FAMILY and any logic in install_pkg that checks for "macos" will now work by adding this mapping to the function.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@install.sh`:
- Around line 101-115: The package installation commands inside install_pkg (the
case on DISTRO_FAMILY using pkgs) currently run without checking failure; change
each package-manager invocation (apt-get, pacman, dnf, zypper) so that failures
are propagated (e.g., chain operations with && or test the command exit status)
and ensure the case block itself exits non‑zero on failure: after the install
command for each distro, add a failure check that logs an error and exits (or
simply use && to make the whole operation fail), and add a final fallback/error
handler in the case statement so install_pkg returns non‑zero and prints a clear
message when a package manager command fails.
---
Duplicate comments:
In `@install.sh`:
- Around line 68-95: The distro detection routine _resolve_distro_family fails
to map macOS, causing DISTRO_FAMILY to be "unknown" and breaking install_pkg;
update _resolve_distro_family to return "macos" for macOS identifiers (e.g.,
match "darwin", "macos", "macosx", "macos*" as appropriate) in the outer case
and also include a fallback in the ID_LIKE inner case (match "*macos*" or
"*darwin*"); ensure references to DISTRO_FAMILY and any logic in install_pkg
that checks for "macos" will now work by adding this mapping to the function.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
install.sh (1)
106-130:⚠️ Potential issue | 🟠 MajorPropagate package-manager failures in
install_pkg().Because Line 16 uses
set -uo pipefail(withoutset -e), failures in Lines 109–123 can be ignored and the script may continue in a partially installed state.Proposed fix
install_pkg() { local pkgs=("$@") echo -e "${YELLOW}Installing: ${pkgs[*]}...${NC}" case "$DISTRO_FAMILY" in debian) export DEBIAN_FRONTEND=noninteractive - sudo apt-get update -qq -y > /dev/null - sudo apt-get install -qq -y "${pkgs[@]}" > /dev/null + sudo apt-get update -qq -y > /dev/null && + sudo apt-get install -qq -y "${pkgs[@]}" > /dev/null ;; arch) - sudo pacman -Sy --quiet --noconfirm --needed "${pkgs[@]}" > /dev/null + sudo pacman -Sy --quiet --noconfirm --needed "${pkgs[@]}" > /dev/null ;; fedora) - sudo dnf install -q -y "${pkgs[@]}" > /dev/null + sudo dnf install -q -y "${pkgs[@]}" > /dev/null ;; suse) - sudo zypper install -q -y "${pkgs[@]}" > /dev/null + sudo zypper install -q -y "${pkgs[@]}" > /dev/null ;; macos) - brew install -q "${pkgs[@]}" > /dev/null + brew install -q "${pkgs[@]}" > /dev/null ;; *) echo -e "${RED}Unsupported distribution: ${DISTRO} (family: ${DISTRO_FAMILY})${NC}" echo -e "${RED}Please install the following packages manually: ${pkgs[*]}${NC}" exit 1 ;; - esac + esac || { + echo -e "${RED}Package installation failed: ${pkgs[*]}${NC}" + exit 1 + } }#!/bin/bash # Read-only verification: confirm install_pkg lacks explicit failure propagation. set -u FILE="$(fd '^install\.sh$' | head -n1)" nl -ba "$FILE" | sed -n '100,132p' echo "---- sanity checks ----" rg -n 'set -uo pipefail|set -e|esac \|\||apt-get update .*&&|apt-get install .*&&|pacman .*&&|dnf .*&&|zypper .*&&|brew install .*&&' "$FILE"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@install.sh` around lines 106 - 130, install_pkg() currently runs distro package commands under set -uo pipefail without failing the script on errors; update it so package-manager failures propagate by either enabling errexit for the function (e.g., start install_pkg() with set -euo pipefail or local +e and restore) or add explicit failure checks to each branch (append || { echo "Failed to install ${pkgs[*]} via <manager>"; exit 1; } to the apt-get update/install, pacman, dnf, zypper and brew commands). Reference install_pkg, DISTRO_FAMILY, and the package command invocations (apt-get update/install, pacman -Sy, dnf install, zypper install, brew install) when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@install.sh`:
- Around line 106-130: install_pkg() currently runs distro package commands
under set -uo pipefail without failing the script on errors; update it so
package-manager failures propagate by either enabling errexit for the function
(e.g., start install_pkg() with set -euo pipefail or local +e and restore) or
add explicit failure checks to each branch (append || { echo "Failed to install
${pkgs[*]} via <manager>"; exit 1; } to the apt-get update/install, pacman, dnf,
zypper and brew commands). Reference install_pkg, DISTRO_FAMILY, and the package
command invocations (apt-get update/install, pacman -Sy, dnf install, zypper
install, brew install) when making the change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR enhances the shell setup by introducing a convenient alias for managing
.zshrc.local, along with improved OS detection and automatic clipboard utilities installation.✨ Changes
1. Added alias for
.zshrc.local.zshrc.local.2. Clipboard utilities installation
pbcopy(macOS)xclip/xsel(Linux)3. Improved OS detection
🧪 Testing
Summary by CodeRabbit
New Features
Chores