Conversation
implemented shellcheck formatting fix terminal spam added better cleanup logic
|
Caution Review failedThe pull request is closed. ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis pull request enhances the installation and uninstallization scripts with robust cross-distro support, improved error handling, and stricter shell linting. Changes include distro-aware package management, OS-specific cleanup logic, expanded Zsh syntax checks, and a new ShellCheck configuration file for consistency across environments. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
✨ 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 |
Summary
This PR addresses 16 issues found during a code review of the installer,
.zshrc, CI workflow, and test scripts. Changes fall into four areas: installer reliability,.zshrccorrectness, CI coverage, and linting consistency.install.shset -e→set -uo pipefail—set -einteracts poorly with subshells and conditional logic. Replaced withpipefailand explicit|| { ...; exit 1; }checks on critical commands.ezaskip pattern. Re-running the installer is now safe.v3.4.0in the FiraCode download URL replaced with a GitHub API lookup. Falls back tov3.4.0if the API is unreachable.EZA_TMPleak fixed — The Fedora 42+ binary temp dir is now registered in aTEMP_DIRS[]array tracked by the globalcleanuptrap, so it's always removed on exit or interrupt.chshfailure surfaced — Previously swallowed with> /dev/null 2>&1. Now prints an actionable warning with the manual command if it fails..zshrcnocompilecomment — Added a comment explaining whynocompile"init.zsh"is used alongsidesrc"init.zsh"on the zoxide ice, which was previously confusing.unalias—unalias zi zpl zplg 2>/dev/nullreplaced with(( ${+aliases[name]} )) && unalias namefor each alias — no stderr suppression needed.zshift-updatemodification warning — The updater now diffs the current.zshrcagainst the backup before overwriting. If local edits are detected, it warns the user and prompts for confirmation, pointing them to~/.zshrc.localfor persistent customisation.help()comment — Added a comment noting the intentional override of the Zsh built-inhelp.ci.ymltheme.shanduninstall.shlinted — Both were previously untested in CI; a syntax error in either would ship silently.zsh -ic "exit" || truenow pipes output to the log before allowing non-zero exit, so a broken.zshrcis visible rather than silently passing.eza,bat,fzf,zoxide, andstarshipare actually callable viazsh -ic "command -v ...". Fails if 3+ tools are missing. Same check added to Docker jobs..shellcheckrc— See below..shellcheckrc(new file)SC2086,SC2034, andSC1091ignore rules. CI andtests/local_test.shpreviously had divergent-eflag lists. Both now pick up.shellcheckrcautomatically.uninstall.shdnf remove eza(which fails on 42+ since eza was installed manually), logged a confusing error, then succeeded via the catch-allrm. Now branches onFEDORA_VERSIONand removes the binary directly without the misleading error.bat,fd,fzf,rg,zoxide,eza).tests/local_test.sh[ -f "../$file" ] || [ -f "$file" ]detection withSCRIPT_DIR/REPO_ROOTderived from$(dirname "$0"). The script now works correctly regardless of the working directory it's called from.install.sh,theme.sh, anduninstall.sh(was onlyinstall.sh). No longer passes-eflags since.shellcheckrchandles that.Testing
.shellcheckrcin placeChecklist
shellcheckwith shared.shellcheckrczsh -n .zshrcpassesSummary by CodeRabbit
New Features
.zshrchas changed before applying updateszsualias for quick update accessBug Fixes
Chores