Zsh/fish/nushell support#2718
Merged
Merged
Conversation
Contributor
🔍 Suggested ReviewersBased on git blame analysis of the changed lines, the following contributors have significant experience with the modified code:
Please consider reviewing this PR as you have authored significant portions of the code being modified. Your expertise would be valuable! 🙏 This comment was automatically generated by git-blame-auto-reviewer Last updated: 2026-04-24T09:21:46.154Z |
Deploying devenv with
|
| Latest commit: |
027f155
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://bda2b163.devenv.pages.dev |
| Branch Preview URL: | https://zsh-shell-support.devenv.pages.dev |
702d81f to
9efc1af
Compare
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The reloaded_remaining branch incorrectly used tokio::select! with a std::sync::mpsc::Receiver, whose recv() returns a Result, not a Future. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The `--shell` argument field in ShellCliArgs was named `shell`, giving it the clap internal ID "shell". This collided with the `Shell` subcommand in the Commands enum, which also has the ID "shell". When clap tried to access the arg value, it found the subcommand match instead and panicked with "Mismatch between definition and access". Rename the field to `shell_type` while keeping `--shell` as the CLI flag name, so the internal ID becomes "shell_type". Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Two fixes for nushell hot-reload: 1. Use `def --env` for __devenv_reload_apply so environment changes propagate to the caller when triggered via Ctrl+Alt+R keybinding. 2. Add auto-reload in pre_prompt hook, mirroring bash's PROMPT_COMMAND behavior. The reload logic is inlined in the closure because nushell closures don't propagate env changes from `def --env` functions called within hooks. Without these fixes, Ctrl+Alt+R would run the reload function but discard all environment changes (missing --env flag), and there was no automatic reload on prompt like bash/zsh have. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…TH hack Switch to domenkozar/libghostty-rs#pkg-config-support which adds pkg-config probing to the sys crate. The nix-built libghostty-vt package now provides a .pc file (generated by the zig build), so the sys crate finds it via pkg-config and NixOS automatically adds rpath. This eliminates the enterShell hack that dynamically searched for the library in cargo build directories. Also update the ghostty pin to 01825411 which generates .pc files natively, and adapt to the updated libghostty-vt API (new `selection` field in FormatterOptions, changed format_alloc signature). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…alidation
- Zsh: register combined precmd hook that calls both __devenv_reload_apply
and __devenv_restore_path, so env reloads auto-apply at each prompt
- Nushell: call __devenv_reload_apply from pre_prompt hook to apply all env
changes (not just PATH) on each prompt
- Fish: use portable single-quote escaping pattern ('\'') instead of \'
which only works in fish >= 3.3.0
- Fish: escape env var keys with shell_escape for consistency with bash/zsh
- Nushell: validate env var keys are valid identifiers before emitting $env.X
- create_dialect: warn on unrecognized shell name before falling back to bash
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace the manually maintained nix/libghostty-vt.nix (which duplicated the ghostty zig build with a hardcoded commit pin) with the official ghostty flake's libghostty-vt package. The crate2nix build.rs override is also removed since libghostty-vt-sys's pkg-config feature finds the library natively. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The cachix/iocraft `cachix` branch was stuck at 0.7.18, so the [patch.crates-io] entry no longer applied to the workspace's 0.8.0 version bound. Cargo silently pulled both iocraft 0.7.18 (git) and iocraft 0.8.0 (registry), causing rustc metadata hash mismatches between devenv-shell and the rest of the workspace and surfacing as `E0463: can't find crate for devenv_reload` during `nix build`. The row-level-diff branch carries iocraft 0.8.0, so the patch applies again and only one iocraft remains in the dependency graph. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…snapshots On macOS the long keybind form renders as `Ctrl-Opt-*` while on Linux it's `Ctrl-Alt-*`, causing insta snapshots to fail on darwin CI. Add a scoped insta filter (`Ctrl-Opt-([A-Z]) → Ctrl-Alt-$1`) applied to each test that snapshots the status line, so a single `.snap` file works on both platforms without changing production rendering. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Commit 492fb41 ("remove dead code") dropped the local `pkgs.callPackage ./nix/libghostty-vt.nix` entry from `devenv.nix` but never added a replacement, leaving `libghostty-vt-sys`'s build script with nothing on pkg-config and falling through to a zig source build — also removed in the same commit. The `libghostty-vt` attribute added to `flake.nix`'s overlay in 492fb41^ only applies to the flake dev shell, not the devenv user shell, which instantiates nixpkgs independently. Declare `ghostty` as a devenv input and pull `inputs.ghostty.packages.\${system}.libghostty-vt` directly into `devenv.nix`'s `packages` so `pkg-config --modversion libghostty-vt` resolves and `cargo check -p devenv-shell --features test-pty` links without needing `zig`. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
blake3 1.8.4 upgraded its digest dependency from 0.10 to 0.11, which is incompatible with snix-castore's digest 0.10 API usage and broke the build. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
xtask transitively links devenv-shell -> libghostty-vt-sys, but crate2nix
doesn't propagate libghostty-vt.so.0 into the binary's rpath. The
wrapDevenv installPhase invokes xtask to generate manpages and shell
completions, which failed on aarch64-linux with "libghostty-vt.so.0:
cannot open shared object file". x86_64-linux masked this by serving
the derivation from Cachix.
Pass -Wl,-rpath,${libghostty-vt}/lib via extraRustcOpts in the devenvBase
crate override so all three binaries (devenv, devenv-run-tests, xtask)
carry the rpath at link time. Drop the now-redundant LD_LIBRARY_PATH
prefixes from the makeWrapper calls.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…orkaround Bumps the ghostty input (both flake.nix and devenv.yaml) to domenkozar/ghostty@8ada077e, which fixes libghostty-vt's pkg-config file to carry the correct split-output libdir under Nix multi-output packaging. Previously the .pc file's `libdir` was interpolated against the `-dev` output via `${prefix}/lib`, so pkg-config-built consumers picked up an rpath to $dev/lib where the SONAME-versioned library did not exist. The loader then failed at runtime with `libghostty-vt.so.<N>: cannot open shared object file`. With the fix, pkg-config emits `-L$out/lib` directly and the NixOS cc-wrapper auto-rpaths the correct directory for both `nix build` and cargo-built binaries inside `devenv shell`. The xtask install step of `nix build .#devenv` now resolves libghostty-vt.so.0 from xtask itself, and `cargo nextest` inside the devenv shell produces test binaries whose dynamic loader can find the library. Drops the `-Wl,-rpath,${libghostty-vt}/lib` extraRustcOpts workaround added in 132f437 (crate-config.nix): the upstream pkg-config fix makes it redundant. The workaround only covered crate2nix-built binaries and left cargo-built test binaries broken, which is why the original CI run on this branch still failed even with that commit. Aligns flake.nix's ghostty input with devenv.yaml's (both now track github:domenkozar/ghostty/nix-darwin-libghostty-vt) until the fix lands upstream in ghostty-org/ghostty. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The pkg-config fix in ghostty works for cc-wrapper mediated builds but crate2nix's buildRustCrate does not go through cc-wrapper, so the rpath is not embedded automatically. Re-add the explicit linker arg so xtask (and other devenvBase binaries) find libghostty-vt at runtime on both macOS and Linux. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The Nix shell environment (from mkShell/stdenv) exports SHELL pointing to the Nix store bash, overriding what apply_shell_env sets on the process. Re-export SHELL after sourcing the devenv env script so non-bash shell sessions see the correct value. Uses command -v to resolve the target shell to an absolute path after the Nix env is sourced, so $SHELL is correct even when the shell is provided by devenv packages rather than the outer system PATH. Also handles the non-interactive command path (bash_init_script) in addition to the dialect rcfiles. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
pkg-config nows works on macOS without manual rpath configuration for every downstream crate. The build will re-run when pkg-config env vars change.
e2adb07 to
3f16ca9
Compare
I think the original issue is gone now. This was not a proper solution anyways.
Without this guard, aborting the manager's tokio task left the PTY child running and stdin/file-watcher threads holding event_tx clones, so the VT thread blocked on recv() forever and the surrounding spawn_blocking join hung the runtime indefinitely. The guard kills the PTY and posts a Shutdown event so the VT loop exits cleanly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…build panic Three independent bug fixes in the manager event loop that the avt to libghostty-rs migration introduced or exposed: * Replay captured terminal state to stdout instead of into the new PTY. Writing escape sequences into the new shell's stdin caused the shell to interpret them as keystrokes and emit garbage characters. * Bound-join the previous PTY reader thread on swap. portable-pty's master fd EOFs the reader when the killed child closes the slave on POSIX, but a 500ms is_finished poll bounds the wait so a stuck reader cannot block reload (Windows ConPTY worst case is detached). * Wrap builder.build in catch_unwind so a panicking builder still sends BuildComplete. Without this, the building flag stayed true forever and every subsequent file change was silently dropped. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Move EscapeState, process_escape_events, and cleanup_forwarded_modes out of impl ShellSession and into devenv-shell/src/escape_state.rs as free items, plus promote escape and terminal_commands to pub modules. Pure mechanical refactor preserving session.rs behaviour byte for byte; the EscapeState unit tests move with the type. Enables devenv-reload to reuse the same mode tracking and on-exit reset without duplicating ~250 lines of escape parsing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Track DEC mode escapes (alt-screen, mouse, bracketed paste, kitty keyboard, etc.) as the embedded shell sets them on the user's terminal, then call cleanup_forwarded_modes before the new shell starts. Without this, modes leaked across reloads and the new shell inherited a dirty terminal (e.g. stuck in alt-screen after running htop). Reuses the EscapeState plumbing extracted from devenv-shell. Passes io::sink() to process_escape_events because manager.rs already forwards raw bytes to stdout itself; only the state-tracking side effect is needed (PTY responses to TextAreaSizeQuery still go to the real PTY). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
Hoorah! |
4 tasks
giodamelio
added a commit
to giodamelio/nixos-configs
that referenced
this pull request
May 6, 2026
The zsh/fish/nushell shell support PR (cachix/devenv#2718) has been released, so the tracking branch is no longer needed.
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
3 participants
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.
Fixes #2592
Fixes #36
Fixes #2487
Fixes #2593