Repository navigation
feat: add --pane and --no-focus flags to markdown open #2821
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2922,7 +2922,9 @@ struct CMUXCLI { | |||||||||||||||||||||||||
| let (windowOpt, argsAfterWindow) = parseOption(argsAfterWorkspace, name: "--window") | ||||||||||||||||||||||||||
| let (surfaceOpt, argsAfterSurface) = parseOption(argsAfterWindow, name: "--surface") | ||||||||||||||||||||||||||
| let (directionOpt, argsAfterDirection) = parseOption(argsAfterSurface, name: "--direction") | ||||||||||||||||||||||||||
| let (paneOpt, argsAfterPane) = parseOption(argsAfterDirection, name: "--pane") | ||||||||||||||||||||||||||
| let noFocus = argsAfterPane.contains("--no-focus") | ||||||||||||||||||||||||||
| args = argsAfterPane.filter { $0 != "--no-focus" } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // Determine subcommand. Explicit "open" is supported, otherwise treat | ||||||||||||||||||||||||||
| // a single positional argument as shorthand path. | ||||||||||||||||||||||||||
|
|
@@ -2937,7 +2939,7 @@ struct CMUXCLI { | |||||||||||||||||||||||||
| if let first = args.first, first.hasPrefix("-") { | ||||||||||||||||||||||||||
| throw CLIError( | ||||||||||||||||||||||||||
| message: | ||||||||||||||||||||||||||
| "markdown open: unknown flag '\(first)'. Usage: cmux markdown open <path> [--workspace <id|ref|index>] [--surface <id|ref|index>] [--window <id|ref|index>] [--direction right|down|left|up] [--pane <id|ref|index>] [--no-focus]" | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
| } else if let first = args.first, looksLikePath(first) || first.contains(".") { | ||||||||||||||||||||||||||
| subArgs = args | ||||||||||||||||||||||||||
|
|
@@ -2955,13 +2957,13 @@ struct CMUXCLI { | |||||||||||||||||||||||||
| if let unknownFlag = trailingArgs.first(where: { $0.hasPrefix("-") }) { | ||||||||||||||||||||||||||
| throw CLIError( | ||||||||||||||||||||||||||
| message: | ||||||||||||||||||||||||||
| "markdown open: unknown flag '\(unknownFlag)'. Usage: cmux markdown open <path> [--workspace <id|ref|index>] [--surface <id|ref|index>] [--window <id|ref|index>] [--direction right|down|left|up] [--pane <id|ref|index>] [--no-focus]" | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| if let extraArg = trailingArgs.first { | ||||||||||||||||||||||||||
| throw CLIError( | ||||||||||||||||||||||||||
| message: | ||||||||||||||||||||||||||
| "markdown open: unexpected argument '\(extraArg)'. Usage: cmux markdown open <path> [--workspace <id|ref|index>] [--surface <id|ref|index>] [--window <id|ref|index>] [--direction right|down|left|up] [--pane <id|ref|index>] [--no-focus]" | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
@@ -2970,6 +2972,9 @@ struct CMUXCLI { | |||||||||||||||||||||||||
| // Build params | ||||||||||||||||||||||||||
| let direction = directionOpt ?? "right" | ||||||||||||||||||||||||||
| var params: [String: Any] = ["path": absolutePath, "direction": direction] | ||||||||||||||||||||||||||
| if noFocus { | ||||||||||||||||||||||||||
| params["focus"] = false | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| if let surfaceRaw = surfaceOpt { | ||||||||||||||||||||||||||
| if let surface = try normalizeSurfaceHandle(surfaceRaw, client: client) { | ||||||||||||||||||||||||||
| params["surface_id"] = surface | ||||||||||||||||||||||||||
|
|
@@ -2986,6 +2991,11 @@ struct CMUXCLI { | |||||||||||||||||||||||||
| params["window_id"] = window | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| if let paneRaw = paneOpt { | ||||||||||||||||||||||||||
| if let pane = try normalizePaneHandle(paneRaw, client: client, workspaceHandle: workspaceRaw) { | ||||||||||||||||||||||||||
| params["pane_id"] = pane | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
Comment on lines
+2994
to
+2998
There was a problem hiding this comment.
Line 2996 can resolve to Suggested fix- if let paneRaw = paneOpt {
- let wsHandle = workspaceOpt ?? ProcessInfo.processInfo.environment["CMUX_WORKSPACE_ID"]
- if let pane = try normalizePaneHandle(paneRaw, client: client, workspaceHandle: wsHandle) {
- params["pane_id"] = pane
- }
- }
+ if let paneRaw = paneOpt {
+ let wsHandle = params["workspace_id"] as? String
+ if let paneHandle = try normalizePaneHandle(paneRaw, client: client, workspaceHandle: wsHandle) {
+ params["pane_id"] = try resolvePaneUUIDHandle(
+ paneHandle,
+ workspaceHandle: wsHandle,
+ client: client
+ )
+ }
+ }private func resolvePaneUUIDHandle(_ handle: String, workspaceHandle: String?, client: SocketClient) throws -> String {
if isUUID(handle) { return handle }
var params: [String: Any] = [:]
if let workspaceHandle { params["workspace_id"] = workspaceHandle }
let listed = try client.sendV2(method: "pane.list", params: params)
let panes = listed["panes"] as? [[String: Any]] ?? []
if let match = panes.first(where: { ($0["ref"] as? String) == handle || ($0["id"] as? String) == handle }),
let paneId = match["id"] as? String {
return paneId
}
throw CLIError(message: "Pane handle not found: \(handle)")
}🤖 Prompt for AI Agents
Author
There was a problem hiding this comment.
There was a problem hiding this comment.
✏️ Learnings added
🧠 Learnings used
Comment on lines
+2994
to
+2998
Contributor
There was a problem hiding this comment.
The workspace param (line 2983) correctly omits
Suggested change
Author
There was a problem hiding this comment. Good catch — fixed in 18c6b77. Now uses the already-computed |
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| let payload = try client.sendV2(method: "markdown.open", params: params) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
@@ -8094,12 +8104,16 @@ struct CMUXCLI { | |||||||||||||||||||||||||
| --surface <id|ref|index> Source surface to split from (default: focused surface) | ||||||||||||||||||||||||||
| --window <id|ref|index> Target window | ||||||||||||||||||||||||||
| --direction <left|right|up|down> Split direction (default: right) | ||||||||||||||||||||||||||
| --pane <id|ref|index> Add as tab in existing pane instead of splitting | ||||||||||||||||||||||||||
| --no-focus Open without focusing the new panel | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Examples: | ||||||||||||||||||||||||||
| cmux markdown open plan.md | ||||||||||||||||||||||||||
| cmux markdown ~/project/CHANGELOG.md | ||||||||||||||||||||||||||
| cmux markdown open ./docs/design.md --workspace 0 | ||||||||||||||||||||||||||
| cmux markdown open plan.md --direction down | ||||||||||||||||||||||||||
| cmux markdown open plan.md --pane pane:5 | ||||||||||||||||||||||||||
| cmux markdown open plan.md --no-focus | ||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||
| default: | ||||||||||||||||||||||||||
| return nil | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7615,35 +7615,58 @@ class TerminalController { | |
| v2MaybeFocusWindow(for: tabManager) | ||
| v2MaybeSelectWorkspace(tabManager, workspace: ws) | ||
|
|
||
| let focus = v2FocusAllowed(requested: v2Bool(params, "focus") ?? true) | ||
|
Contributor
There was a problem hiding this comment.
This matches the socket focus policy (non-focus-intent commands don't mutate in-app focus), but it means
Author
There was a problem hiding this comment.
|
||
| let requestedPaneUUID = v2UUID(params, "pane_id") | ||
|
There was a problem hiding this comment. P2: Malformed Prompt for AI agents
Author
There was a problem hiding this comment. The silent fallback is intentional and matches how |
||
|
|
||
| let markdownPanelId: UUID? | ||
| let sourceSurfaceId: UUID? | ||
| let sourcePaneUUID: UUID? | ||
|
|
||
| if let requestedPaneUUID { | ||
| // --pane mode: add as tab in existing pane | ||
| guard let paneId = ws.bonsplitController.allPaneIds.first(where: { $0.id == requestedPaneUUID }) else { | ||
| result = .err(code: "not_found", message: "Pane not found", data: ["pane_id": requestedPaneUUID.uuidString]) | ||
| return | ||
| } | ||
| sourceSurfaceId = nil | ||
| sourcePaneUUID = paneId.id | ||
| markdownPanelId = ws.newMarkdownSurface( | ||
| inPane: paneId, | ||
| filePath: filePath, | ||
| focus: focus | ||
| )?.id | ||
| } else { | ||
| // Default: split from source surface | ||
| let resolvedSourceId = v2UUID(params, "surface_id") ?? ws.focusedPanelId | ||
|
Comment on lines
+7619
to
+7640
There was a problem hiding this comment.
Line 7619 only accepts a raw UUID, but this command now advertises Based on learnings: if a parameter key is present but invalid, the API should return 🤖 Prompt for AI Agents
Author
There was a problem hiding this comment. Same as above — There was a problem hiding this comment.
One small edge case remains: if pane_id is present but unresolvable (blank string, unknown ref), the code currently falls back to split mode. For consistency with other v2 endpoints that treat “present but invalid” as invalid_params, we can add a tiny guard before the if-let: diff --git a/Sources/TerminalController.swift b/Sources/TerminalController.swift
@@ func v2MarkdownOpen(params: [String: Any]) -> V2CallResult {
- let focus = v2FocusAllowed(requested: v2Bool(params, "focus") ?? true)
- let requestedPaneUUID = v2UUID(params, "pane_id")
+ let focus = v2FocusAllowed(requested: v2Bool(params, "focus") ?? true)
+ let requestedPaneUUID = v2UUID(params, "pane_id")
+ let hasPaneParam = v2HasNonNullParam(params, "pane_id")
+ let rawPaneParam = v2RawString(params, "pane_id")
+
+ if hasPaneParam && requestedPaneUUID == nil {
+ result = .err(
+ code: "invalid_params",
+ message: "Invalid pane_id",
+ data: ["pane_id": v2OrNull(rawPaneParam)]
+ )
+ return
+ }
let markdownPanelId: UUID?
let sourceSurfaceId: UUID?
let sourcePaneUUID: UUID?
if let requestedPaneUUID {
// --pane mode: add as tab in existing paneEffect:
If you’d like, I can open a tiny follow-up with this defensive check. ✏️ Learnings added
🧠 Learnings used |
||
| guard let resolvedSourceId else { | ||
| result = .err(code: "not_found", message: "No focused surface to split", data: nil) | ||
| return | ||
| } | ||
| guard ws.panels[resolvedSourceId] != nil else { | ||
| result = .err(code: "not_found", message: "Source surface not found", data: ["surface_id": resolvedSourceId.uuidString]) | ||
| return | ||
| } | ||
| sourceSurfaceId = resolvedSourceId | ||
| sourcePaneUUID = ws.paneId(forPanelId: resolvedSourceId)?.id | ||
|
|
||
| let directionStr = v2String(params, "direction") ?? "right" | ||
| guard let direction = parseSplitDirection(directionStr) else { | ||
| result = .err(code: "invalid_params", message: "Invalid direction '\(directionStr)' (left|right|up|down)", data: nil) | ||
| return | ||
| } | ||
| let orientation: SplitOrientation = direction.isHorizontal ? .horizontal : .vertical | ||
| let insertFirst = (direction == .left || direction == .up) | ||
|
|
||
| markdownPanelId = ws.newMarkdownSplit( | ||
| from: resolvedSourceId, | ||
| orientation: orientation, | ||
| insertFirst: insertFirst, | ||
| filePath: filePath, | ||
| focus: focus | ||
| )?.id | ||
| } | ||
|
|
||
| guard let markdownPanelId else { | ||
| result = .err(code: "internal_error", message: "Failed to create markdown panel", data: nil) | ||
| return | ||
| } | ||
|
|
@@ -7659,7 +7682,7 @@ class TerminalController { | |
| "pane_ref": v2Ref(kind: .pane, uuid: targetPaneUUID), | ||
| "surface_id": markdownPanelId.uuidString, | ||
| "surface_ref": v2Ref(kind: .surface, uuid: markdownPanelId), | ||
| "source_surface_id": v2OrNull(sourceSurfaceId?.uuidString), | ||
| "source_surface_ref": v2Ref(kind: .surface, uuid: sourceSurfaceId), | ||
| "source_pane_id": v2OrNull(sourcePaneUUID?.uuidString), | ||
| "source_pane_ref": v2Ref(kind: .pane, uuid: sourcePaneUUID), | ||
|
|
||
Oops, something went wrong.
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.
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.
P2: Pane index resolution may target the wrong pane when
--windowis used without--workspace, because workspace scoping is dropped beforenormalizePaneHandle.Prompt for AI agents