Skip to content

feat: add runtime-configurable shell hook controls#247

Open
tetzng wants to merge 1 commit intomainfrom
push-uwnspzptyrqr
Open

feat: add runtime-configurable shell hook controls#247
tetzng wants to merge 1 commit intomainfrom
push-uwnspzptyrqr

Conversation

@tetzng
Copy link
Copy Markdown
Owner

@tetzng tetzng commented Apr 2, 2026

This pull request introduces a comprehensive and configurable shell hook system for pez, allowing users to control how and when Fish shell conf.d hooks 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 from fisher.

Shell hook configuration and behavior:

  • Adds a [shell_hooks] section to pez.toml (and schema) with emit and source booleans (both default to false), letting users control if and how conf.d hooks 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) to install, upgrade, uninstall, and activate commands, allowing per-command override of hook behavior. [1] [2] [3] [4] [5] [6] [7]

  • Introduces a new hook-config command 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.d files 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.

@tetzng tetzng requested a review from Copilot April 2, 2026 15:53
@tetzng tetzng self-assigned this Apr 2, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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) to pez.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 a hook-config command 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.

Comment on lines +78 to +84
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
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 .*).

Suggested change
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"

Copilot uses AI. Check for mistakes.
Comment on lines +127 to +131
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),
}
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
Comment on lines +384 to +387
let mut shell_hooks = resolve_shell_hooks_with_override(override_value)?;
if env::var_os("PEZ_SUPPRESS_EMIT").is_some() {
shell_hooks.emit = false;
}
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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)?;

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants