feat: add runtime-configurable shell hook controls#247
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces runtime-configurable Fish conf.d hook behavior in pez, making hook execution opt-in via config and providing per-command and wrapper-local overrides.
Changes:
- Adds
[shell_hooks](emit,source) topez.toml(plus schema + parsing defaults) and uses it to control whether hooks are emitted/sourced. - Extends CLI commands (
install/upgrade/uninstall/activate) with hook override flags and adds ahook-configcommand to show the effective hook configuration. - Updates Fish activation wrapper, diagnostics (
doctor), and documentation to reflect the new explicit/opt-in behavior.
Reviewed changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils.rs | Adds shell hook override/config resolution and gates emit_event via config/overrides. |
| src/lib.rs | Wires new hook-config command and passes activate args into Fish wrapper generation. |
| src/config.rs | Introduces ShellHooksConfig, adds it to Config, updates defaults and tests. |
| src/cmd/upgrade.rs | Adds --emit-hooks overrides into upgrade flows and event emission. |
| src/cmd/uninstall.rs | Adds --emit-hooks overrides into uninstall flows and event emission. |
| src/cmd/prune.rs | Updates tests to include new shell_hooks config field. |
| src/cmd/mod.rs | Registers the new hook_config module. |
| src/cmd/migrate.rs | Ensures migration-triggered installs pass new CLI flags; updates tests for config shape. |
| src/cmd/list.rs | Updates tests to include new shell_hooks config field. |
| src/cmd/install.rs | Adds --emit-hooks overrides into install flows and event emission. |
| src/cmd/hook_config.rs | New command to compute/print effective hook config (config + CLI + --from passthrough). |
| src/cmd/doctor.rs | Makes doctor aware of shell hook settings and activation requirements. |
| src/cmd/activate.rs | Updates Fish activation wrapper to query hook config at runtime and apply wrapper-local overrides. |
| src/cli.rs | Adds hook-config command and new hook override flags across relevant subcommands. |
| src/bin/gen_config_schema.rs | Adds test ensuring checked-in schema matches generated output. |
| README.md | Documents new shell hook defaults and behavior; updates command list and security notes. |
| docs/migrate-from-fisher.md | Updates migration steps to incorporate opt-in shell hook behavior. |
| docs/faq.md | Updates hook guidance to require enabling config + activation wrapper. |
| docs/configuration.md | Documents [shell_hooks], defaults, and CLI/env override interactions. |
| docs/commands.md | Documents new flags and the hook-config command. |
| docs/architecture.md | Updates architecture notes to reflect runtime hook config and optional emission. |
| config.schema.json | Updates checked-in schema to include shell_hooks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| set -l hook_config (command pez hook-config --format json --from $from $__pez_hook_config_args -- $passthrough) | ||
| set -l source_enabled 0 | ||
| set -l emit_enabled 0 | ||
| if string match -rq '.*"source"[[:space:]]*:[[:space:]]*true.*' -- $hook_config | ||
| set source_enabled 1 | ||
| end | ||
| if string match -rq '.*"emit"[[:space:]]*:[[:space:]]*true.*' -- $hook_config |
There was a problem hiding this comment.
hook_config is captured via command substitution into a Fish variable (set -l hook_config (...)). In Fish, command substitution output is split on whitespace/newlines into a list, so $hook_config becomes many tokens. The subsequent string match -rq '.*"source"...true.*' -- $hook_config checks each token independently and will never match the multi-token JSON, meaning source_enabled/emit_enabled will stay 0 even when hooks are enabled. Consider collecting the command output into a single string (e.g., pipe through string collect, or quote/join it), and/or emit compact single-line JSON from pez hook-config for wrapper consumption (and simplify the regex so it doesn't rely on .*).
| set -l hook_config (command pez hook-config --format json --from $from $__pez_hook_config_args -- $passthrough) | |
| set -l source_enabled 0 | |
| set -l emit_enabled 0 | |
| if string match -rq '.*"source"[[:space:]]*:[[:space:]]*true.*' -- $hook_config | |
| set source_enabled 1 | |
| end | |
| if string match -rq '.*"emit"[[:space:]]*:[[:space:]]*true.*' -- $hook_config | |
| set -l hook_config (command pez hook-config --format json --from $from $__pez_hook_config_args -- $passthrough | string collect) | |
| set -l source_enabled 0 | |
| set -l emit_enabled 0 | |
| if string match -rq '"source"[[:space:]]*:[[:space:]]*true' -- "$hook_config" | |
| set source_enabled 1 | |
| end | |
| if string match -rq '"emit"[[:space:]]*:[[:space:]]*true' -- "$hook_config" |
| pub(crate) fn load_shell_hooks_config() -> anyhow::Result<config::ShellHooksConfig> { | ||
| match load_config() { | ||
| Ok((config, _)) => Ok(config.shell_hooks), | ||
| Err(_) => Ok(config::init().shell_hooks), | ||
| } |
There was a problem hiding this comment.
load_shell_hooks_config falls back to defaults on any load_config() error, including parse/validation errors in an existing pez.toml. That can silently disable hooks when the config is malformed, making it hard for users to diagnose. Suggest only defaulting when the config file is missing, and propagating (or at least warning on) real load/parse errors.
| let mut shell_hooks = resolve_shell_hooks_with_override(override_value)?; | ||
| if env::var_os("PEZ_SUPPRESS_EMIT").is_some() { | ||
| shell_hooks.emit = false; | ||
| } |
There was a problem hiding this comment.
emit_event_with_override resolves the shell hook config by reading/parsing pez.toml on every emitted event. In install/upgrade/uninstall this can be called once per conf.d file, causing repeated disk reads and parse work. Consider resolving the effective ShellHooksConfig once per command (after applying overrides) and passing it through, or caching within a command invocation.
| let mut shell_hooks = resolve_shell_hooks_with_override(override_value)?; | |
| if env::var_os("PEZ_SUPPRESS_EMIT").is_some() { | |
| shell_hooks.emit = false; | |
| } | |
| if env::var_os("PEZ_SUPPRESS_EMIT").is_some() { | |
| // When emission is globally suppressed, avoid resolving shell hooks | |
| // and return early. | |
| return Ok(()); | |
| } | |
| let mut shell_hooks = resolve_shell_hooks_with_override(override_value)?; |
This pull request introduces a comprehensive and configurable shell hook system for pez, allowing users to control how and when Fish shell
conf.dhooks are emitted or sourced. It adds new configuration options, updates documentation, and enhances CLI commands to support runtime and per-command overrides for hook behavior. The changes make hook execution explicit, safer by default, and much more flexible, especially for users migrating fromfisher.Shell hook configuration and behavior:
Adds a
[shell_hooks]section topez.toml(and schema) withemitandsourcebooleans (both default tofalse), letting users control if and howconf.dhooks are executed. The activation wrapper now reads this config at runtime, so changes take effect immediately without regenerating the wrapper. [1] [2] [3] [4] [5] [6] [7]Updates the activation wrapper (
pez activate fish) to use the new runtime-configured shell hook logic, and documents the new behavior and config in the README and migration guide. [1] [2] [3] [4]CLI and command enhancements:
Adds new CLI flags (
--emit-hooks,--no-emit-hooks,--source-hooks,--no-source-hooks) toinstall,upgrade,uninstall, andactivatecommands, allowing per-command override of hook behavior. [1] [2] [3] [4] [5] [6] [7]Introduces a new
hook-configcommand to display the effective shell hook configuration, supporting CLI overrides and subcommand context. [1] [2] [3]Documentation and migration improvements:
Thoroughly updates documentation (
README.md,docs/commands.md,docs/configuration.md,docs/faq.md,docs/migrate-from-fisher.md,docs/architecture.md) to explain the new shell hook system, its defaults, migration steps, and security implications. [1] [2] [3] [4] [5] [6] [7]Clarifies that copying
conf.dfiles is always performed, but emitting or sourcing hooks is only done if enabled via config or CLI override. [1] [2]These changes make pez's hook behavior explicit, opt-in, and highly configurable, providing a safer default for new users and a familiar experience for those migrating from fisher.