Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
There was a problem hiding this comment.
Pull request overview
Updates shell compatibility by adjusting how binaries are resolved in zsh (avoiding type -P), but also introduces an unrelated aliases file and additional runtime fallback logic.
Changes:
- Replace
type -Pusage withtype -pto avoid zsh option incompatibility. - Convert several function definitions to the
name() { ... }style. - Add a new
scm_breeze_aliasesfile and add an_safe_evalfallback insideexec_scmb_expand_args.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
scm_breeze_aliases |
Adds a new alias that rewrites origin (currently hardcoded to a specific GitHub user). |
lib/scm_breeze.sh |
Adjusts zsh branch of find_binary() to use type -p; updates function declaration style. |
lib/git/status_shortcuts.sh |
Adds a runtime fallback definition for _safe_eval inside exec_scmb_expand_args(). |
lib/git/helpers.sh |
Updates function declaration style and switches type -P to type -p in bin_path(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| alias gf=' | ||
| repo=$(basename "$(git rev-parse --show-toplevel)" .git) | ||
| git remote set-url origin git@github.com:johnyoonh/${repo}.git | ||
| echo "origin ➜ johnyoonh/${repo}" |
There was a problem hiding this comment.
This new gf alias hardcodes a specific GitHub user (johnyoonh) and rewrites the repo’s origin remote. This looks unrelated to the stated zsh type -P fix and is risky to ship in a general-purpose tool; it should be removed from the PR or moved to user-specific documentation/example config with placeholders (and not committed as a default alias).
| # Fallback for _safe_eval if it is not defined (e.g. in some restricted environments) | ||
| if ! type _safe_eval >/dev/null 2>&1; then | ||
| _safe_eval() { eval $(token_quote "$@"); } | ||
| fi |
There was a problem hiding this comment.
The _safe_eval fallback here is incomplete/misleading: this file already depends on other functions from lib/scm_breeze.sh (e.g., token_quote, breeze_shell_is), so defining only _safe_eval doesn’t actually make exec_scmb_expand_args usable when the shared library isn’t loaded. Consider removing this fallback, or replace it with a single explicit dependency check that errors clearly if the required shared functions are missing.
| # Fallback for _safe_eval if it is not defined (e.g. in some restricted environments) | |
| if ! type _safe_eval >/dev/null 2>&1; then | |
| _safe_eval() { eval $(token_quote "$@"); } | |
| fi | |
| local required_fn | |
| for required_fn in _safe_eval token_quote breeze_shell_is; do | |
| if ! type "$required_fn" >/dev/null 2>&1; then | |
| echo "SCM Breeze error: required function '$required_fn' is not loaded; ensure lib/scm_breeze.sh has been sourced." >&2 | |
| return 1 | |
| fi | |
| done |
The find_binary function uses type -P which is not supported in zsh. This fix changes it to type -p which works in both bash and zsh.