-
Notifications
You must be signed in to change notification settings - Fork 944
Handle OMX extended-keys tmux probes in cmux compat layer #2675
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 2 commits
3096a65
758c722
7637ca6
e0a1a27
15cd50e
912beac
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2694,7 +2694,13 @@ struct CMUXCLI { | |
| "bind-key", | ||
| "unbind-key", | ||
| "copy-mode", | ||
| "show-option", | ||
| "show-options", | ||
| "set", | ||
| "set-option", | ||
| "set-window-option", | ||
| "set-buffer", | ||
| "setw", | ||
| "paste-buffer", | ||
| "list-buffers", | ||
| "respawn-pane", | ||
|
|
@@ -11626,7 +11632,7 @@ struct CMUXCLI { | |
| print(buffer) | ||
| } | ||
|
|
||
| case "last-window", "next-window", "previous-window", "set-hook", "set-buffer", "list-buffers": | ||
| case "last-window", "next-window", "previous-window", "set-hook", "show-option", "show-options", "set", "set-option", "set-window-option", "set-buffer", "list-buffers", "setw": | ||
| try runTmuxCompatCommand( | ||
| command: command, | ||
| commandArgs: rawArgs, | ||
|
|
@@ -11689,7 +11695,7 @@ struct CMUXCLI { | |
| try tmuxPruneCompatWorkspaceState(workspaceId: workspaceId) | ||
| } | ||
|
|
||
| case "set-option", "set", "set-window-option", "setw", "source-file", "refresh-client", "attach-session", "detach-client": | ||
| case "source-file", "refresh-client", "attach-session", "detach-client": | ||
| return | ||
|
|
||
| case "-V", "-v": | ||
|
|
@@ -11712,6 +11718,7 @@ struct CMUXCLI { | |
| private struct TmuxCompatStore: Codable { | ||
| var buffers: [String: String] = [:] | ||
| var hooks: [String: String] = [:] | ||
| var options: [String: String] = [:] | ||
| /// Tracks main-vertical layout state per workspace, keyed by workspace ID. | ||
| var mainVerticalLayouts: [String: MainVerticalState] = [:] | ||
| /// Tracks the last surface created by split-window per workspace. | ||
|
|
@@ -11720,19 +11727,36 @@ struct CMUXCLI { | |
| var lastSplitSurface: [String: String] = [:] | ||
|
|
||
| /// Custom decoder so older store files missing newer keys | ||
| /// (mainVerticalLayouts, lastSplitSurface) decode gracefully | ||
| /// (options, mainVerticalLayouts, lastSplitSurface) decode gracefully | ||
| /// instead of throwing and resetting the entire store. | ||
| init(from decoder: Decoder) throws { | ||
| let container = try decoder.container(keyedBy: CodingKeys.self) | ||
| buffers = try container.decodeIfPresent([String: String].self, forKey: .buffers) ?? [:] | ||
| hooks = try container.decodeIfPresent([String: String].self, forKey: .hooks) ?? [:] | ||
| options = try container.decodeIfPresent([String: String].self, forKey: .options) ?? [:] | ||
| mainVerticalLayouts = try container.decodeIfPresent([String: MainVerticalState].self, forKey: .mainVerticalLayouts) ?? [:] | ||
| lastSplitSurface = try container.decodeIfPresent([String: String].self, forKey: .lastSplitSurface) ?? [:] | ||
| } | ||
|
|
||
| init() {} | ||
| } | ||
|
|
||
| private func tmuxCompatDefaultOptionValue(_ name: String) -> String? { | ||
| switch name.lowercased() { | ||
| case "extended-keys": | ||
| // cmux is not a tmux server, but OMX expects the tmux compatibility | ||
| // shim to answer lease-management probes for extended-keys. | ||
| return "off" | ||
| default: | ||
| return nil | ||
| } | ||
| } | ||
|
|
||
| private func tmuxCompatOptionValue(_ name: String, store: TmuxCompatStore) -> String? { | ||
| let normalizedName = name.lowercased() | ||
| return store.options[normalizedName] ?? tmuxCompatDefaultOptionValue(normalizedName) | ||
| } | ||
|
|
||
| private func tmuxCompatStoreURL() -> URL { | ||
| let homePath = ProcessInfo.processInfo.environment["HOME"] | ||
| ?? NSString(string: "~").expandingTildeInPath | ||
|
|
@@ -12204,6 +12228,61 @@ struct CMUXCLI { | |
| try saveTmuxCompatStore(store) | ||
| print("OK") | ||
|
|
||
| case "show-option", "show-options": | ||
| let parsed = try parseTmuxArguments( | ||
| commandArgs, | ||
| valueFlags: [], | ||
| boolFlags: ["-g", "-p", "-q", "-s", "-v", "-w"] | ||
| ) | ||
| let store = loadTmuxCompatStore() | ||
| guard let optionName = parsed.positional.first? | ||
| .trimmingCharacters(in: .whitespacesAndNewlines), | ||
| !optionName.isEmpty else { | ||
| throw CLIError(message: "\(command) requires an option name") | ||
|
||
| } | ||
|
|
||
| guard let value = tmuxCompatOptionValue(optionName, store: store) else { | ||
| if parsed.hasFlag("-q") { | ||
| return | ||
| } | ||
| throw CLIError(message: "Unsupported tmux compatibility option: \(optionName)") | ||
| } | ||
|
|
||
| if parsed.hasFlag("-v") { | ||
| print(value) | ||
| } else { | ||
| print("\(optionName) \(value)") | ||
| } | ||
|
|
||
| case "set-option", "set", "set-window-option", "setw": | ||
| let parsed = try parseTmuxArguments( | ||
| commandArgs, | ||
| valueFlags: [], | ||
| boolFlags: ["-F", "-a", "-g", "-o", "-p", "-q", "-s", "-u", "-w"] | ||
| ) | ||
| guard let optionName = parsed.positional.first? | ||
| .trimmingCharacters(in: .whitespacesAndNewlines), | ||
| !optionName.isEmpty else { | ||
| throw CLIError(message: "\(command) requires an option name") | ||
| } | ||
|
|
||
| var store = loadTmuxCompatStore() | ||
| let normalizedName = optionName.lowercased() | ||
| if parsed.hasFlag("-u") { | ||
| store.options.removeValue(forKey: normalizedName) | ||
| try saveTmuxCompatStore(store) | ||
| return | ||
| } | ||
|
|
||
| let value = parsed.positional.dropFirst().joined(separator: " ") | ||
| .trimmingCharacters(in: .whitespacesAndNewlines) | ||
| guard !value.isEmpty else { | ||
| throw CLIError(message: "\(command) requires an option value") | ||
| } | ||
coderabbitai[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| store.options[normalizedName] = value | ||
| try saveTmuxCompatStore(store) | ||
|
|
||
|
Comment on lines
+12308
to
+12345
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Implement parsed Line 12261 parses 🔧 Suggested patch case "set-option", "set", "set-window-option", "setw":
let parsed = try parseTmuxArguments(
commandArgs,
valueFlags: [],
boolFlags: ["-F", "-a", "-g", "-o", "-p", "-q", "-s", "-u", "-w"]
)
@@
if parsed.hasFlag("-u") {
store.options.removeValue(forKey: normalizedName)
try saveTmuxCompatStore(store)
return
}
@@
- store.options[normalizedName] = value
+ if parsed.hasFlag("-o"), store.options[normalizedName] != nil {
+ return
+ }
+
+ if parsed.hasFlag("-a"), let existing = store.options[normalizedName] {
+ store.options[normalizedName] = existing + value
+ } else {
+ store.options[normalizedName] = value
+ }
try saveTmuxCompatStore(store)🤖 Prompt for AI Agents |
||
| case "popup": | ||
| throw CLIError(message: "popup is not supported yet in cmux CLI parity mode") | ||
|
|
||
|
|
@@ -14393,6 +14472,8 @@ struct CMUXCLI { | |
| last-pane [--workspace <id|ref>] | ||
| find-window [--content] [--select] <query> | ||
| clear-history [--workspace <id|ref>] [--surface <id|ref>] | ||
| show-options [-sv] <name> | ||
| set-option [-sq] <name> <value> | ||
| set-hook [--list] [--unset <event>] | <event> <command> | ||
| popup | ||
| bind-key | unbind-key | copy-mode | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -157,6 +157,13 @@ def main() -> int: | |||||
| c.send_surface(s1, f"echo {capture_token}\n") | ||||||
| _wait_for(lambda: _surface_has(c, ws, s1, capture_token)) | ||||||
|
|
||||||
| show_extended_keys = _run_cli(cli, ["show-options", "-sv", "extended-keys"]) | ||||||
| _must(show_extended_keys.stdout.strip() == "off", f"show-options should default extended-keys to off, got {show_extended_keys.stdout!r}") | ||||||
| _run_cli(cli, ["set-option", "-sq", "extended-keys", "always"]) | ||||||
| show_extended_keys = _run_cli(cli, ["show-options", "-sv", "extended-keys"]) | ||||||
| _must(show_extended_keys.stdout.strip() == "always", f"set-option should persist extended-keys, got {show_extended_keys.stdout!r}") | ||||||
| _run_cli(cli, ["set-option", "-sq", "extended-keys", "off"]) | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The teardown sets
Suggested change
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. current OMX restores extended-keys with set-option -sq ..., not -u |
||||||
|
|
||||||
| cap = _run_cli(cli, ["capture-pane", "--workspace", ws, "--surface", s1, "--scrollback"]) | ||||||
| _must(capture_token in cap.stdout, f"capture-pane missing token: {cap.stdout!r}") | ||||||
|
|
||||||
|
|
||||||
Uh oh!
There was an error while loading. Please reload this page.