Repository navigation
Add file tree sidebar with cmd-click path composition #728
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
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
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 | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -1225,6 +1225,10 @@ struct ContentView: View { | |||||||||||||||||||||||||
| @EnvironmentObject var notificationStore: TerminalNotificationStore | ||||||||||||||||||||||||||
| @EnvironmentObject var sidebarState: SidebarState | ||||||||||||||||||||||||||
| @EnvironmentObject var sidebarSelectionState: SidebarSelectionState | ||||||||||||||||||||||||||
| @EnvironmentObject var sidebarContentModeState: SidebarContentModeState | ||||||||||||||||||||||||||
| @StateObject private var fileTreeModel = FileTreeModel() | ||||||||||||||||||||||||||
| @AppStorage("sidebarFileTreeLayout") private var fileTreeLayout: SidebarFileTreeLayout = .toggle | ||||||||||||||||||||||||||
| @State private var splitDividerRatio: CGFloat = 0.4 | ||||||||||||||||||||||||||
| @State private var sidebarWidth: CGFloat = 200 | ||||||||||||||||||||||||||
| @State private var hoveredResizerHandles: Set<SidebarResizerHandle> = [] | ||||||||||||||||||||||||||
| @State private var isResizerDragging = false | ||||||||||||||||||||||||||
|
|
@@ -1773,14 +1777,149 @@ struct ContentView: View { | |||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| private var tabsSidebarContent: some View { | ||||||||||||||||||||||||||
| VerticalTabsSidebar( | ||||||||||||||||||||||||||
| updateViewModel: updateViewModel, | ||||||||||||||||||||||||||
| selection: $sidebarSelectionState.selection, | ||||||||||||||||||||||||||
| selectedTabIds: $selectedTabIds, | ||||||||||||||||||||||||||
| lastSidebarSelectionIndex: $lastSidebarSelectionIndex | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| private var fileTreeSidebarContent: some View { | ||||||||||||||||||||||||||
| Group { | ||||||||||||||||||||||||||
| if let workspace = tabManager.selectedTab { | ||||||||||||||||||||||||||
| FileTreeSidebar( | ||||||||||||||||||||||||||
| model: fileTreeModel, | ||||||||||||||||||||||||||
| workspace: workspace, | ||||||||||||||||||||||||||
| onComposePath: { path in | ||||||||||||||||||||||||||
| sendPathToFocusedTerminal(path) | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||
| Text("No workspace") | ||||||||||||||||||||||||||
| .font(.system(size: 12)) | ||||||||||||||||||||||||||
| .foregroundColor(.secondary) | ||||||||||||||||||||||||||
| .frame(maxWidth: .infinity, maxHeight: .infinity) | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| private var sidebarModeHeader: some View { | ||||||||||||||||||||||||||
| HStack(spacing: 4) { | ||||||||||||||||||||||||||
| Button { | ||||||||||||||||||||||||||
| sidebarContentModeState.mode = .tabs | ||||||||||||||||||||||||||
| } label: { | ||||||||||||||||||||||||||
| Image(systemName: "sidebar.left") | ||||||||||||||||||||||||||
| .font(.system(size: 12)) | ||||||||||||||||||||||||||
| .foregroundColor(sidebarContentModeState.mode == .tabs ? .accentColor : .secondary) | ||||||||||||||||||||||||||
| .frame(width: 24, height: 24) | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| .buttonStyle(.plain) | ||||||||||||||||||||||||||
| .help("Show tabs") | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Button { | ||||||||||||||||||||||||||
| sidebarContentModeState.mode = .fileTree | ||||||||||||||||||||||||||
| } label: { | ||||||||||||||||||||||||||
| Image(systemName: "folder") | ||||||||||||||||||||||||||
| .font(.system(size: 12)) | ||||||||||||||||||||||||||
| .foregroundColor(sidebarContentModeState.mode == .fileTree ? .accentColor : .secondary) | ||||||||||||||||||||||||||
| .frame(width: 24, height: 24) | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| .buttonStyle(.plain) | ||||||||||||||||||||||||||
| .help("Show file tree") | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Spacer() | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| .padding(.horizontal, 8) | ||||||||||||||||||||||||||
| .padding(.top, 30) | ||||||||||||||||||||||||||
| .padding(.bottom, 4) | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| private var sidebarView: some View { | ||||||||||||||||||||||||||
| VStack(spacing: 0) { | ||||||||||||||||||||||||||
| if fileTreeLayout == .split { | ||||||||||||||||||||||||||
| sidebarSplitContent | ||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||
| sidebarToggleContent | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| .frame(width: sidebarWidth) | ||||||||||||||||||||||||||
| .background(SidebarBackdrop().ignoresSafeArea()) | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| private var sidebarToggleContent: some View { | ||||||||||||||||||||||||||
| VStack(spacing: 0) { | ||||||||||||||||||||||||||
| sidebarModeHeader | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| if sidebarContentModeState.mode == .tabs { | ||||||||||||||||||||||||||
| tabsSidebarContent | ||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||
| fileTreeSidebarContent | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| private var sidebarSplitContent: some View { | ||||||||||||||||||||||||||
| GeometryReader { geo in | ||||||||||||||||||||||||||
| let totalHeight = geo.size.height - 30 // account for header | ||||||||||||||||||||||||||
| let tabsHeight = totalHeight * splitDividerRatio | ||||||||||||||||||||||||||
| let fileTreeHeight = totalHeight * (1 - splitDividerRatio) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| VStack(spacing: 0) { | ||||||||||||||||||||||||||
| sidebarModeHeader | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| tabsSidebarContent | ||||||||||||||||||||||||||
| .frame(height: tabsHeight) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // Draggable divider | ||||||||||||||||||||||||||
| Rectangle() | ||||||||||||||||||||||||||
| .fill(Color.primary.opacity(0.1)) | ||||||||||||||||||||||||||
| .frame(height: 1) | ||||||||||||||||||||||||||
| .overlay( | ||||||||||||||||||||||||||
| Rectangle() | ||||||||||||||||||||||||||
| .fill(Color.clear) | ||||||||||||||||||||||||||
| .frame(height: 8) | ||||||||||||||||||||||||||
| .contentShape(Rectangle()) | ||||||||||||||||||||||||||
| .gesture( | ||||||||||||||||||||||||||
| DragGesture() | ||||||||||||||||||||||||||
| .onChanged { value in | ||||||||||||||||||||||||||
| let newRatio = (tabsHeight + value.translation.height) / totalHeight | ||||||||||||||||||||||||||
| splitDividerRatio = min(max(newRatio, 0.15), 0.85) | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
Comment on lines
+1865
to
+1889
There was a problem hiding this comment. Fix split-divider math to avoid drift and unstable sizing. Line 1865 uses a hard-coded height offset, and Line 1887 compounds drag translation by adding it to a value already derived from the current ratio. This causes divider drift/jumps during drag and can produce unstable behavior on small heights. 馃挕 Proposed fix+ `@State` private var splitDragStartRatio: CGFloat?
+
private var sidebarSplitContent: some View {
- GeometryReader { geo in
- let totalHeight = geo.size.height - 30 // account for header
- let tabsHeight = totalHeight * splitDividerRatio
- let fileTreeHeight = totalHeight * (1 - splitDividerRatio)
-
- VStack(spacing: 0) {
- sidebarModeHeader
+ VStack(spacing: 0) {
+ sidebarModeHeader
+ GeometryReader { geo in
+ let totalHeight = max(geo.size.height, 1)
+ let tabsHeight = totalHeight * splitDividerRatio
+ let fileTreeHeight = totalHeight * (1 - splitDividerRatio)
tabsSidebarContent
.frame(height: tabsHeight)
@@
.gesture(
DragGesture()
.onChanged { value in
- let newRatio = (tabsHeight + value.translation.height) / totalHeight
+ if splitDragStartRatio == nil {
+ splitDragStartRatio = splitDividerRatio
+ }
+ let baseRatio = splitDragStartRatio ?? splitDividerRatio
+ let newRatio = baseRatio + (value.translation.height / totalHeight)
splitDividerRatio = min(max(newRatio, 0.15), 0.85)
}
+ .onEnded { _ in
+ splitDragStartRatio = nil
+ }
)馃 Prompt for AI Agents |
||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
| .onHover { hovering in | ||||||||||||||||||||||||||
| if hovering { | ||||||||||||||||||||||||||
| NSCursor.resizeUpDown.push() | ||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||
| NSCursor.pop() | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| fileTreeSidebarContent | ||||||||||||||||||||||||||
| .frame(height: fileTreeHeight) | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| private func sendPathToFocusedTerminal(_ path: String) { | ||||||||||||||||||||||||||
| guard let workspace = tabManager.selectedTab, | ||||||||||||||||||||||||||
| let terminalPanel = workspace.focusedTerminalPanel else { return } | ||||||||||||||||||||||||||
| // Shell-escape the path so spaces and special chars don't break commands | ||||||||||||||||||||||||||
| let escaped = shellEscapePath(path) | ||||||||||||||||||||||||||
| terminalPanel.sendText(escaped) | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| private func shellEscapePath(_ path: String) -> String { | ||||||||||||||||||||||||||
| let needsQuoting = path.contains(" ") || path.contains("'") || path.contains("\"") | ||||||||||||||||||||||||||
| || path.contains("(") || path.contains(")") || path.contains("&") | ||||||||||||||||||||||||||
| || path.contains("|") || path.contains(";") || path.contains("$") | ||||||||||||||||||||||||||
| || path.contains("`") || path.contains("!") | ||||||||||||||||||||||||||
|
Comment on lines
+1914
to
+1918
Contributor
There was a problem hiding this comment. Shell escaping doesn't check for all metacharacters. Missing: Consider always quoting paths or adding more characters to the check:
Suggested change
|
||||||||||||||||||||||||||
| guard needsQuoting else { return path } | ||||||||||||||||||||||||||
| // Use single quotes with escaped inner single quotes | ||||||||||||||||||||||||||
| let escaped = path.replacingOccurrences(of: "'", with: "'\\''") | ||||||||||||||||||||||||||
| return "'\(escaped)'" | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
Comment on lines
+1914
to
1923
There was a problem hiding this comment. 馃З Analysis chain馃弫 Script executed: # First, let's find and examine the file
fd Sources/ContentView.swiftRepository: manaflow-ai/cmux Length of output: 42 馃弫 Script executed: # Get the line count to understand file size
wc -l Sources/ContentView.swiftRepository: manaflow-ai/cmux Length of output: 90 馃弫 Script executed: # Read the specific lines mentioned in the review (1914-1923)
sed -n '1914,1923p' Sources/ContentView.swiftRepository: manaflow-ai/cmux Length of output: 641 馃弫 Script executed: # Search for all usages of shellEscapePath to understand context
rg "shellEscapePath" -B2 -A2Repository: manaflow-ai/cmux Length of output: 777 馃弫 Script executed: # Look for any other shell escaping utilities or patterns in the codebase
rg "shell.*escape|escape.*shell" -iRepository: manaflow-ai/cmux Length of output: 2194 馃弫 Script executed: # Examine the escapeDropForShell implementation in GhosttyTerminalView.swift
rg "escapeDropForShell|fileprivate static func escape" -A 10 Sources/GhosttyTerminalView.swiftRepository: manaflow-ai/cmux Length of output: 1433 馃弫 Script executed: # Check test cases to see what edge cases are covered
cat tests/test_file_drop_paths.pyRepository: manaflow-ai/cmux Length of output: 2106 馃弫 Script executed: # Search for any newline handling in existing code
rg "newline|\\\\n" Sources/ContentView.swiftRepository: manaflow-ai/cmux Length of output: 92 馃弫 Script executed: # Find all callers of shellEscapePath to confirm it's used for untrusted input
rg "shellEscapePath" -B5 -A5Repository: manaflow-ai/cmux Length of output: 1231 Always quote file paths with single quotes to prevent shell metacharacter injection. The current implementation uses a character allowlist to decide whether quoting is needed, but misses shell-significant characters including glob chars ( Adopt the safer POSIX standard approach: always wrap in single quotes and escape inner single quotes with Proposed fix private func shellEscapePath(_ path: String) -> String {
- let needsQuoting = path.contains(" ") || path.contains("'") || path.contains("\"")
- || path.contains("(") || path.contains(")") || path.contains("&")
- || path.contains("|") || path.contains(";") || path.contains("$")
- || path.contains("`") || path.contains("!")
- guard needsQuoting else { return path }
- // Use single quotes with escaped inner single quotes
- let escaped = path.replacingOccurrences(of: "'", with: "'\\''")
- return "'\(escaped)'"
+ // Always single-quote for literal shell argument safety.
+ // Handles whitespace, glob chars, newlines, and metacharacters.
+ let escaped = path.replacingOccurrences(of: "'", with: "'\\''")
+ return "'\(escaped)'"
}馃 Prompt for AI Agents |
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| /// Space at top of content area for the titlebar. This must be at least the actual titlebar | ||||||||||||||||||||||||||
|
|
@@ -2493,7 +2632,8 @@ struct ContentView: View { | |||||||||||||||||||||||||
| windowId: windowId, | ||||||||||||||||||||||||||
| tabManager: tabManager, | ||||||||||||||||||||||||||
| sidebarState: sidebarState, | ||||||||||||||||||||||||||
| sidebarSelectionState: sidebarSelectionState, | ||||||||||||||||||||||||||
| sidebarContentModeState: sidebarContentModeState | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
| installFileDropOverlay(on: window, tabManager: tabManager) | ||||||||||||||||||||||||||
| })) | ||||||||||||||||||||||||||
|
|
@@ -5665,7 +5805,6 @@ struct VerticalTabsSidebar: View { | |||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| .accessibilityIdentifier("Sidebar") | ||||||||||||||||||||||||||
| .ignoresSafeArea() | ||||||||||||||||||||||||||
| .background( | ||||||||||||||||||||||||||
| WindowAccessor { window in | ||||||||||||||||||||||||||
| commandKeyMonitor.setHostWindow(window) | ||||||||||||||||||||||||||
|
|
@@ -8374,7 +8513,7 @@ enum SidebarSelection { | |||||||||||||||||||||||||
| case notifications | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| struct ClearScrollBackground: ViewModifier { | ||||||||||||||||||||||||||
| func body(content: Content) -> some View { | ||||||||||||||||||||||||||
| if #available(macOS 13.0, *) { | ||||||||||||||||||||||||||
| content | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
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.
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.
sidebarContentModeStatecan drift from the active window contextLine 3320 and Line 3768 only update this state in fallback branches. The primary activation path (
setActiveMainWindow) never assigns it, so Cmd+Shift+E can toggle the wrong window mode or no-op after window switches.馃敡 Proposed fix
private func setActiveMainWindow(_ window: NSWindow) { guard let context = contextForMainTerminalWindow(window) else { return } `#if` DEBUG let beforeManagerToken = debugManagerToken(tabManager) `#endif` tabManager = context.tabManager sidebarState = context.sidebarState sidebarSelectionState = context.sidebarSelectionState + sidebarContentModeState = context.sidebarContentModeState TerminalController.shared.setActiveTabManager(context.tabManager) `#if` DEBUG dlog( "mainWindow.active window={\(debugWindowToken(window))} context={\(debugContextToken(context))} beforeMgr=\(beforeManagerToken) afterMgr=\(debugManagerToken(tabManager)) \(debugShortcutRouteSnapshot())" ) `#endif` }if tabManager === removed.tabManager { // Repoint "active" pointers to any remaining main terminal window. let nextContext: MainWindowContext? = { if let keyWindow = NSApp.keyWindow, let ctx = contextForMainTerminalWindow(keyWindow, reindex: false) { return ctx } return mainWindowContexts.values.first }() if let nextContext { tabManager = nextContext.tabManager sidebarState = nextContext.sidebarState sidebarSelectionState = nextContext.sidebarSelectionState + sidebarContentModeState = nextContext.sidebarContentModeState TerminalController.shared.setActiveTabManager(nextContext.tabManager) } else { tabManager = nil sidebarState = nil sidebarSelectionState = nil + sidebarContentModeState = nil TerminalController.shared.setActiveTabManager(nil) } }Also applies to: 3768-3768
馃 Prompt for AI Agents