Open
Conversation
- split bootstrap and heavy init for zsh entrypoints - add upstream lazy widget registration, fallback, and key binding APIs - document lazy-load usage and cover eager/lazy flows with shell tests
📝 WalkthroughWalkthroughAdds a Zsh lazy-loading bootstrap and runtime: new bootstrap entrypoint, autoloadable init APIs (zeno-init, zeno-ensure-loaded, zeno-preload), lazy widget registration/dispatch/fallback, default keybinding helper, hook cleanup, XDG config discovery helpers, and extensive tests for init and lazy API behavior. Changes
Sequence DiagramssequenceDiagram
actor Shell
participant zeno_zsh as "zeno.zsh"
participant bootstrap as "zeno-bootstrap.zsh"
participant zeno_init as "zeno-init"
participant hooks as "zeno-enable-sock"
Shell->>zeno_zsh: source zeno.zsh
zeno_zsh->>bootstrap: source bootstrap
bootstrap->>bootstrap: set PATH, fpath, autoloads, widget dirs
bootstrap->>Shell: export ZENO_BOOTSTRAPPED=1
alt heavy init requested
Shell->>zeno_init: zeno-init / zeno-ensure-loaded
zeno_init->>zeno_init: check ZENO_LOADED, run deno cache, detect deno version
zeno_init->>hooks: zeno-enable-sock (register hooks)
hooks->>hooks: add precmd/chpwd/zshexit hooks
zeno_init->>Shell: export ZENO_LOADED=1
end
sequenceDiagram
actor User
participant Key as "zle / Key"
participant dispatcher as "zeno-lazy-widget-dispatch"
participant ensure as "zeno-ensure-loaded"
participant init as "zeno-init"
participant widget as "Real Widget"
participant fallback as "zeno-run-lazy-fallback"
User->>Key: press key
Key->>dispatcher: invoke widget (WIDGET)
dispatcher->>ensure: call zeno-ensure-loaded
alt not loaded
ensure->>init: invoke zeno-init
init->>ensure: return (ZENO_LOADED=1)
else loaded
ensure-->>dispatcher: return
end
alt widget defined
dispatcher->>widget: call widget fn
widget->>User: perform action
else missing
dispatcher->>fallback: call zeno-run-lazy-fallback
fallback->>User: perform fallback action
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Zsh integration by introducing a lazy-loading mechanism. This change aims to improve shell startup performance by deferring the loading of heavier components until they are actually needed, while ensuring that existing eager loading configurations continue to function without modification. It provides new public APIs for users to explicitly opt into lazy-loading for various Zsh features, making the shell more responsive. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
- apply deno fmt output for new zsh lazy-load tests - align local branch with CI fmt-check expectations
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a significant feature: lazy-loading for zsh initialization. This is a great improvement for startup performance. The implementation is well-structured, splitting the initialization into a lightweight bootstrap and a heavy, on-demand init. The new APIs for lazy widget registration and key binding are clear and well-documented in the README.
I've found a few issues:
- A couple of correctness bugs in the lazy-loading fallback logic and Deno version parsing. These could cause errors in specific scenarios.
- A minor maintainability issue with some redundant code in a fallback implementation.
Overall, this is a solid contribution. Addressing the feedback will make the new lazy-loading feature more robust.
There was a problem hiding this comment.
Pull request overview
This PR adds a Zsh “bootstrap vs heavy init” split to support lazy-loading while keeping source zeno.zsh eager and backward-compatible, and introduces public lazy-load APIs with shell-level test coverage.
Changes:
- Add
zeno-bootstrap.zshand new public Zsh APIs (zeno-init,zeno-ensure-loaded,zeno-preload, lazy widget registration, default key binding helper). - Update
zeno.zshto source the bootstrap and run eager initialization viazeno-init. - Add Deno-driven shell tests for eager init, lazy widget dispatch, fallbacks, and idempotency; document eager vs lazy usage in README.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| zeno.zsh | Switches the main entrypoint to source the new bootstrap + invoke eager init. |
| zeno-bootstrap.zsh | New lightweight bootstrap that sets ZENO_ROOT, updates path/fpath, and registers widgets without heavy initialization. |
| shells/zsh/functions/zeno-init | New heavy initialization entrypoint (cache, sock, hooks, sets ZENO_LOADED). |
| shells/zsh/functions/zeno-ensure-loaded | New helper to ensure heavy init is done (idempotent). |
| shells/zsh/functions/zeno-preload | New public alias for preloading (zeno-ensure-loaded). |
| shells/zsh/functions/zeno-register-lazy-widget | New API to register a widget as lazy-dispatched. |
| shells/zsh/functions/zeno-register-lazy-widgets | New batch API for lazy widget registration. |
| shells/zsh/functions/zeno-lazy-widget-dispatch | New dispatcher that ensures load before calling the real widget and falls back on failure. |
| shells/zsh/functions/zeno-run-lazy-fallback | New upstream fallback behavior for common widgets when lazy load fails. |
| shells/zsh/functions/zeno-bind-default-keys | New upstream helper to bind default keys in eager or lazy mode. |
| shells/zsh/functions/zeno-enable-sock | Makes hook registration idempotent by removing existing hooks before adding them. |
| test/shell/zsh_test_utils.ts | Exposes additional paths for shell tests (repo root / entrypoints). |
| test/shell/zsh_lazy_api_test.ts | Adds tests validating lazy APIs, dispatch behavior, fallbacks, and key binding helper. |
| test/shell/zsh_init_test.ts | Adds tests for eager entrypoint parity, bootstrap deferral, wrapper-init, and idempotency. |
| README.md | Documents eager vs lazy usage and the new public APIs/state variables. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@shells/zsh/functions/zeno-bind-default-keys`:
- Around line 31-39: The lazy and eager branches ignore failures when
registering widgets; change zeno-bind-default-keys so that after calling
zeno-register-lazy-widgets it checks the return status and returns immediately
on non-zero, and in the eager branch check the exit status of each zle -N call
inside the for loop and return immediately (or propagate the error) if any zle
-N fails, before proceeding to bindkey; reference zeno-register-lazy-widgets,
zeno-register-lazy-widget, zeno-ensure-loaded, and the zle -N invocations to
locate and guard the failing calls.
In `@shells/zsh/functions/zeno-init`:
- Around line 15-37: The init function unconditionally exports ZENO_LOADED=1
even if earlier steps fail; update the failing steps—specifically the deno cache
invocation, the zeno-enable-sock call inside the deno_version branch, and the
zeno-history-hooks and zeno-preprompt-hooks invocations—to propagate errors back
to the caller by appending a short-circuit return (e.g., use "|| return $?" or
equivalent) so that any failure prevents exporting ZENO_LOADED and stops
initialization.
In `@shells/zsh/functions/zeno-run-lazy-fallback`:
- Around line 10-12: The current fallback selects fzf-completion based on
functions[fzf-completion], but zle requires a registered widget; update the
selection in zeno-run-lazy-fallback (and the same pattern in
shells/zsh/widgets/zeno-completion) to check the $widgets array for
'fzf-completion' (or use the existing ZENO_COMPLETION_FALLBACK override) before
assigning fallback_widget, and if it is not present fall back to
'expand-or-complete'; then call zle "$fallback_widget" as before so an
unregistered function won’t cause zle to error.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: efe429b0-1113-4a53-ba71-fb3537d6b400
📒 Files selected for processing (15)
README.mdshells/zsh/functions/zeno-bind-default-keysshells/zsh/functions/zeno-enable-sockshells/zsh/functions/zeno-ensure-loadedshells/zsh/functions/zeno-initshells/zsh/functions/zeno-lazy-widget-dispatchshells/zsh/functions/zeno-preloadshells/zsh/functions/zeno-register-lazy-widgetshells/zsh/functions/zeno-register-lazy-widgetsshells/zsh/functions/zeno-run-lazy-fallbacktest/shell/zsh_init_test.tstest/shell/zsh_lazy_api_test.tstest/shell/zsh_test_utils.tszeno-bootstrap.zshzeno.zsh
- resolve ZENO_ROOT from the bootstrap file path - fail fast when required zsh asset directories are missing - add regression tests for wrapper sourcing and invalid roots
- add ~/.config as the first XDG config base when XDG_CONFIG_HOME is empty - reuse the same XDG base resolution in config path lookup and config manager - add regression tests for ~/.config discovery and isolate config tests from host env
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/config/loader.ts`:
- Around line 59-68: The code treats an untrimmed whitespace-only
XDG_CONFIG_HOME as set, skipping the fallback; fix by trimming the
environment/param value before the presence check and before passing into
appendUniqueDir: read the raw value (params?.xdgConfigHome ??
Deno.env.get("XDG_CONFIG_HOME")), compute a trimmedXdg = raw?.trim() ||
undefined, use trimmedXdg in the ternary that sets fallbackConfigHome and in the
later appendUniqueDir call(s), ensuring xdgConfigHome/fallbackConfigHome are
based on trimmed input.
In `@src/config/manager.ts`:
- Around line 493-501: The cache key for settings omits XDG environment
variables, causing stale results when XDG_CONFIG_HOME or XDG_CONFIG_DIRS change;
update the settings cache signature (where getSettings() builds its cache key)
to also include Deno.env.get("XDG_CONFIG_HOME") and
Deno.env.get("XDG_CONFIG_DIRS") (or their parsed/normalized equivalents
returned/used by getXdgConfigBaseDirs/parseXdgConfigDirs) so that changes to
those variables invalidate the cache and new config locations are discovered.
In `@test/shell/zsh_init_test.ts`:
- Around line 258-289: The test file formatting is out of sync; run the
formatter on the failing test (the "source zeno-bootstrap.zsh fails fast when
ZENO_ROOT does not contain zsh assets" test) to fix the spacing/indentation
around the block that builds the script with runZshScriptRaw, shellQuote and
ZSH_BOOTSTRAP_ENTRYPOINT; run the repository formatter (deno fmt) targeting
test/shell/zsh_init_test.ts (or the whole repo) and re-run CI to ensure no
remaining formatting diffs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 409eda14-4877-4481-9f0b-fc547aadba23
📒 Files selected for processing (9)
src/config/loader.tssrc/config/manager.tstest/config/loader_test.tstest/config/manager_multi_yaml_test.tstest/config/manager_typescript_test.tstest/settings_test.tstest/shell/zsh_init_test.tstest/shell/zsh_test_utils.tszeno-bootstrap.zsh
🚧 Files skipped from review as they are similar to previous changes (1)
- zeno-bootstrap.zsh
- propagate lazy init and widget registration failures - fix XDG config env handling and cache invalidation - add regression coverage for review findings
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@shells/zsh/functions/zeno-bind-default-keys`:
- Around line 21-50: The lazy widget list omits zeno-insert-space and
zeno-toggle-auto-snippet so in --lazy mode those keys are bound without
dispatcher handlers; update the lazy_widgets array to include
"zeno-insert-space" and "zeno-toggle-auto-snippet" (or remove their bindkey
calls in lazy mode), and ensure zeno-run-lazy-fallback has a fallback entry for
zeno-toggle-auto-snippet if you keep lazy binding; locate the array named
lazy_widgets and the registration call zeno-register-lazy-widgets to make the
change so both widgets are registered by the lazy dispatcher.
In `@src/config/loader.ts`:
- Around line 38-48: The function appendUniqueDir currently only trims input
which allows semantically identical paths like "/etc/xdg" and "/etc/xdg/" to be
considered different; update appendUniqueDir to canonicalize the dir before
deduplicating by using path normalization/resolution (e.g., call path.normalize
or path.resolve on dir after trimming and remove any trailing path separators)
so the normalized value is what you check against seen, add to seen, and push
onto target; update callers such as getXdgConfigBaseDirs() to rely on this
canonicalized behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: df23a27a-c6b7-4337-b1b7-a338ca1068a4
📒 Files selected for processing (12)
shells/zsh/functions/zeno-bind-default-keysshells/zsh/functions/zeno-initshells/zsh/functions/zeno-run-lazy-fallbackshells/zsh/widgets/zeno-completionsrc/config/loader.tssrc/config/manager.tstest/config/loader_test.tstest/settings_test.tstest/shell/zsh_completion_widget_basic_test.tstest/shell/zsh_init_test.tstest/shell/zsh_lazy_api_test.tszeno.zsh
🚧 Files skipped from review as they are similar to previous changes (4)
- shells/zsh/functions/zeno-run-lazy-fallback
- test/config/loader_test.ts
- src/config/manager.ts
- test/settings_test.ts
- assert zeno-insert-space and zeno-toggle-auto-snippet stay usable in lazy mode - verify those lightweight widgets do not trigger heavy initialization
- canonicalize trailing separators before deduplicating config roots - add regression coverage for slash-suffixed XDG directories
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/shell/zsh_lazy_api_test.ts (1)
254-256: Consider named assertions instead of hardcoded indices.The array indices
[1],[3],[10]assume a specific binding order. If the implementation changes the order ofbindkeycalls, these tests will break silently or produce confusing failures.A more robust approach could search the log array for expected patterns:
# Example: Find binding containing "zeno-auto-snippet" for entry in "${ZENO_TEST_BINDKEY_LOG[@]}"; do [[ "$entry" == *"zeno-auto-snippet"* ]] && print -r -- "$entry" doneThis is a minor concern since the current approach does validate the expected behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/shell/zsh_lazy_api_test.ts` around lines 254 - 256, The test currently asserts bindkey outputs by indexing into ZENO_TEST_BINDKEY_LOG (e.g. ZENO_TEST_BINDKEY_LOG[1], [3], [10]) which is brittle; change the assertions to search ZENO_TEST_BINDKEY_LOG for entries matching the expected patterns instead of fixed indices—iterate over "${ZENO_TEST_BINDKEY_LOG[@]}" and check each entry for the expected key/value (use pattern matches for strings used by zeno-test-print-kv) and assert presence; update the assertions that reference ZENO_TEST_BINDKEY_LOG and zeno-test-print-kv so they verify existence of a matching entry rather than relying on a specific array index.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@test/shell/zsh_lazy_api_test.ts`:
- Around line 254-256: The test currently asserts bindkey outputs by indexing
into ZENO_TEST_BINDKEY_LOG (e.g. ZENO_TEST_BINDKEY_LOG[1], [3], [10]) which is
brittle; change the assertions to search ZENO_TEST_BINDKEY_LOG for entries
matching the expected patterns instead of fixed indices—iterate over
"${ZENO_TEST_BINDKEY_LOG[@]}" and check each entry for the expected key/value
(use pattern matches for strings used by zeno-test-print-kv) and assert
presence; update the assertions that reference ZENO_TEST_BINDKEY_LOG and
zeno-test-print-kv so they verify existence of a matching entry rather than
relying on a specific array index.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3f40c3b7-9d1a-4e6f-b7b9-8ce21b62470d
📒 Files selected for processing (3)
src/config/loader.tstest/config/loader_test.tstest/shell/zsh_lazy_api_test.ts
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
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
source zeno.zsheager and backward-compatibleTesting
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Tests