Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

feat: opt-in tmux config support + enable extended-keys by default#32

Closed
bigph00t wants to merge 1 commit into
main from
feat/tmux-config-extended-keys
Closed

feat: opt-in tmux config support + enable extended-keys by default #32
bigph00t wants to merge 1 commit into
main from
feat/tmux-config-extended-keys

Conversation

@bigph00t

@bigph00t bigph00t commented May 12, 2026
edited
Loading

Copy link
Copy Markdown
Contributor

Problem

mobilecli spawns tmux with -f /dev/null (or NUL on Windows), which completely ignores the user's ~/.tmux.conf. This causes two pain points:

  1. Warnings in modern terminals — applications using the kitty keyboard protocol or other extended key sequences emit "extended-keys is off" warnings because tmux hasn't been told to enable them.
  2. No escape hatch for power users — users who have customized tmux configs (colors, key bindings, etc.) get a barebones tmux environment inside mobilecli sessions.

Solution

This PR takes the "careful best of both worlds" approach:

1. Surgical fix: enable extended-keys by default

setup_tmux_session() now explicitly runs set-option -g extended-keys on after setting up the session. This silences warnings without giving up control over tmux behavior.

2. Opt-in custom tmux config

Instead of broadly loading ~/.tmux.conf (which would break mobilecli's streaming assumptions), we introduce a two-tier lookup:

Priority Source
1 MOBILECLI_TMUX_CONFIG=/path/to/config env var
2 ~/.mobilecli/tmux.conf (if the file exists)
3 Null device (/dev/null or NUL) — safe default

When a custom config is provided, mobilecli still loads it, then immediately enforces its own required options:

  • history-limit 200000
  • mouse (per platform policy)
  • window-size latest
  • status off, allow-rename off
  • extended-keys on

This means user configs only affect things mobilecli doesn't care about (colors, key bindings, etc.), while streaming reliability is preserved.

3. Cross-platform cleanup

  • Extracted tmux_null_device() helper to centralize the Windows NUL vs Unix /dev/null logic.
  • Both the std::process::Command setup path and the portable_pty::CommandBuilder attach path now use the same config resolution.

4. Safety hardening

  • resolve_tmux_config_path() uses path.is_file() instead of path.exists() to reject directories (defensive against misconfigured env vars).
  • Refactored tracing::info! block to use if let instead of is_some() + unwrap() (clippy clean).

Testing

  • cargo test pty_wrapper passes (10/10)
  • cargo clippy passes cleanly
  • Added new test: tmux_session_enables_extended_keys_by_default
  • Updated existing tests to pass explicit None for config path

Documentation

  • Added tmux.conf to the ~/.mobilecli/ file table in README
  • Documented MOBILECLI_TMUX_CONFIG, enforced options, and warnings about dangerous tmux settings (destroy-unattached, remain-on-exit)
  • Documented that daemon-spawned headless sessions use the default tmux server (different from pty-wrapper attach sessions)

Why not just load ~/.tmux.conf automatically?

User tmux configs can override behavior mobilecli depends on:

  • mouse on breaking desktop clipboard selection
  • alternate-screen settings interfering with scrollback capture
  • status on wasting vertical space on mobile screens
  • Custom destroy-unattached or remain-on-exit breaking session lifecycle

The opt-in model keeps the safe default for 99% of users while giving power users an escape hatch.

Known limitations

  • Daemon-spawned headless sessions (phone-only, no desktop terminal) currently use the default tmux server and load ~/.tmux.conf, not ~/.mobilecli/tmux.conf. This is intentional because headless sessions don't need the isolated control that attach-mode sessions require. If there's demand, we can extend custom config support to headless sessions in a follow-up.
  • On Windows, tmux is typically installed via MSYS2/Git Bash. Backslash path separators in MOBILECLI_TMUX_CONFIG are passed through as-is; most Windows tmux builds accept forward slashes, so users with custom configs may want to use those.

Closes the "extended-keys is off" warning issue and provides a path for tmux customization without compromising streaming reliability.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

Problem:
- mobilecli spawns tmux with -f /dev/null, which ignores ~/.tmux.conf
- This causes 'extended-keys is off' warnings in modern terminals/TUIs
- Power users have no escape hatch to customize their tmux environment
Solution:
- Explicitly set 'extended-keys on' during session setup (surgical fix)
- Add opt-in custom tmux config resolution:
 1. MOBILECLI_TMUX_CONFIG env var (highest priority)
 2. ~/.mobilecli/tmux.conf file fallback
 3. Null device (/dev/null or NUL) as safe default
- MobileCLI still enforces its required options (history-limit, mouse,
 window-size, etc.) after loading the custom config, preserving reliable
 streaming behavior while allowing colors, key bindings, and other
 user preferences to come through.
Cross-platform:
- Extracts tmux_null_device() helper to avoid duplicating the Windows/NUL
 vs Unix /dev/null logic
- Paths are passed through to_string_lossy() for tmux compatibility
Safety fixes:
- Use path.is_file() instead of path.exists() to reject directories
- Refactor unnecessary unwrap to if-let (clippy::unnecessary_unwrap)
- Allow clippy::too_many_arguments on setup_tmux_session (pre-existing +1)
Tests:
- Add tmux_session_enables_extended_keys_by_default test
- Update all existing tests to pass explicit None for config path
Docs:
- Add tmux.conf to ~/.mobilecli/ file table in README
- Document MOBILECLI_TMUX_CONFIG, enforced options, and warnings about
 dangerous tmux settings (destroy-unattached, remain-on-exit)
- Document headless daemon session behavior difference
@bigph00t bigph00t force-pushed the feat/tmux-config-extended-keys branch from bfa3a7e to 8c83c96 Compare May 12, 2026 19:16

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

Copy link
Copy Markdown
Contributor Author

Rolling back — caused terminal positioning regressions in practice. The extended-keys warning is cosmetic and not worth the disruption. Will revisit if a safer approach emerges.

@bigph00t bigph00t deleted the feat/tmux-config-extended-keys branch May 12, 2026 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@greptile-apps greptile-apps[bot] greptile-apps[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

AltStyle によって変換されたページ (->オリジナル) /