Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #1025 +/- ##
==========================================
+ Coverage 81.01% 81.95% +0.93%
==========================================
Files 28 32 +4
Lines 2629 3087 +458
Branches 492 601 +109
==========================================
+ Hits 2130 2530 +400
- Misses 368 408 +40
- Partials 131 149 +18 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
d64904e to
db303db
Compare
Code reviewFound 4 issues:
tmuxp/src/tmuxp/workspace/builder.py Lines 674 to 680 in db303db
Lines 82 to 88 in db303db Lines 81 to 88 in db303db Lines 81 to 88 in db303db Lines 82 to 88 in db303db
Lines 326 to 334 in db303db
tmuxp/src/tmuxp/workspace/importers.py Lines 93 to 107 in db303db 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
…amocil Comprehensive side-by-side comparison covering architecture, config keys, CLI commands, hooks, and config file discovery across all three tools.
Documents 12 feature gaps (hooks, stop command, pane sync, pane titles, ERB templating, wemux, debug/dry-run, config management CLIs), import behavior with bug inventory, and WorkspaceBuilder requirements.
Documents v0.x vs v1.x format differences, 3 feature gaps (--here flag, debug mode, shell_command_after), import bugs (v1.x incompatibility, redundant filter loops), and WorkspaceBuilder requirements.
Classifies each config key as difference (translatable) or limitation (needs tmuxp feature). Identifies pre/pre_window bug, missing rvm/pre_tab mappings, and 5 features requiring new tmuxp capabilities.
Documents v0.x-only targeting (v1.x unsupported), string pane TypeError bug, redundant filter loop bug, and 6 missing v1.x key mappings (commands, focus, options, string shorthand).
- Remove duplicate 'Attach on create' row in comparison table, keep
corrected version with '(default: true)' near socket_path
- Annotate pre_tab as (deprecated) in comparison table
- Annotate startup_window as accepting name or index
- Fix pre_tab description: deprecated predecessor, not alias (it was
renamed in tmuxinator, not aliased)
- Clarify startup_window renders as "#{name}:#{value}"
- tmuxinator min tmux is 1.8 (recommended), not 1.5; tmux 2.5 is explicitly unsupported - teamocil has no documented min tmux version - tmuxinator detach is via `attach: false` config or `--no-attach` CLI flag, not `-d` (which doesn't exist in tmuxinator)
- Add socket_path as item 16 (tmuxinator config key not handled) - socket_path takes precedence over socket_name in tmuxinator - tmuxp only accepts socket path via CLI -S flag - Add to summary table as missing Difference
- with_env_var is an import-only fix (tmuxp already has environment key), not a Limitation — moved to new "Import-Only Fixes" section - cmd_separator is irrelevant (tmuxp sends commands individually), clarified it needs no import
- Fix "1.5+" to "1.8+" in architecture description (was already fixed in overview table but missed in prose) - Clarify YAML anchors: tmuxinator enables via YAML.safe_load aliases param, not a config key - Clarify tmuxinator edit is alias of new command
…s equivalent tmuxp doesn't have startup_window/startup_pane keys but achieves the same result via focus: true on individual windows/panes. Add cross-reference annotation so users aren't misled by (none).
- before_script maps to on_project_first_start (runs only when session doesn't exist), not on_project_start (runs every invocation) - Add teamocil --here implementation details: sends cd via send-keys, decrements window count for index calculation
- import-teamocil.md: Code block comment said "Lines 144-149" but the `if "filters"` guard is on line 143, so range is 143-149 - parity-teamocil.md: Referenced "Line 142" for `clear` handling but actual code is lines 140-141 (line 142 is blank)
…, expand CLI table - Fix min tmux: 1.5+ (not "1.8 recommended; not 2.5"), per tmux_version.rb - Note teamocil renames session (rename-session) rather than creating new - Add teamocil auto-generated session name detail - Expand pre_window to show full deprecation chain (rbenv/rvm/pre_tab) - Add synchronize values (true/before/after) - Add --suppress-tmux-version-warning to CLI table - Split deprecated pre/post into separate rows with hook mappings - Fix tmuxp --append flag syntax - Fix pane focus to note startup_pane equivalent
… chain, remove --here - Fix startup_window: accepts name OR index (not just name) - Document pre_window fallback chain: rbenv → rvm → pre_tab → pre_window - Remove section 12 (--here) — this is a teamocil feature, not tmuxinator - Renumber section 13 → 12 - Clarify freeze vs tmuxinator new comparison - Add rvm source reference (project.rb:181) - Add tmuxinator version range to header
…md_separator - Add section 1: teamocil renames session (rename-session), not creates - Note auto-generated session name (teamocil-session-RANDOM) - Add window focus implementation detail (session.rb:24-25) - Add --list and --edit note for teamocil CLI - Reclassify with_env_var and cmd_separator as unverified (not in source) - Add session rename mode to WorkspaceBuilder gaps - Fix line number references (144-149, 147-149, 161-163) - Renumber sections to account for new section 1
… analysis - Fix pre/pre_window mapping: pre → before_script (session-level, runs once), pre_window → shell_command_before (per-pane) - Add template.erb line references for execution order - Expand cli_args fragility analysis (str.replace is unsafe) - Add tmuxinator source references for tmux_options and socket handling
…tale TODOs - with_env_var does not exist in teamocil 1.4.2 source - cmd_separator does not exist in teamocil 1.4.2 source - Both are only in importer docstring TODOs (importers.py:121-123) - Reclassify both as unverified in summary table - Update code issues section to note stale TODOs
Analyze libtmux and tmuxp limitations blocking feature parity with tmuxinator and teamocil. Document dead config keys, importer bugs, and required API additions organized by implementation phase.
…ails - L2: Fix method name (raise_if_dead, not is_alive), document two independent code paths - L4: Fix Pane.select() line number (577, not 581) - T4: Add session rename mode alongside --here, note --append gap - T8: Correct env var expansion scope (works in most values, not just start_directory) - I1: Document isinstance check bug (checks pre type instead of pre_window type)
L1: pane_title is excluded from libtmux's bulk format queries, not
removed from tmux itself — tmux still supports #{pane_title}
(format.c:205) and select-pane -T (added in tmux 2.6).
T1: synchronize before/true insertion point is build() line 320
(after on_window_create hook, before iter_create_panes loop), not
iter_create_windows() line 424 which is inside the generator.
T3: shell_command_after is a window-level key (set by teamocil
importer on window_dict), not per-pane. Correct insertion point
is config_after_window() or after the pane loop in build().
T2: Session-level pane title options insertion is alongside other session options at lines 303-309, not "around line 311". Pane-level title should be set after commands are sent (around line 535), before focus handling at line 536. I7: Stale TODOs are at lines 121 and 123 (not 121-123), since line 122 is `clear` which is a real teamocil feature.
why: The --debug handler was installed before YAML parsing but cleanup was only reached via conditional paths. A parse error leaked the handler. what: - Wrap config parse and expand in try/except that calls _cleanup_debug - Ensures handler is removed even if _from_file or expand raises
why: EDITOR='code -w' passed as a single string to subprocess.call raised FileNotFoundError because the space was part of the command name. what: - Use shlex.split(sys_editor) to split editor command and flags - Add test with EDITOR containing flags
…k keys why: The importer silently dropped enable_pane_titles, pane_title_*, and on_project_* keys even though both tmuxinator and tmuxp support them. what: - Add passthrough for enable_pane_titles, pane_title_position, pane_title_format - Add passthrough for on_project_start, restart, exit, stop - Preserve on_project_first_start with warning (not yet supported in builder) - Add tests verifying passthrough and warning behavior
…tach, post keys why: These tmuxinator YAML keys were silently dropped with no warning, unlike other unsupported keys which already log warnings. Users migrating from tmuxinator with tmux_command: wemux or attach: false got no feedback. what: - Log WARNING for tmux_command (no custom binary support in tmuxp) - Log WARNING for attach (use tmuxp load -d instead) - Log WARNING for post (deprecated; use on_project_exit instead) - Add parametrized tests with NamedTuple fixtures
why: tmuxinator validates pane_title_position against ["top","bottom","off"]
(project.rb:472) but tmuxp passed any value through to tmux's
pane-border-status, causing cryptic tmux errors for invalid values.
what:
- Validate position against {"top", "bottom", "off"}
- Log WARNING and default to "top" for invalid values
- Add parametrized tests with NamedTuple fixtures
…mands why: test_load_workspace_no_shell_command_before had expect_before_cmd parametrized but never used in assertions. It only checked session.name, passing trivially even if the flag was broken. what: - Capture pane output and verify __BEFORE__ presence based on expect_before_cmd - Use retry_until for positive case, time.sleep+assert for negative case
why: subprocess.call raised unhandled FileNotFoundError when EDITOR was set to a nonexistent binary, crashing after the workspace file was already created. what: - Catch FileNotFoundError and show helpful error with colors - Replace single test with parametrized NamedTuple fixture covering valid editor, editor with flags, and missing editor
… pre why: pre_window and pre_tab were only mapped to shell_command_before when pre was also present. tmuxinator treats pre_window independently (project.rb:175, template.erb:60,71). A config with only pre_window silently lost the per-window command. what: - Add elif branch for pre_window_val when pre is absent - Handle list pre_window by joining with "; " (matches tmuxinator) - Add parametrized tests covering all 5 pre/pre_window combinations
…ndex resolution why: tmuxinator passes startup_window directly to tmux as a target (project.rb:262), where tmux resolves against base-index. tmuxp uses 0-based Python list indices. With base-index=1, startup_window: 1 picks different windows in each tool. Name-based matching avoids this. what: - Log INFO when numeric fallback is used, suggesting window names - Log WARNING when numeric index is out of range - Same treatment for startup_pane - Add parametrized tests for name match, numeric, out-of-range, no-match
…--here mode why: --here blindly renamed the current session to the config's session_name. If another session already owned that name, tmux errored with a duplicate session name. what: - Check server.sessions for existing name before rename_session - Raise TmuxpException with clear message on conflict - Add parametrized tests: same-name, no-conflict, and conflict cases
…mode why: The --here path only renamed the window, killed extra panes, and sent cd. The normal path provisions window_shell and environment at window creation. A config with environment or window_shell worked normally but not with --here. what: - Export environment variables into active pane via send_keys - Send window_shell command to active pane before shell_command - Extract from first pane config (same precedence as normal path) - Add test verifying environment is accessible in --here mode
…xist yet why: copy called get_workspace_dir() which skips non-existent directories. When TMUXP_CONFIGDIR was set but didn't exist, the file landed in the fallback ~/.tmuxp instead. command_new already had the correct pattern. what: - Check TMUXP_CONFIGDIR directly before falling back to get_workspace_dir - Same pattern as command_new (new.py:101-107) - Add parametrized tests for existing and non-existing configdir
… output The importer preserved on_project_first_start in the output dict, but nothing in the builder or CLI ever reads it — dead data that misleads users into thinking the hook works. The warning log already tells users to use on_project_start instead; stop also copying the value.
…s also present When pre and pre_window were both present and pre_window was a list, the importer passed the array through as-is to shell_command_before. But tmuxinator's parsed_parameters() always joins arrays with "; ". The standalone pre_window path already did this joining correctly; the combo path did not. Now both paths match tmuxinator semantics.
…_project_start) The docs said tmuxinator's pre maps to on_project_start, but the importer code maps pre to before_script in all branches. These have different semantics: before_script runs during session build via run_before_script(), while on_project_start is a lifecycle hook that runs via run_hook_commands().
run_hook_commands only caught TimeoutExpired. A nonexistent cwd raises FileNotFoundError (subclass of OSError), which propagated unhandled and could crash tmuxp stop before session.kill() executes. Now catches OSError and logs a warning, matching tmuxinator's graceful handling.
The no-args guard in cli/__init__.py printed help and returned before command_stop() was called, making the get_session() fallback in stop.py dead code. tmuxinator stop without args stops the current session; tmuxp should too. Removed the guard so command_stop handles resolution.
Without this guard, tmuxp stop with no args outside tmux would connect to the default server and kill server.sessions[0] — the user's first real session. Now requires TMUX env var to be set (proving we're inside tmux) before falling back to current session detection. Outside tmux with no args, shows a clear error message instead.
…w precedence tmuxinator's pre_window method uses an exclusive if/elsif chain: rbenv > rvm > pre_tab > pre_window — only ONE is selected. The tmuxp importer was unconditionally appending rbenv and rvm after pre_window, combining them all. A config with rvm and pre_tab produced both commands in tmuxp but only the rvm command in tmuxinator. Restructured to mirror tmuxinator's exclusive precedence. The pre key (before_script) remains independent since tmuxinator treats it separately from pre_window.
on_project_exit ran via tmux run-shell with no workspace directory. Relative commands behaved differently from on_project_start and on_project_stop which do get cwd. tmuxinator's template runs cd <root> before all hooks. Now prepends cd <start_directory> && to the hook command when start_directory is set, using shlex.quote for safety.
Both flags were accepted silently with --here taking precedence. Now uses argparse mutually exclusive group so passing both produces a clear error. teamocil's --here is standalone; this matches that pattern.
--here outside tmux silently fell through to normal _load_attached with no indication. Now logs a warning and shows a user-facing message before falling back. teamocil's --here also requires being inside tmux.
AST-based test scans all test files for subprocess.run/call invocations that run tmuxp mutation commands (stop, load) without -L test socket. These could kill real tmux sessions when tests run inside tmux (e.g. via just watch-test in the tmuxp dev session). The test would have caught the bug where tmuxp stop no-args killed the dev session.
…fore_script before_script calls run_before_script() which uses shlex.split + Popen(shell=False) — it expects a file path. tmuxinator pre is a raw shell command (e.g., pre: "mysql.server start"). Imported configs with raw commands crashed with BeforeLoadScriptNotExists. on_project_start uses run_hook_commands(shell=True) which handles raw shell commands correctly, matching tmuxinator's template.erb behavior where pre is emitted as a raw shell line.
build(here=True) was setting options, environment, and hooks on the session before checking if the target session name already exists. If rename failed, the user's session was left with stale hooks/options. Now checks for conflicts first, before any session state is modified.
command_stop caught SessionNotFound, printed the error, but returned normally with exit code 0. This broke CI/scripting that relies on non-zero exit codes to detect failures. Now calls sys.exit(1).
--detached silently overrode --here (short-circuited in _dispatch_build before reaching the here check). Now all three load modes (-d, -a/ --append, --here) are in the same argparse mutually exclusive group.
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. Review scope (5 parallel agents):
Prior review rounds: 3 rounds of 3-model (Claude/Gemini/GPT) loom reviews found 16 issues, all fixed in subsequent commits on this branch. 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
Summary
Bring tmuxp to feature parity with tmuxinator and teamocil. This adds the missing CLI commands, config keys, lifecycle hooks, config templating, and importer improvements that users migrating from those tools expect — plus comprehensive documentation including a full feature comparison page.
New CLI commands
tmuxp stop— kill a session with cleanup$ tmuxp stop mysessionRuns the
on_project_stoplifecycle hook before killing the session, giving projects a chance to tear down background services, save state, etc.tmuxp new— create a workspace config$ tmuxp new myprojectCreates a new workspace config from a minimal template and opens it in
$EDITOR.tmuxp copy— copy a workspace config$ tmuxp copy myproject myproject-backupCopies an existing workspace config to a new name. Source is resolved using the same logic as
tmuxp load.tmuxp delete— delete workspace configs$ tmuxp delete old-projectDeletes workspace config files. Prompts for confirmation unless
-yis passed.Lifecycle hooks
Workspace configs now support four hooks, matching tmuxinator's hook system:
on_project_starton_project_restarton_project_exitclient-detachedhook)on_project_stoptmuxp stopkills the sessionConfig templating
Workspace configs now support
{{ variable }}placeholders with values passed via--set:$ tmuxp load --set project=myapp mytemplate.yamlNew config keys
Pane titles
synchronizeshorthandDesugars
synchronize: before→options: {synchronize-panes: on}andsynchronize: after→options_after: {synchronize-panes: on}.trueis equivalent tobefore.shell_command_afterandclearNew
tmuxp loadflags--here--no-shell-command-beforeshell_command_beforeentries--debug--set KEY=VALUEImporter improvements
tmuxinator
pre→on_project_start,pre_window→shell_command_beforecli_args(-f,-S,-L) parsed into tmuxp equivalentssynchronizewindow key convertedstartup_window/startup_pane→focus: trueon the targettitleon the panestr(fixes numeric/emoji YAML keys)teamocil
windowsat top level,commandskey in panes)focus: trueon windows and panes convertedoptionspassed throughBug fixes
on_project_starthook when load actually proceeds (not on cancellation)on_project_restartafter the user confirms reattachDocumentation
docs/comparison.md): Side-by-side of tmuxp vs tmuxinator vs teamocil — architecture, config keys, CLI flags, hooksdocs/configuration/top-level.md): New keys, lifecycle hooks, synchronize, pane titlesdocs/configuration/examples.md): Working examples for each new featureTest plan
uv run py.testpassestmuxp loadwith lifecycle hooks,--here,--debug,--settmuxp stop,tmuxp new,tmuxp copy,tmuxp delete