Skip to content

feat(tmux): make exec pane split-window args configurable#161

Open
karto wants to merge 2 commits intoalvinunreal:mainfrom
karto:main
Open

feat(tmux): make exec pane split-window args configurable#161
karto wants to merge 2 commits intoalvinunreal:mainfrom
karto:main

Conversation

@karto
Copy link

@karto karto commented Mar 6, 2026

I got tired of the chat window always being half the screen, so i extracted the exec split-window args into the config file. Here are the results. Use them if you like.

@alvinunreal
Copy link
Owner

@greptile-app

@greptile-apps
Copy link

greptile-apps bot commented Mar 7, 2026

Greptile Summary

This PR makes the tmux split-window arguments used when creating the exec pane fully configurable via a new tmux.exec_split_args config key, replacing the previously hardcoded -d -h flags. The change is backward-compatible (default behavior is preserved), well-documented, and includes unit tests for the new buildSplitWindowArgs helper.

Key changes:

  • New TmuxConfig struct added to config/config.go with ExecSplitArgs []string; default ["-d", "-h"] set in DefaultConfig()
  • system.TmuxCreateNewPane signature updated to accept splitArgs []string; a buildSplitWindowArgs helper assembles the final argument list
  • internal/exec_pane.go forwards m.Config.Tmux.ExecSplitArgs to the updated function
  • Three unit tests cover the nil-args fallback, configured args, and empty-slice fallback
  • config.example.yaml and README.md updated with clear usage examples

Two concerns worth addressing in system/tmux.go:

  1. The empty-slice fallback in buildSplitWindowArgs duplicates the default already set in DefaultConfig(), creating two independent sources of truth for the same default value
  2. No validation prevents users from including reserved flags (-t, -P, -F) in exec_split_args; a misconfigured entry (e.g. ["-F"] without a following value) would cause the -t <target> argument to be consumed as the format string, silently breaking pane-ID retrieval since the error from TmuxCreateNewPane is discarded in InitExecPane

Confidence Score: 3/5

  • Safe to merge for typical usage, but misconfigured split args can silently break exec-pane creation with no user-visible error.
  • The implementation is clean and backward-compatible; default behavior is unchanged and tests pass the happy paths. The score is reduced because (1) the error from TmuxCreateNewPane is silently discarded in InitExecPane, so an invalid exec_split_args value will fail without any feedback to the user, and (2) duplicate default definitions in DefaultConfig and buildSplitWindowArgs create a maintenance hazard.
  • system/tmux.go — the buildSplitWindowArgs helper has a duplicate-default issue and no guard against reserved flag conflicts.

Important Files Changed

Filename Overview
system/tmux.go Refactored TmuxCreateNewPane to accept user-supplied split args; new buildSplitWindowArgs helper has a duplicate-default concern and no guard against conflicting reserved flags (-t, -P, -F).
config/config.go Added TmuxConfig struct and wired it into the top-level Config; default ["-d", "-h"] set in DefaultConfig(). Clean and idiomatic.
internal/exec_pane.go One-line change to forward m.Config.Tmux.ExecSplitArgs to TmuxCreateNewPane; straightforward and correct.
system/tmux_test.go New test file covering nil-args fallback, configured args pass-through, and empty-slice fallback for buildSplitWindowArgs. Good coverage of the happy paths.
config.example.yaml Added tmux.exec_split_args example with helpful comments and multiple usage examples. No issues.
README.md New documentation section accurately describes the option and its injection point. No issues.

Sequence Diagram

sequenceDiagram
    participant U as User Config (YAML)
    participant C as config.DefaultConfig()
    participant M as Manager.InitExecPane()
    participant T as system.TmuxCreateNewPane()
    participant B as buildSplitWindowArgs()
    participant TM as tmux process

    U-->>C: exec_split_args (or omitted)
    C->>M: Config.Tmux.ExecSplitArgs (default: ["-d","-h"])
    M->>T: TmuxCreateNewPane(paneId, splitArgs)
    T->>B: buildSplitWindowArgs(target, splitArgs)
    B-->>B: fallback to ["-d","-h"] if len==0
    B-->>T: ["split-window", ...splitArgs, "-t", target, "-P", "-F", "#{pane_id}"]
    T->>TM: exec.Command("tmux", args...)
    TM-->>T: new pane ID on stdout
    T-->>M: paneId (or error, silently discarded)
Loading

Last reviewed commit: 806eb42

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
@alvinunreal
Copy link
Owner

Looks good, just small issues reported by greptile

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.

3 participants