-
Notifications
You must be signed in to change notification settings - Fork 7
feat: opt-in tmux config support + enable extended-keys by default#32
Closed
bigph00t wants to merge 1 commit into
Closed
feat: opt-in tmux config support + enable extended-keys by default #32bigph00t wants to merge 1 commit into
bigph00t wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
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
May 12, 2026 19:16
bfa3a7e to
8c83c96
Compare
There was a problem hiding this comment.
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.
bigph00t
commented
May 12, 2026
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.
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
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.
Uh oh!
There was an error while loading. Please reload this page.
Problem
mobilecli spawns tmux with
-f /dev/null(orNULon Windows), which completely ignores the user's~/.tmux.conf. This causes two pain points:Solution
This PR takes the "careful best of both worlds" approach:
1. Surgical fix: enable extended-keys by default
setup_tmux_session()now explicitly runsset-option -g extended-keys onafter 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:MOBILECLI_TMUX_CONFIG=/path/to/configenv var~/.mobilecli/tmux.conf(if the file exists)/dev/nullorNUL) — safe defaultWhen a custom config is provided, mobilecli still loads it, then immediately enforces its own required options:
history-limit 200000mouse(per platform policy)window-size lateststatus off,allow-rename offextended-keys onThis means user configs only affect things mobilecli doesn't care about (colors, key bindings, etc.), while streaming reliability is preserved.
3. Cross-platform cleanup
tmux_null_device()helper to centralize the WindowsNULvs Unix/dev/nulllogic.std::process::Commandsetup path and theportable_pty::CommandBuilderattach path now use the same config resolution.4. Safety hardening
resolve_tmux_config_path()usespath.is_file()instead ofpath.exists()to reject directories (defensive against misconfigured env vars).tracing::info!block to useif letinstead ofis_some() + unwrap()(clippy clean).Testing
cargo test pty_wrapperpasses (10/10)cargo clippypasses cleanlytmux_session_enables_extended_keys_by_defaultNonefor config pathDocumentation
tmux.confto the~/.mobilecli/file table in READMEMOBILECLI_TMUX_CONFIG, enforced options, and warnings about dangerous tmux settings (destroy-unattached,remain-on-exit)Why not just load ~/.tmux.conf automatically?
User tmux configs can override behavior mobilecli depends on:
mouse onbreaking desktop clipboard selectionalternate-screensettings interfering with scrollback capturestatus onwasting vertical space on mobile screensdestroy-unattachedorremain-on-exitbreaking session lifecycleThe opt-in model keeps the safe default for 99% of users while giving power users an escape hatch.
Known limitations
~/.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.MOBILECLI_TMUX_CONFIGare 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.