Repository navigation
Conversation
Adds a file tree browser to the sidebar that can be toggled with the tab list via persistent header buttons or Cmd+Shift+E. The file tree scans the active workspace directory, supports expand/collapse, hidden file toggle, refresh, and opens files in $EDITOR via the terminal. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds a "Sidebar File Tree" setting (Settings > App) with two modes: - Toggle (default): tabs and file tree are mutually exclusive (Cmd+Shift+E) - Split: tabs on top, file tree on bottom with a draggable horizontal divider Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Opens files from the file tree sidebar as in-app editor tabs instead of sending $EDITOR to the terminal. Plain text editing with monospaced NSTextView, Cmd+S save, dirty state tracking, and close confirmation for unsaved changes. Deduplicates tabs by file path. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Wire up Neon + CodeEditLanguages for incremental, viewport-aware syntax highlighting across 41 languages. Uses Dracula color spec, async highlighting setup with per-language caching, and plain-text paste override for the rich text view. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Swap out the custom NSTextView/Neon/CodeEditLanguages code editor for STTextView (TextKit 2) with Plugin-Neon syntax highlighting and Plugin-TextFormation for bracket pairing and smart indentation. - STTextView handles line numbers, current line highlight, scroll natively - Plugin-Neon provides Tree-sitter syntax highlighting (20 languages, Dracula theme) - Plugin-TextFormation provides auto-close brackets/quotes, skip-over, delete-matching, newline-between-pairs, and pattern-based indentation - Removes unused CodeEditorTextEditing.swift and LineNumberRulerView.swift - Adds editor-tab-width config key (default 4, clamped 1-16) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@Shehryar is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds a file-tree sidebar (toggleable or split), an in-app code editor panel with STTextView-based syntax highlighting and indentation plugins, per-window sidebar mode state, keyboard shortcut for toggling the file tree, and related build/package updates. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AppDelegate
participant ContentView
participant SidebarContentModeState
participant FileTreeModel
participant FileTreeSidebar
participant Workspace
User->>AppDelegate: Launch app
AppDelegate->>SidebarContentModeState: create per-window state
AppDelegate->>ContentView: inject environment object
User->>ContentView: Toggle File Tree (shortcut)
ContentView->>SidebarContentModeState: set mode = fileTree
SidebarContentModeState-->>ContentView: publishes mode change
ContentView->>FileTreeModel: loadDirectory(currentWorkspace)
FileTreeModel->>FileTreeModel: scanDirectory async
FileTreeModel-->>FileTreeSidebar: publish rootNodes
FileTreeSidebar->>User: render file tree
User->>FileTreeSidebar: click file
FileTreeSidebar->>Workspace: newCodeEditorSurfaceInFocusedPane(filePath)
Workspace->>Workspace: create CodeEditorPanel (load content)
Workspace-->>ContentView: provide panel to render
sequenceDiagram
participant User
participant CodeEditorPanelView
participant CodeEditorTextView
participant STTextView
participant NeonPlugin
participant TextFormationPlugin
participant CodeEditorPanel
User->>CodeEditorPanelView: open panel
CodeEditorPanelView->>CodeEditorPanel: request initial content
CodeEditorPanel-->>CodeEditorPanelView: return content
CodeEditorPanelView->>CodeEditorTextView: create NSViewRepresentable
CodeEditorTextView->>STTextView: initialize with theme & plugins
User->>STTextView: edit text
STTextView->>CodeEditorTextView: delegate textDidChange
CodeEditorTextView->>CodeEditorPanel: update isDirty
STTextView->>NeonPlugin: syntax tokens
NeonPlugin->>LanguageDetection: detect language
NeonPlugin-->>STTextView: apply highlighting
STTextView->>TextFormationPlugin: indentation/formatting
User->>STTextView: press Cmd+S
STTextView->>CodeEditorPanel: invoke save()
CodeEditorPanel->>Filesystem: write file, set isDirty=false
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Comment |
Contributor
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db85d157dd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+12
to
+14
| Task { | ||
| let nodes = await scanDirectory(path) | ||
| self.rootNodes = nodes |
There was a problem hiding this comment.
loadDirectory launches an untracked async scan and always assigns its result to rootNodes, so if users switch workspaces/directories quickly, an older scan can finish after a newer one and overwrite the tree with stale entries (while rootPath now points elsewhere). This causes the sidebar to show files from the wrong directory and can open the wrong file when clicked; add cancellation or verify the scanned path still matches the current root before publishing.
Useful? React with 👍 / 👎.
Comment on lines
+477
to
+483
| Button("Toggle File Tree") { | ||
| if sidebarContentModeState.mode == .fileTree { | ||
| sidebarContentModeState.mode = .tabs | ||
| } else { | ||
| sidebarContentModeState.mode = .fileTree | ||
| if !sidebarState.isVisible { | ||
| sidebarState.toggle() |
There was a problem hiding this comment.
This menu command mutates the app-level sidebarContentModeState/sidebarState directly, unlike the adjacent "Toggle Sidebar" action that first targets the active main window via AppDelegate. In multi-window use, invoking "Toggle File Tree" from a non-primary window will toggle the primary window's state instead of the focused window, producing inconsistent behavior between menu action and per-window shortcut handling.
Useful? React with 👍 / 👎.
Contributor
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db85d157dd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+12
to
+14
| Task { | ||
| let nodes = await scanDirectory(path) | ||
| self.rootNodes = nodes |
There was a problem hiding this comment.
loadDirectory starts a new Task and always assigns its scan result to rootNodes, but there is no generation/cancellation check against the latest rootPath. If the user switches tabs/workspaces quickly (or triggers another load before the first scan finishes), an older task can complete last and overwrite the tree with the wrong directory contents.
Useful? React with 👍 / 👎.
Comment on lines
+147
to
+149
| let _ = self.findAndUpdate(in: ¤t, id: nodeId) { n in | ||
| n.children = children | ||
| } |
There was a problem hiding this comment.
refresh() collects expanded IDs recursively, but in restoreExpandedState the async lazy-load path only does n.children = children and never reapplies expandedIds to the newly loaded subtree. This means nested folders that were expanded before refresh are collapsed afterward, despite the method claiming to preserve expanded state.
Useful? React with 👍 / 👎.
Comment on lines
+477
to
+481
| Button("Toggle File Tree") { | ||
| if sidebarContentModeState.mode == .fileTree { | ||
| sidebarContentModeState.mode = .tabs | ||
| } else { | ||
| sidebarContentModeState.mode = .fileTree |
There was a problem hiding this comment.
This command mutates the app-level sidebarContentModeState/sidebarState captured by cmuxApp, while additional windows get their own SidebarContentModeState in AppDelegate.createMainWindow. In multi-window use, invoking Toggle File Tree from a non-primary window can toggle the wrong window’s sidebar state.
Useful? React with 👍 / 👎.
AlexBoudreaux
added a commit
to AlexBoudreaux/cmux
that referenced
this pull request
Mar 1, 2026
Port file tree sidebar from PR manaflow-ai#601 (credit @Shehryar) and add cmd-click to inject file paths into the active terminal pane. - File tree sidebar toggled with Cmd+Shift+E - Toggle or split layout with workspace tabs - Cmd-click any file/folder to insert its shell-escaped path into terminal - Regular click expands/collapses dirs, selects files - Right-click context menu (copy path, reveal in Finder, open, insert path) - Lazy directory loading, hidden files toggle, manual refresh Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
8 tasks
AlexBoudreaux
added a commit
to AlexBoudreaux/cmux
that referenced
this pull request
Mar 2, 2026
Port file tree sidebar concept from PR manaflow-ai#601 (credit @Shehryar) and add cmd-click to inject file paths into the active terminal pane. - File tree sidebar toggled with Cmd+E or titlebar folder button - Cmd-click any file/folder to insert shell-escaped path into terminal - Regular click to select files, expand/collapse folders - Right-click context menu (Copy Path, Reveal in Finder, Open in Default App) - FSEvents file watching for automatic tree refresh - Hidden files shown by default - Lazy directory loading with preserved expand state on refresh Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
9 tasks done
- Add loadGeneration counter to guard against stale async directory scans overwriting rootNodes on rapid directory switches - Route Toggle Sidebar/File Tree menu actions through AppDelegate weak refs so they target the focused window instead of the app-level singleton - Replace fire-and-forget Task chain in restoreExpandedState with fully async restoreExpandedTree that builds the complete tree before setting rootNodes once, fixing nested expanded folders collapsing on refresh Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Resolved conflicts in: - AppDelegate.swift: kept session persistence + file tree content mode state - ContentView.swift: kept display-cycle fix, file tree onChange, upstream sidebar width/scrim - Workspace.swift: kept code editor method + upstream split zoom/context menu shortcuts - cmuxApp.swift: kept file tree settings + upstream sidebar toggles/workspace colors - project.pbxproj: merged upstream test files + file tree source refs - Added codeEditor cases to session persistence switches - Fixed DoubleClickZoomView -> WindowDragHandleView rename - Added triggerFlash() to CodeEditorPanel for Panel protocol conformance Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Shehryar
force-pushed
the
feature/file-tree-code-editor
branch
from
March 5, 2026 04:40
db85d15 to
6f35ec1
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (6)
Sources/Panels/CodeEditorPanelView.swift (1)
167-179: TheonSaveparameter inCoordinatoris unused.The coordinator stores
onSavebut only implementstextViewDidChangeText. Save functionality is handled separately viaSaveAwareSTTextView.onSave. Consider removing the unused parameter.♻️ Proposed cleanup
class Coordinator: NSObject, STTextViewDelegate { let onTextChange: () -> Void - let onSave: () -> Void - init(onTextChange: `@escaping` () -> Void, onSave: `@escaping` () -> Void) { + init(onTextChange: `@escaping` () -> Void) { self.onTextChange = onTextChange - self.onSave = onSave } func textViewDidChangeText(_ notification: Notification) { onTextChange() } }And update
makeCoordinator:func makeCoordinator() -> Coordinator { - Coordinator(onTextChange: onTextChange, onSave: onSave) + Coordinator(onTextChange: onTextChange) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Panels/CodeEditorPanelView.swift` around lines 167 - 179, Coordinator currently stores an unused onSave closure; remove the onSave property and its initializer parameter from class Coordinator and update makeCoordinator to stop passing onSave when creating a Coordinator instance, leaving only the onTextChange handling (textViewDidChangeText) intact; note that save behavior remains implemented in SaveAwareSTTextView.onSave so no additional save wiring is needed.Sources/FileTree/FileTreeSidebar.swift (1)
13-36: Consider removing the unusedGeometryReader.The
proxyparameter fromGeometryReaderis never used. If geometry information isn't needed, removing the wrapper simplifies the view hierarchy.♻️ Proposed simplification
VStack(spacing: 0) { // File tree content - GeometryReader { proxy in - ScrollView { + ScrollView { + if model.rootNodes.isEmpty { ... - } - .modifier(ClearScrollBackground()) } + .modifier(ClearScrollBackground()) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/FileTree/FileTreeSidebar.swift` around lines 13 - 36, The GeometryReader wrapper is unused because its proxy parameter isn’t referenced; remove the GeometryReader { proxy in ... } and its closing brace, and place the ScrollView (and its contents including the Empty directory branch, LazyVStack with ForEach over model.rootNodes, and the .modifier(ClearScrollBackground())) directly where the GeometryReader was so the view hierarchy is simplified; ensure FileTreeRow usages, depth: 0, model: model, and onFileAction remain unchanged.Sources/Panels/CodeEditorPanel.swift (1)
36-40: File I/O on@MainActormay cause UI jank for large files.Both
init(reading file) andsave()(writing file) perform synchronous file I/O on the main actor. For large files, this could block the UI thread.Consider moving file operations to a background task and updating state on completion, or accept this trade-off for simplicity given typical file sizes.
Also applies to: 43-51
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Panels/CodeEditorPanel.swift` around lines 36 - 40, The initializer currently reads the file synchronously on the `@MainActor` and save() likewise writes synchronously, which can block the UI; move both file I/O operations off the main actor (e.g., use Task.detached or a background DispatchQueue) to perform String(contentsOfFile:encoding:) and write(to:atomically:encoding:) asynchronously, then switch back to the main actor to assign self.initialContent or update any UI state; specifically change the init block that sets self.initialContent and the save() method to perform I/O off the main thread and only update state/UI on the main actor after the operation completes.Sources/Panels/LanguageDetection.swift (1)
75-85: The extension case"makefile"is unlikely to match.Files named
MakefileorGNUmakefiletypically have no file extension, sopathExtensionreturns an empty string. The extension case"makefile"on line 75 would only match files likefoo.makefile, which is uncommon. The filename-based fallback correctly handles the common case.This is a minor observation—the code works correctly due to the fallback.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Panels/LanguageDetection.swift` around lines 75 - 85, Summary: The extension case "makefile" on the pathExtension switch is misleading because Makefile/GNUmakefile files usually have no extension; remove or adjust it. Fix: In the switch that inspects pathExtension (the code branch containing case "makefile": return "Makefile"), either remove that "makefile" case entirely and rely on the filename-based fallback that uses (path as NSString).lastPathComponent.lowercased(), or, if you prefer to keep an extension check, normalize pathExtension with pathExtension.lowercased() and trim any leading dots before comparison; update the extension-switch accordingly and keep the filename-based switch (lastPathComponent) as the canonical detection for Makefile/GNUmakefile.Sources/Workspace.swift (1)
2168-2170: Normalize file paths before deduping editor panels.Line 2169 compares raw strings. Equivalent paths (e.g.,
~/repo/a.swiftvs absolute standardized path) can bypass dedupe and open duplicate tabs.♻️ Suggested refactor
/// Find an existing code editor panel with the given file path in this workspace. func existingCodeEditorPanel(forFilePath path: String) -> CodeEditorPanel? { - panels.values.compactMap { $0 as? CodeEditorPanel }.first { $0.filePath == path } + let normalizedPath = NSString(string: path).expandingTildeInPath + return panels.values + .compactMap { $0 as? CodeEditorPanel } + .first { NSString(string: $0.filePath).expandingTildeInPath == normalizedPath } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Workspace.swift` around lines 2168 - 2170, The current existingCodeEditorPanel(forFilePath:) compares raw filePath strings and can miss equivalent paths; update it to normalize both the incoming path and each CodeEditorPanel.filePath before comparison (e.g., use URL(fileURLWithPath:).standardized.path or NSString(string:).standardizingPath / resolving symlinks) so ~/ and relative/absolute differences and symlink variants are normalized; modify existingCodeEditorPanel(forFilePath:) to compute a standardizedPath for the input and compare against standardized versions of $0.filePath when filtering panels.GhosttyTabs.xcodeproj/project.pbxproj (1)
1016-1031: Consider pinning to explicit revisions for STTextView plugins.The project lockfile already pins these plugins to exact revisions (
5a30db4ce7908a5414e7b499e2379bdc49991cd1for Neon and77613b515506d561aa3e1bdc3881caab549803a4for TextFormation), which ensures reproducible builds. However, switching the pbxproj requirements fromkind = branch; branch = mainto pinned revisions would make the dependency contract more explicit and improve clarity across the build configuration.Suggested change pattern
A5003072 /* XCRemoteSwiftPackageReference "STTextView-Plugin-Neon" */ = { isa = XCRemoteSwiftPackageReference; repositoryURL = "https://github.com/krzyzanowskim/STTextView-Plugin-Neon.git"; requirement = { - kind = branch; - branch = main; + kind = revision; + revision = "5a30db4ce7908a5414e7b499e2379bdc49991cd1"; }; }; A5003082 /* XCRemoteSwiftPackageReference "STTextView-Plugin-TextFormation" */ = { isa = XCRemoteSwiftPackageReference; repositoryURL = "https://github.com/krzyzanowskim/STTextView-Plugin-TextFormation.git"; requirement = { - kind = branch; - branch = main; + kind = revision; + revision = "77613b515506d561aa3e1bdc3881caab549803a4"; }; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GhosttyTabs.xcodeproj/project.pbxproj` around lines 1016 - 1031, Change the package requirement from branch pinning to explicit revisions for the two XCRemoteSwiftPackageReference entries (A5003072 for "STTextView-Plugin-Neon" and A5003082 for "STTextView-Plugin-TextFormation"): replace the requirement block that currently has kind = branch; branch = main with a requirement block that pins kind = revision and the corresponding revision strings (use 5a30db4ce7908a5414e7b499e2379bdc49991cd1 for Neon and 77613b515506d561aa3e1bdc3881caab549803a4 for TextFormation), and ensure the pbxproj change matches the versions recorded in the project lockfile for reproducible builds.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/AppDelegate.swift`:
- Around line 6408-6421: The handler currently always returns true when the
toggle-file-tree shortcut matches, even if sidebarContentModeState is nil;
change the logic in the matchShortcut branch so you only return true when you
actually handled it (i.e., when sidebarContentModeState is non-nil and you
toggled modeState and optionally showed the sidebar via sidebarState.toggle());
if sidebarContentModeState is nil, return false so the shortcut is not consumed.
Ensure you reference the matchShortcut(event:),
KeyboardShortcutSettings.shortcut(for: .toggleFileTree),
sidebarContentModeState, modeState, and sidebarState/isVisible symbols when
making the change.
- Around line 7754-7757: Several context-sync fallback branches set sidebarState
and sidebarSelectionState (and call
TerminalController.shared.setActiveTabManager(context.tabManager)) but omit
updating sidebarContentModeState, causing Cmd+Shift+E to act on stale or nil
state; update every fallback path that currently assigns sidebarState and
sidebarSelectionState to also assign sidebarContentModeState =
context.sidebarContentModeState so the active mode is always propagated (search
for occurrences where sidebarState and sidebarSelectionState are set in
AppDelegate.swift and mirror the assignment for sidebarContentModeState,
including the branches implied around
TerminalController.shared.setActiveTabManager and context.tabManager).
In `@Sources/cmuxApp.swift`:
- Around line 3066-3079: The new SettingsCardRow UI uses bare user-facing string
literals ("Sidebar File Tree", the two subtitle variants, "Toggle", "Split" and
the picker accessibility label) which must be localized; update all literals in
the SettingsCardRow and its Picker to use String(localized: ..., defaultValue:
...) calls instead of plain strings (e.g., replace the "Sidebar File Tree"
title, each subtitle branch that references fileTreeLayout, and the Picker Texts
"Toggle"/"Split" with localized keys and defaultValue fallbacks) so that
SettingsCardRow, fileTreeLayout handling, and the Picker labels all display
localized strings.
- Around line 510-522: The Button currently uses a hard-coded label and isn't
wired to the stored keyboard shortcut; replace the plain Button("Toggle File
Tree") with a localized label (use NSLocalizedString or LocalizedStringKey for
"Toggle File Tree") and attach the app's configurable shortcut (use the existing
toggleFileTree shortcut/symbol from your shortcuts store, e.g.,
AppShortcuts.toggleFileTree or the command identifier you defined) so the UI
reflects the stored accelerator; keep the existing action body that toggles
AppDelegate.shared?.sidebarContentModeState.mode and calls
AppDelegate.shared?.sidebarState?.toggle() when revealing the file tree. Ensure
the displayed title uses the localized key and the shortcut reference is the one
used elsewhere for the command menu.
- Line 2772: resetAllSettings() currently omits the new `@AppStorage` property
sidebarFileTreeLayout so Reset All Settings doesn't clear that preference;
update resetAllSettings() to set sidebarFileTreeLayout back to its default value
SidebarFileTreeLayout.toggle.rawValue (use the same property name
sidebarFileTreeLayout in the reset method) so the sidebar file tree layout is
reset alongside the other settings.
In `@Sources/ContentView.swift`:
- Line 1851: The divider drag math uses NSApp.keyWindow?.contentView height
which is fragile; update splitDividerHandle and the other divider-related drag
calculations to use the local GeometryReader's height (the geometry parameter
passed into the surrounding GeometryReader) instead of querying NSApp.keyWindow.
Locate the splitDividerHandle binding/closure and any code blocks between the
other occurrences (around the other divider handlers at the later range) and
replace uses of keyWindow?.contentView?.frame.height with geometry.size.height
(or a local let containerHeight = geometry.size.height) so the ratio math uses
the local view height and works correctly in multi-window contexts.
- Around line 1865-1868: Replace the bare user-facing Text literals in
ContentView (e.g. the Text("No workspace") instance and the other occurrences
around the reported ranges) with localized strings by wrapping them with
String(localized:..., defaultValue:...), e.g. use a key like
"contentview.no_workspace" and defaultValue "No workspace"; do the same for the
other two literals mentioned (lines ~1958–1959 and ~1968) and ensure the Text
views are constructed with Text(String(localized: "your.key", defaultValue:
"English text")) so all UI strings follow the localization guideline.
- Around line 2646-2652: The reload currently only runs when
sidebarContentModeState.mode == .fileTree, but the split layout can render the
file tree even when mode == .tabs, so update the onChange handler (the closure
observing tabManager.selectedTabId) to call fileTreeModel.loadDirectory(dir)
whenever the file tree is actually rendered: replace the simple mode check with
a predicate that detects whether the file tree is shown (e.g. add a computed
Bool on SidebarContentModeState like rendersFileTree or a helper method and use
it), then when tabManager.selectedTab?.currentDirectory is non-nil call
fileTreeModel.loadDirectory(dir); keep using the same symbols
(tabManager.selectedTabId, sidebarContentModeState.mode,
tabManager.selectedTab?.currentDirectory, fileTreeModel.loadDirectory) so the
check covers both .fileTree and the split layout case.
In `@Sources/FileTree/FileTreeModel.swift`:
- Around line 23-40: The toggleExpand(_ node: FileTreeNode) async load must be
guarded by the same loadGeneration check used in loadDirectory/refresh: capture
the current loadGeneration (e.g., let gen = loadGeneration) before starting
Task, then inside the Task after awaiting scanDirectory and on `@MainActor` verify
that loadGeneration == gen before mutating rootNodes or assigning n.children; if
it differs, bail out to avoid overwriting newer tree state. Update the Task in
toggleExpand and the inner findAndUpdate usage so writes to rootNodes are
skipped when generation mismatches.
In `@Sources/FileTree/FileTreeRow.swift`:
- Around line 60-73: Replace the hard-coded context menu labels in FileTreeRow
(the Button initializers showing "Copy Path", "Reveal in Finder", "Open in
Default App", "Open in Editor") with localized strings using String(localized:
"key.name", defaultValue: "English text"); update each Button label to call
String(localized: ...) with an appropriate key (e.g., "file.copyPath",
"file.revealInFinder", "file.openInDefaultApp", "file.openInEditor") and keep
existing actions (NSPasteboard, NSWorkspace, onFileAction(node.path)) unchanged
so only the visible text is localized.
In `@Sources/FileTree/FileTreeSidebar.swift`:
- Around line 15-20: The placeholder Text("Empty directory") is a hard-coded
user-facing string; change it to use a localized key (e.g. replace the literal
Text with a Text initialized from a LocalizedStringKey like
LocalizedStringKey("Empty_directory") or use NSLocalizedString) and add the
corresponding "Empty_directory" = "Empty directory"; entry to your
Localizable.strings files so the UI uses the localized value; update the branch
that checks model.rootNodes.isEmpty and ensure the Text uses the new localized
key.
In `@Sources/KeyboardShortcutSettings.swift`:
- Line 80: The switch case returning the user-facing literal for the
.toggleFileTree shortcut should be localized; replace the bare string in
KeyboardShortcutSettings.swift (the case .toggleFileTree return) with a
localization call using String(localized: "keyboardShortcut.toggleFileTree",
defaultValue: "Toggle File Tree") (or your project's key naming convention) so
the UI uses the localized string; update any imports if needed and ensure the
localization key is added to the strings files.
In `@Sources/Panels/CodeEditorPanel.swift`:
- Around line 20-23: The fallback title in the displayTitle computed property
currently returns a hardcoded "Untitled"; change it to use a localized string
call (e.g. NSLocalizedString("Untitled", comment: "Fallback title for unnamed
file in code editor")) so the user-facing fallback is localized—update the
displayTitle getter (the var displayTitle and its use of name.isEmpty) to return
the localized string instead of the literal.
In `@Sources/Workspace.swift`:
- Around line 2179-2187: The existing branch reselects the tab even when focus
is explicitly false, mutating active selection; update the early-return block in
existingCodeEditorPanel(forFilePath:) so that after computing tabId via
surfaceIdFromPanelId(existing.id) you only call
bonsplitController.selectTab(tabId) (and focusPanel(existing.id)) when focus !=
false, otherwise skip selecting/focusing and just return existing; ensure you
still return existing in all cases and do not change selection when focus is
false.
---
Nitpick comments:
In `@GhosttyTabs.xcodeproj/project.pbxproj`:
- Around line 1016-1031: Change the package requirement from branch pinning to
explicit revisions for the two XCRemoteSwiftPackageReference entries (A5003072
for "STTextView-Plugin-Neon" and A5003082 for
"STTextView-Plugin-TextFormation"): replace the requirement block that currently
has kind = branch; branch = main with a requirement block that pins kind =
revision and the corresponding revision strings (use
5a30db4ce7908a5414e7b499e2379bdc49991cd1 for Neon and
77613b515506d561aa3e1bdc3881caab549803a4 for TextFormation), and ensure the
pbxproj change matches the versions recorded in the project lockfile for
reproducible builds.
In `@Sources/FileTree/FileTreeSidebar.swift`:
- Around line 13-36: The GeometryReader wrapper is unused because its proxy
parameter isn’t referenced; remove the GeometryReader { proxy in ... } and its
closing brace, and place the ScrollView (and its contents including the Empty
directory branch, LazyVStack with ForEach over model.rootNodes, and the
.modifier(ClearScrollBackground())) directly where the GeometryReader was so the
view hierarchy is simplified; ensure FileTreeRow usages, depth: 0, model: model,
and onFileAction remain unchanged.
In `@Sources/Panels/CodeEditorPanel.swift`:
- Around line 36-40: The initializer currently reads the file synchronously on
the `@MainActor` and save() likewise writes synchronously, which can block the UI;
move both file I/O operations off the main actor (e.g., use Task.detached or a
background DispatchQueue) to perform String(contentsOfFile:encoding:) and
write(to:atomically:encoding:) asynchronously, then switch back to the main
actor to assign self.initialContent or update any UI state; specifically change
the init block that sets self.initialContent and the save() method to perform
I/O off the main thread and only update state/UI on the main actor after the
operation completes.
In `@Sources/Panels/CodeEditorPanelView.swift`:
- Around line 167-179: Coordinator currently stores an unused onSave closure;
remove the onSave property and its initializer parameter from class Coordinator
and update makeCoordinator to stop passing onSave when creating a Coordinator
instance, leaving only the onTextChange handling (textViewDidChangeText) intact;
note that save behavior remains implemented in SaveAwareSTTextView.onSave so no
additional save wiring is needed.
In `@Sources/Panels/LanguageDetection.swift`:
- Around line 75-85: Summary: The extension case "makefile" on the pathExtension
switch is misleading because Makefile/GNUmakefile files usually have no
extension; remove or adjust it. Fix: In the switch that inspects pathExtension
(the code branch containing case "makefile": return "Makefile"), either remove
that "makefile" case entirely and rely on the filename-based fallback that uses
(path as NSString).lastPathComponent.lowercased(), or, if you prefer to keep an
extension check, normalize pathExtension with pathExtension.lowercased() and
trim any leading dots before comparison; update the extension-switch accordingly
and keep the filename-based switch (lastPathComponent) as the canonical
detection for Makefile/GNUmakefile.
In `@Sources/Workspace.swift`:
- Around line 2168-2170: The current existingCodeEditorPanel(forFilePath:)
compares raw filePath strings and can miss equivalent paths; update it to
normalize both the incoming path and each CodeEditorPanel.filePath before
comparison (e.g., use URL(fileURLWithPath:).standardized.path or
NSString(string:).standardizingPath / resolving symlinks) so ~/ and
relative/absolute differences and symlink variants are normalized; modify
existingCodeEditorPanel(forFilePath:) to compute a standardizedPath for the
input and compare against standardized versions of $0.filePath when filtering
panels.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8bf6f8bf-d3c7-439a-91b8-9cd10367c636
📒 Files selected for processing (19)
GhosttyTabs.xcodeproj/project.pbxprojGhosttyTabs.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolvedSources/AppDelegate.swiftSources/ContentView.swiftSources/FileTree/FileTreeModel.swiftSources/FileTree/FileTreeNode.swiftSources/FileTree/FileTreeRow.swiftSources/FileTree/FileTreeSidebar.swiftSources/FileTree/SidebarContentMode.swiftSources/GhosttyConfig.swiftSources/KeyboardShortcutSettings.swiftSources/Panels/CodeEditorPanel.swiftSources/Panels/CodeEditorPanelView.swiftSources/Panels/LanguageDetection.swiftSources/Panels/Panel.swiftSources/Panels/PanelContentView.swiftSources/Panels/SyntaxHighlightTheme.swiftSources/Workspace.swiftSources/cmuxApp.swift
Comment on lines
+6408
to
+6421
| if matchShortcut(event: event, shortcut: KeyboardShortcutSettings.shortcut(for: .toggleFileTree)) { | ||
| if let modeState = sidebarContentModeState { | ||
| if modeState.mode == .fileTree { | ||
| modeState.mode = .tabs | ||
| } else { | ||
| modeState.mode = .fileTree | ||
| // Show sidebar if hidden | ||
| if sidebarState?.isVisible == false { | ||
| sidebarState?.toggle() | ||
| } | ||
| } | ||
| } | ||
| return true | ||
| } |
There was a problem hiding this comment.
Don’t consume Toggle File Tree when no active mode state is available.
This block returns true even when sidebarContentModeState is nil, so the shortcut is swallowed with no behavior. Prefer returning false in that case.
🔧 Proposed fix
if matchShortcut(event: event, shortcut: KeyboardShortcutSettings.shortcut(for: .toggleFileTree)) {
- if let modeState = sidebarContentModeState {
- if modeState.mode == .fileTree {
- modeState.mode = .tabs
- } else {
- modeState.mode = .fileTree
- // Show sidebar if hidden
- if sidebarState?.isVisible == false {
- sidebarState?.toggle()
- }
- }
- }
+ guard let modeState = sidebarContentModeState else { return false }
+ if modeState.mode == .fileTree {
+ modeState.mode = .tabs
+ } else {
+ modeState.mode = .fileTree
+ // Show sidebar if hidden
+ if sidebarState?.isVisible == false {
+ sidebarState?.toggle()
+ }
+ }
return true
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if matchShortcut(event: event, shortcut: KeyboardShortcutSettings.shortcut(for: .toggleFileTree)) { | |
| if let modeState = sidebarContentModeState { | |
| if modeState.mode == .fileTree { | |
| modeState.mode = .tabs | |
| } else { | |
| modeState.mode = .fileTree | |
| // Show sidebar if hidden | |
| if sidebarState?.isVisible == false { | |
| sidebarState?.toggle() | |
| } | |
| } | |
| } | |
| return true | |
| } | |
| if matchShortcut(event: event, shortcut: KeyboardShortcutSettings.shortcut(for: .toggleFileTree)) { | |
| guard let modeState = sidebarContentModeState else { return false } | |
| if modeState.mode == .fileTree { | |
| modeState.mode = .tabs | |
| } else { | |
| modeState.mode = .fileTree | |
| // Show sidebar if hidden | |
| if sidebarState?.isVisible == false { | |
| sidebarState?.toggle() | |
| } | |
| } | |
| return true | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/AppDelegate.swift` around lines 6408 - 6421, The handler currently
always returns true when the toggle-file-tree shortcut matches, even if
sidebarContentModeState is nil; change the logic in the matchShortcut branch so
you only return true when you actually handled it (i.e., when
sidebarContentModeState is non-nil and you toggled modeState and optionally
showed the sidebar via sidebarState.toggle()); if sidebarContentModeState is
nil, return false so the shortcut is not consumed. Ensure you reference the
matchShortcut(event:), KeyboardShortcutSettings.shortcut(for: .toggleFileTree),
sidebarContentModeState, modeState, and sidebarState/isVisible symbols when
making the change.
Comment on lines
7754
to
7757
| sidebarState = context.sidebarState | ||
| sidebarSelectionState = context.sidebarSelectionState | ||
| sidebarContentModeState = context.sidebarContentModeState | ||
| TerminalController.shared.setActiveTabManager(context.tabManager) |
There was a problem hiding this comment.
Propagate sidebarContentModeState in all context-sync fallback branches.
Line 7756 and Line 7799 correctly set the active mode state, but fallback sync paths still omit it (Line 4082 and Line 4529). That can leave Cmd+Shift+E acting on stale/nil state when routing through a context without a concrete window object.
🔧 Proposed fix
--- a/Sources/AppDelegate.swift
+++ b/Sources/AppDelegate.swift
@@
let alreadyActive =
tabManager === context.tabManager
&& sidebarState === context.sidebarState
&& sidebarSelectionState === context.sidebarSelectionState
+ && sidebarContentModeState === context.sidebarContentModeState
@@
} else {
tabManager = context.tabManager
sidebarState = context.sidebarState
sidebarSelectionState = context.sidebarSelectionState
+ sidebarContentModeState = context.sidebarContentModeState
TerminalController.shared.setActiveTabManager(context.tabManager)
}
@@
let alreadyActive =
tabManager === context.tabManager
&& sidebarState === context.sidebarState
&& sidebarSelectionState === context.sidebarSelectionState
+ && sidebarContentModeState === context.sidebarContentModeState
@@
} else {
tabManager = context.tabManager
sidebarState = context.sidebarState
sidebarSelectionState = context.sidebarSelectionState
+ sidebarContentModeState = context.sidebarContentModeState
TerminalController.shared.setActiveTabManager(context.tabManager)
}Also applies to: 7797-7800
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/AppDelegate.swift` around lines 7754 - 7757, Several context-sync
fallback branches set sidebarState and sidebarSelectionState (and call
TerminalController.shared.setActiveTabManager(context.tabManager)) but omit
updating sidebarContentModeState, causing Cmd+Shift+E to act on stale or nil
state; update every fallback path that currently assigns sidebarState and
sidebarSelectionState to also assign sidebarContentModeState =
context.sidebarContentModeState so the active mode is always propagated (search
for occurrences where sidebarState and sidebarSelectionState are set in
AppDelegate.swift and mirror the assignment for sidebarContentModeState,
including the branches implied around
TerminalController.shared.setActiveTabManager and context.tabManager).
Comment on lines
+510
to
+522
| Button("Toggle File Tree") { | ||
| if let modeState = AppDelegate.shared?.sidebarContentModeState { | ||
| if modeState.mode == .fileTree { | ||
| modeState.mode = .tabs | ||
| } else { | ||
| modeState.mode = .fileTree | ||
| if AppDelegate.shared?.sidebarState?.isVisible == false { | ||
| AppDelegate.shared?.sidebarState?.toggle() | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Wire the command to the configurable shortcut (and localize its title).
This menu action is added as a plain Button, so it won’t reflect/use the stored toggleFileTree shortcut in the command menu path.
⌨️ Proposed fix
+ `@AppStorage`(KeyboardShortcutSettings.Action.toggleFileTree.defaultsKey)
+ private var toggleFileTreeShortcutData = Data()
@@
+ private var toggleFileTreeMenuShortcut: StoredShortcut {
+ decodeShortcut(
+ from: toggleFileTreeShortcutData,
+ fallback: KeyboardShortcutSettings.Action.toggleFileTree.defaultShortcut
+ )
+ }
@@
- Button("Toggle File Tree") {
+ splitCommandButton(
+ title: String(localized: "menu.toggleFileTree", defaultValue: "Toggle File Tree"),
+ shortcut: toggleFileTreeMenuShortcut
+ ) {
if let modeState = AppDelegate.shared?.sidebarContentModeState {
if modeState.mode == .fileTree {
modeState.mode = .tabs
} else {
modeState.mode = .fileTree
if AppDelegate.shared?.sidebarState?.isVisible == false {
AppDelegate.shared?.sidebarState?.toggle()
}
}
}
}As per coding guidelines: user-facing Swift strings must be localized, and bare UI string literals should not be used.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/cmuxApp.swift` around lines 510 - 522, The Button currently uses a
hard-coded label and isn't wired to the stored keyboard shortcut; replace the
plain Button("Toggle File Tree") with a localized label (use NSLocalizedString
or LocalizedStringKey for "Toggle File Tree") and attach the app's configurable
shortcut (use the existing toggleFileTree shortcut/symbol from your shortcuts
store, e.g., AppShortcuts.toggleFileTree or the command identifier you defined)
so the UI reflects the stored accelerator; keep the existing action body that
toggles AppDelegate.shared?.sidebarContentModeState.mode and calls
AppDelegate.shared?.sidebarState?.toggle() when revealing the file tree. Ensure
the displayed title uses the localized key and the shortcut reference is the one
used elsewhere for the command menu.
| @AppStorage(WorkspacePlacementSettings.placementKey) private var newWorkspacePlacement = WorkspacePlacementSettings.defaultPlacement.rawValue | ||
| @AppStorage(WorkspaceAutoReorderSettings.key) private var workspaceAutoReorder = WorkspaceAutoReorderSettings.defaultValue | ||
| @AppStorage(SidebarBranchLayoutSettings.key) private var sidebarBranchVerticalLayout = SidebarBranchLayoutSettings.defaultVerticalLayout | ||
| @AppStorage("sidebarFileTreeLayout") private var fileTreeLayout = SidebarFileTreeLayout.toggle.rawValue |
There was a problem hiding this comment.
Include sidebarFileTreeLayout in “Reset All Settings”.
The new @AppStorage("sidebarFileTreeLayout") setting is introduced, but resetAllSettings() does not reset it, so “Reset All Settings” leaves this preference stale.
🔁 Proposed fix
private func resetAllSettings() {
@@
sidebarBranchVerticalLayout = SidebarBranchLayoutSettings.defaultVerticalLayout
+ fileTreeLayout = SidebarFileTreeLayout.toggle.rawValue
sidebarActiveTabIndicatorStyle = SidebarActiveTabIndicatorSettings.defaultStyle.rawValue
@@
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @AppStorage("sidebarFileTreeLayout") private var fileTreeLayout = SidebarFileTreeLayout.toggle.rawValue | |
| private func resetAllSettings() { | |
| sidebarBranchVerticalLayout = SidebarBranchLayoutSettings.defaultVerticalLayout | |
| fileTreeLayout = SidebarFileTreeLayout.toggle.rawValue | |
| sidebarActiveTabIndicatorStyle = SidebarActiveTabIndicatorSettings.defaultStyle.rawValue | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/cmuxApp.swift` at line 2772, resetAllSettings() currently omits the
new `@AppStorage` property sidebarFileTreeLayout so Reset All Settings doesn't
clear that preference; update resetAllSettings() to set sidebarFileTreeLayout
back to its default value SidebarFileTreeLayout.toggle.rawValue (use the same
property name sidebarFileTreeLayout in the reset method) so the sidebar file
tree layout is reset alongside the other settings.
Comment on lines
+3066
to
+3079
| SettingsCardRow( | ||
| "Sidebar File Tree", | ||
| subtitle: fileTreeLayout == SidebarFileTreeLayout.split.rawValue | ||
| ? "Split: tabs and file tree shown together with a draggable divider." | ||
| : "Toggle: switch between tabs and file tree with Cmd+Shift+E.", | ||
| controlWidth: pickerColumnWidth | ||
| ) { | ||
| Picker("", selection: $fileTreeLayout) { | ||
| Text("Toggle").tag(SidebarFileTreeLayout.toggle.rawValue) | ||
| Text("Split").tag(SidebarFileTreeLayout.split.rawValue) | ||
| } | ||
| .labelsHidden() | ||
| .pickerStyle(.menu) | ||
| } |
There was a problem hiding this comment.
Localize new “Sidebar File Tree” settings strings.
The newly added row/subtitle/options are user-facing literals and should be keyed localized strings.
🌐 Proposed fix
- SettingsCardRow(
- "Sidebar File Tree",
+ SettingsCardRow(
+ String(localized: "settings.sidebarFileTree.title", defaultValue: "Sidebar File Tree"),
subtitle: fileTreeLayout == SidebarFileTreeLayout.split.rawValue
- ? "Split: tabs and file tree shown together with a draggable divider."
- : "Toggle: switch between tabs and file tree with Cmd+Shift+E.",
+ ? String(localized: "settings.sidebarFileTree.subtitle.split", defaultValue: "Split: tabs and file tree shown together with a draggable divider.")
+ : String(localized: "settings.sidebarFileTree.subtitle.toggle", defaultValue: "Toggle: switch between tabs and file tree with Cmd+Shift+E."),
controlWidth: pickerColumnWidth
) {
Picker("", selection: $fileTreeLayout) {
- Text("Toggle").tag(SidebarFileTreeLayout.toggle.rawValue)
- Text("Split").tag(SidebarFileTreeLayout.split.rawValue)
+ Text(String(localized: "settings.sidebarFileTree.option.toggle", defaultValue: "Toggle")).tag(SidebarFileTreeLayout.toggle.rawValue)
+ Text(String(localized: "settings.sidebarFileTree.option.split", defaultValue: "Split")).tag(SidebarFileTreeLayout.split.rawValue)
}As per coding guidelines: all user-facing Swift strings must be localized with String(localized:..., defaultValue:...) rather than bare literals.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| SettingsCardRow( | |
| "Sidebar File Tree", | |
| subtitle: fileTreeLayout == SidebarFileTreeLayout.split.rawValue | |
| ? "Split: tabs and file tree shown together with a draggable divider." | |
| : "Toggle: switch between tabs and file tree with Cmd+Shift+E.", | |
| controlWidth: pickerColumnWidth | |
| ) { | |
| Picker("", selection: $fileTreeLayout) { | |
| Text("Toggle").tag(SidebarFileTreeLayout.toggle.rawValue) | |
| Text("Split").tag(SidebarFileTreeLayout.split.rawValue) | |
| } | |
| .labelsHidden() | |
| .pickerStyle(.menu) | |
| } | |
| SettingsCardRow( | |
| String(localized: "settings.sidebarFileTree.title", defaultValue: "Sidebar File Tree"), | |
| subtitle: fileTreeLayout == SidebarFileTreeLayout.split.rawValue | |
| ? String(localized: "settings.sidebarFileTree.subtitle.split", defaultValue: "Split: tabs and file tree shown together with a draggable divider.") | |
| : String(localized: "settings.sidebarFileTree.subtitle.toggle", defaultValue: "Toggle: switch between tabs and file tree with Cmd+Shift+E."), | |
| controlWidth: pickerColumnWidth | |
| ) { | |
| Picker("", selection: $fileTreeLayout) { | |
| Text(String(localized: "settings.sidebarFileTree.option.toggle", defaultValue: "Toggle")).tag(SidebarFileTreeLayout.toggle.rawValue) | |
| Text(String(localized: "settings.sidebarFileTree.option.split", defaultValue: "Split")).tag(SidebarFileTreeLayout.split.rawValue) | |
| } | |
| .labelsHidden() | |
| .pickerStyle(.menu) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/cmuxApp.swift` around lines 3066 - 3079, The new SettingsCardRow UI
uses bare user-facing string literals ("Sidebar File Tree", the two subtitle
variants, "Toggle", "Split" and the picker accessibility label) which must be
localized; update all literals in the SettingsCardRow and its Picker to use
String(localized: ..., defaultValue: ...) calls instead of plain strings (e.g.,
replace the "Sidebar File Tree" title, each subtitle branch that references
fileTreeLayout, and the Picker Texts "Toggle"/"Split" with localized keys and
defaultValue fallbacks) so that SettingsCardRow, fileTreeLayout handling, and
the Picker labels all display localized strings.
Comment on lines
+2179
to
+2187
| // If a code editor for this file already exists, just focus it | ||
| if let existing = existingCodeEditorPanel(forFilePath: filePath) { | ||
| if let tabId = surfaceIdFromPanelId(existing.id) { | ||
| bonsplitController.selectTab(tabId) | ||
| if focus != false { | ||
| focusPanel(existing.id) | ||
| } | ||
| } | ||
| return existing |
There was a problem hiding this comment.
Respect focus == false when reopening an existing editor tab.
On Line 2182, the tab is selected even when focus is explicitly false. That still mutates active selection and can break non-focus callers.
🔧 Proposed fix
// If a code editor for this file already exists, just focus it
if let existing = existingCodeEditorPanel(forFilePath: filePath) {
- if let tabId = surfaceIdFromPanelId(existing.id) {
- bonsplitController.selectTab(tabId)
- if focus != false {
- focusPanel(existing.id)
- }
- }
+ if focus != false {
+ focusPanel(existing.id)
+ }
return existing
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/Workspace.swift` around lines 2179 - 2187, The existing branch
reselects the tab even when focus is explicitly false, mutating active
selection; update the early-return block in
existingCodeEditorPanel(forFilePath:) so that after computing tabId via
surfaceIdFromPanelId(existing.id) you only call
bonsplitController.selectTab(tabId) (and focusPanel(existing.id)) when focus !=
false, otherwise skip selecting/focusing and just return existing; ensure you
still return existing in all cases and do not change selection when focus is
false.
- Guard toggleExpand async load with loadGeneration to prevent race conditions - Localize all user-facing strings (context menus, empty state, keyboard shortcut) - Remove unused GeometryReader wrapper in FileTreeSidebar - Move file I/O off MainActor in CodeEditorPanel (async load + detached save) - Remove unused onSave from Coordinator (save handled by SaveAwareSTTextView) - Remove misleading "makefile" extension case in LanguageDetection - Normalize file paths in existingCodeEditorPanel comparison - Reload file tree on tab switch in split layout mode (not just toggle mode) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
Sources/ContentView.swift (2)
1865-1868:⚠️ Potential issue | 🟠 MajorLocalize the new file-tree user-facing strings.
These literals are still bare strings (
"No workspace", hidden-files help text,"Refresh"), which violates the Swift localization rule.🌐 Proposed fix
- Text("No workspace") + Text(String(localized: "sidebar.fileTree.emptyWorkspace", defaultValue: "No workspace")) .font(.caption) .foregroundColor(.secondary) - .help(fileTreeModel.showHiddenFiles ? "Hide hidden files" : "Show hidden files") + .help( + fileTreeModel.showHiddenFiles + ? String(localized: "sidebar.fileTree.hideHiddenFiles", defaultValue: "Hide hidden files") + : String(localized: "sidebar.fileTree.showHiddenFiles", defaultValue: "Show hidden files") + ) - .help("Refresh") + .help(String(localized: "sidebar.fileTree.refresh", defaultValue: "Refresh"))As per coding guidelines “All user-facing strings must be localized using
String(localized: "key.name", defaultValue: "English text")for every UI string” and “Do not use bare string literals in SwiftUIText(),Button(), alert titles, etc.; use localized strings instead.”Also applies to: 1958-1959, 1968-1968
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ContentView.swift` around lines 1865 - 1868, Replace all bare user-facing string literals in the file-tree UI with localized calls using String(localized:defaultValue:); specifically update Text("No workspace"), the hidden-files help text, and the "Refresh" button label (and the other occurrences flagged around the file-tree section) to use String(localized: "key.name", defaultValue: "No workspace"/appropriate English text) with meaningful localization keys; ensure you add unique keys for each string, keep the English text as defaultValue, and update the corresponding SwiftUI Text and Button initializers (e.g., the Text in ContentView that shows "No workspace" and the buttons/help labels) to use these localized values.
1851-1851:⚠️ Potential issue | 🟠 MajorUse local split container height for divider drag math.
Line 1893 still uses
NSApp.keyWindow?.contentView, so ratio math can drift in multi-window contexts. Use theGeometryReaderheight for this split container.📏 Proposed fix
- splitDividerHandle + splitDividerHandle(parentHeight: geo.size.height) - private var splitDividerHandle: some View { + private func splitDividerHandle(parentHeight: CGFloat) -> some View { ZStack { Divider() } @@ .onChanged { value in - guard let parent = NSApp.keyWindow?.contentView else { return } - let parentHeight = parent.frame.height guard parentHeight > 0 else { return } if splitDragStartRatio == nil { splitDragStartRatio = splitDividerRatio } let delta = value.translation.height / parentHeightAlso applies to: 1876-1901
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ContentView.swift` at line 1851, The drag math for the split divider uses the global window height (via NSApp.keyWindow?.contentView) which drifts across windows; update the split divider logic in the splitDividerHandle view so it uses the local GeometryReader height (the GeometryProxy.size.height passed into that container) for ratio calculations and clamping instead of any NSApp.keyWindow lookup; locate the drag gesture and any references to keyWindow/contentView in the splitDividerHandle and the surrounding split container (the region covering lines ~1876-1901) and replace them to compute newRatio = delta / localHeight (or similar) using the GeometryReader height variable.Sources/Workspace.swift (1)
2182-2190:⚠️ Potential issue | 🟡 MinorRespect
focus == falsewhen reopening an existing editor tab.
bonsplitController.selectTab(tabId)is called unconditionally on Line 2185, even whenfocusis explicitlyfalse. This mutates the active selection and can break callers that want to open/check a file without stealing focus.🔧 Proposed fix
// If a code editor for this file already exists, just focus it if let existing = existingCodeEditorPanel(forFilePath: filePath) { - if let tabId = surfaceIdFromPanelId(existing.id) { - bonsplitController.selectTab(tabId) - if focus != false { - focusPanel(existing.id) - } + if focus != false { + focusPanel(existing.id) } return existing }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Workspace.swift` around lines 2182 - 2190, The code unconditionally calls bonsplitController.selectTab(tabId) when an existing editor is found, which violates the caller's intent when focus == false; update the existingCodeEditorPanel handling so that surfaceIdFromPanelId(existing.id) and bonsplitController.selectTab(tabId) are only invoked when focus is not explicitly false, and keep focusPanel(existing.id) guarded as currently (i.e., only call focusPanel when focus != false); ensure the method returns the existing panel unchanged when focus == false so no tab selection occurs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/Panels/CodeEditorPanel.swift`:
- Around line 38-43: The current static func load(workspaceId:filePath:)
swallows file-read errors by collapsing failures to an empty string, risking
overwriting unreadable or non-UTF8 files; change load to surface read failures
instead of returning "" — e.g., make static func load(...) async throws, use let
content = try await Task.detached { try String(contentsOfFile: filePath,
encoding: .utf8) }.value (so the thrown error propagates), and update
callers/initializer (CodeEditorPanel(...) usage) to handle or propagate the
thrown error; this preserves the original file when reads fail and prevents
accidental overwrites.
In `@Sources/Panels/CodeEditorPanelView.swift`:
- Around line 54-56: The current closure assigned to panel.currentTextProvider
returns an empty string when textView is nil, allowing CodeEditorPanel.save() to
persist an empty file; change the provider to return an optional String (e.g.,
currentTextProvider: (() -> String?)?) and update the closure assigned to
panel.currentTextProvider to return textView?.text (no fallback). Then modify
CodeEditorPanel.save() to use guard let provider = currentTextProvider, let text
= provider() else { return } so save aborts when the editor view is unavailable.
In `@Sources/Panels/LanguageDetection.swift`:
- Around line 39-84: The switch in languageName(forFilePath:) returns hard-coded
English display names; update each returned literal (e.g., "Bash", "C++", "Plain
Text", "Makefile", "Dockerfile", "Git Config", etc.) to localized forms by
wrapping them with String(localized: "key", defaultValue: "English text") using
a clear key per label (e.g., "language.bash", "language.cpp",
"language.plainText", "filename.makefile", "filename.dockerfile",
"filename.gitConfig"); ensure every return in both the extension cases and the
filename-based inner switch uses the localized constructor so all user-facing
names are localizable.
---
Duplicate comments:
In `@Sources/ContentView.swift`:
- Around line 1865-1868: Replace all bare user-facing string literals in the
file-tree UI with localized calls using String(localized:defaultValue:);
specifically update Text("No workspace"), the hidden-files help text, and the
"Refresh" button label (and the other occurrences flagged around the file-tree
section) to use String(localized: "key.name", defaultValue: "No
workspace"/appropriate English text) with meaningful localization keys; ensure
you add unique keys for each string, keep the English text as defaultValue, and
update the corresponding SwiftUI Text and Button initializers (e.g., the Text in
ContentView that shows "No workspace" and the buttons/help labels) to use these
localized values.
- Line 1851: The drag math for the split divider uses the global window height
(via NSApp.keyWindow?.contentView) which drifts across windows; update the split
divider logic in the splitDividerHandle view so it uses the local GeometryReader
height (the GeometryProxy.size.height passed into that container) for ratio
calculations and clamping instead of any NSApp.keyWindow lookup; locate the drag
gesture and any references to keyWindow/contentView in the splitDividerHandle
and the surrounding split container (the region covering lines ~1876-1901) and
replace them to compute newRatio = delta / localHeight (or similar) using the
GeometryReader height variable.
In `@Sources/Workspace.swift`:
- Around line 2182-2190: The code unconditionally calls
bonsplitController.selectTab(tabId) when an existing editor is found, which
violates the caller's intent when focus == false; update the
existingCodeEditorPanel handling so that surfaceIdFromPanelId(existing.id) and
bonsplitController.selectTab(tabId) are only invoked when focus is not
explicitly false, and keep focusPanel(existing.id) guarded as currently (i.e.,
only call focusPanel when focus != false); ensure the method returns the
existing panel unchanged when focus == false so no tab selection occurs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8aac5c60-7b4f-481d-920d-e637777a9047
📒 Files selected for processing (9)
Sources/ContentView.swiftSources/FileTree/FileTreeModel.swiftSources/FileTree/FileTreeRow.swiftSources/FileTree/FileTreeSidebar.swiftSources/KeyboardShortcutSettings.swiftSources/Panels/CodeEditorPanel.swiftSources/Panels/CodeEditorPanelView.swiftSources/Panels/LanguageDetection.swiftSources/Workspace.swift
🚧 Files skipped from review as they are similar to previous changes (1)
- Sources/FileTree/FileTreeSidebar.swift
Comment on lines
+38
to
+43
| static func load(workspaceId: UUID, filePath: String) async -> CodeEditorPanel { | ||
| let content = await Task.detached { | ||
| (try? String(contentsOfFile: filePath, encoding: .utf8)) ?? "" | ||
| }.value | ||
| return await CodeEditorPanel(workspaceId: workspaceId, filePath: filePath, content: content) | ||
| } |
There was a problem hiding this comment.
Do not collapse file-read failures into empty content.
If loading fails, the panel is initialized with ""; a subsequent save can overwrite the original unreadable/non-UTF8 file with empty text.
🛡️ Proposed fail-safe
final class CodeEditorPanel: Panel, ObservableObject {
+ `@Published` private(set) var loadFailed: Bool = false
@@
static func load(workspaceId: UUID, filePath: String) async -> CodeEditorPanel {
- let content = await Task.detached {
- (try? String(contentsOfFile: filePath, encoding: .utf8)) ?? ""
- }.value
- return await CodeEditorPanel(workspaceId: workspaceId, filePath: filePath, content: content)
+ let content = await Task.detached {
+ try? String(contentsOfFile: filePath, encoding: .utf8)
+ }.value
+ let panel = await CodeEditorPanel(workspaceId: workspaceId, filePath: filePath, content: content ?? "")
+ if content == nil {
+ await MainActor.run { panel.loadFailed = true }
+ }
+ return panel
}
@@
func save() {
+ guard !loadFailed else { return }
guard let text = currentTextProvider?() else { return }
...
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/Panels/CodeEditorPanel.swift` around lines 38 - 43, The current
static func load(workspaceId:filePath:) swallows file-read errors by collapsing
failures to an empty string, risking overwriting unreadable or non-UTF8 files;
change load to surface read failures instead of returning "" — e.g., make static
func load(...) async throws, use let content = try await Task.detached { try
String(contentsOfFile: filePath, encoding: .utf8) }.value (so the thrown error
propagates), and update callers/initializer (CodeEditorPanel(...) usage) to
handle or propagate the thrown error; this preserves the original file when
reads fail and prevents accidental overwrites.
Comment on lines
+54
to
+56
| panel.currentTextProvider = { [weak textView] in | ||
| textView?.text ?? "" | ||
| } |
There was a problem hiding this comment.
Avoid empty-string fallback when the editor view is unavailable.
Line 55 returns "" when textView is gone, so CodeEditorPanel.save() can persist an empty file instead of aborting.
🔧 Proposed fix
- panel.currentTextProvider = { [weak textView] in
- textView?.text ?? ""
- }
+ panel.currentTextProvider = { [weak textView] in
+ textView?.text
+ }// Sources/Panels/CodeEditorPanel.swift
var currentTextProvider: (() -> String?)?
func save() {
guard let provider = currentTextProvider, let text = provider() else { return }
// existing save logic...
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/Panels/CodeEditorPanelView.swift` around lines 54 - 56, The current
closure assigned to panel.currentTextProvider returns an empty string when
textView is nil, allowing CodeEditorPanel.save() to persist an empty file;
change the provider to return an optional String (e.g., currentTextProvider: (()
-> String?)?) and update the closure assigned to panel.currentTextProvider to
return textView?.text (no fallback). Then modify CodeEditorPanel.save() to use
guard let provider = currentTextProvider, let text = provider() else { return }
so save aborts when the editor view is unavailable.
Comment on lines
+39
to
+84
| case "sh", "bash", "zsh": return "Bash" | ||
| case "c", "h": return "C" | ||
| case "cpp", "cc", "cxx", "hpp", "hxx": return "C++" | ||
| case "cs": return "C#" | ||
| case "css": return "CSS" | ||
| case "dart": return "Dart" | ||
| case "dockerfile": return "Dockerfile" | ||
| case "ex", "exs": return "Elixir" | ||
| case "go": return "Go" | ||
| case "hs": return "Haskell" | ||
| case "html", "htm": return "HTML" | ||
| case "java": return "Java" | ||
| case "js", "mjs", "cjs": return "JavaScript" | ||
| case "json", "jsonc": return "JSON" | ||
| case "jsx": return "JSX" | ||
| case "kt", "kts": return "Kotlin" | ||
| case "lua": return "Lua" | ||
| case "md", "markdown": return "Markdown" | ||
| case "m": return "Objective-C" | ||
| case "ml", "mli": return "OCaml" | ||
| case "pl", "pm": return "Perl" | ||
| case "php": return "PHP" | ||
| case "py", "pyw": return "Python" | ||
| case "rb": return "Ruby" | ||
| case "rs": return "Rust" | ||
| case "scala": return "Scala" | ||
| case "sql": return "SQL" | ||
| case "swift": return "Swift" | ||
| case "toml": return "TOML" | ||
| case "tsx": return "TSX" | ||
| case "ts", "mts", "cts": return "TypeScript" | ||
| case "yaml", "yml": return "YAML" | ||
| case "zig": return "Zig" | ||
| case "xml", "svg", "plist": return "XML" | ||
| case "txt": return "Plain Text" | ||
| case "cfg", "conf", "ini": return "Config" | ||
| default: | ||
| // Check filename-based detection | ||
| let name = (path as NSString).lastPathComponent.lowercased() | ||
| switch name { | ||
| case "makefile", "gnumakefile": return "Makefile" | ||
| case "dockerfile": return "Dockerfile" | ||
| case ".gitignore", ".gitattributes": return "Git Config" | ||
| case ".env": return "Env" | ||
| default: return nil | ||
| } |
There was a problem hiding this comment.
Localize returned language display names.
languageName(forFilePath:) returns user-visible English literals, so the header badge is not localizable.
🌐 Proposed approach
+ private static func localizedLanguage(_ key: String, _ fallback: String) -> String {
+ String(localized: key, defaultValue: fallback)
+ }
+
static func languageName(forFilePath path: String) -> String? {
let ext = (path as NSString).pathExtension.lowercased()
switch ext {
- case "sh", "bash", "zsh": return "Bash"
+ case "sh", "bash", "zsh": return localizedLanguage("codeEditor.language.bash", "Bash")
- case "py", "pyw": return "Python"
+ case "py", "pyw": return localizedLanguage("codeEditor.language.python", "Python")
...
- case "txt": return "Plain Text"
+ case "txt": return localizedLanguage("codeEditor.language.plainText", "Plain Text")
default:
let name = (path as NSString).lastPathComponent.lowercased()
switch name {
- case "makefile", "gnumakefile": return "Makefile"
+ case "makefile", "gnumakefile": return localizedLanguage("codeEditor.language.makefile", "Makefile")
...
}
}
}As per coding guidelines: "All user-facing strings must be localized using String(localized: "key.name", defaultValue: "English text") for every UI string (labels, buttons, menus, dialogs, tooltips, error messages)".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| case "sh", "bash", "zsh": return "Bash" | |
| case "c", "h": return "C" | |
| case "cpp", "cc", "cxx", "hpp", "hxx": return "C++" | |
| case "cs": return "C#" | |
| case "css": return "CSS" | |
| case "dart": return "Dart" | |
| case "dockerfile": return "Dockerfile" | |
| case "ex", "exs": return "Elixir" | |
| case "go": return "Go" | |
| case "hs": return "Haskell" | |
| case "html", "htm": return "HTML" | |
| case "java": return "Java" | |
| case "js", "mjs", "cjs": return "JavaScript" | |
| case "json", "jsonc": return "JSON" | |
| case "jsx": return "JSX" | |
| case "kt", "kts": return "Kotlin" | |
| case "lua": return "Lua" | |
| case "md", "markdown": return "Markdown" | |
| case "m": return "Objective-C" | |
| case "ml", "mli": return "OCaml" | |
| case "pl", "pm": return "Perl" | |
| case "php": return "PHP" | |
| case "py", "pyw": return "Python" | |
| case "rb": return "Ruby" | |
| case "rs": return "Rust" | |
| case "scala": return "Scala" | |
| case "sql": return "SQL" | |
| case "swift": return "Swift" | |
| case "toml": return "TOML" | |
| case "tsx": return "TSX" | |
| case "ts", "mts", "cts": return "TypeScript" | |
| case "yaml", "yml": return "YAML" | |
| case "zig": return "Zig" | |
| case "xml", "svg", "plist": return "XML" | |
| case "txt": return "Plain Text" | |
| case "cfg", "conf", "ini": return "Config" | |
| default: | |
| // Check filename-based detection | |
| let name = (path as NSString).lastPathComponent.lowercased() | |
| switch name { | |
| case "makefile", "gnumakefile": return "Makefile" | |
| case "dockerfile": return "Dockerfile" | |
| case ".gitignore", ".gitattributes": return "Git Config" | |
| case ".env": return "Env" | |
| default: return nil | |
| } | |
| private static func localizedLanguage(_ key: String, _ fallback: String) -> String { | |
| String(localized: key, defaultValue: fallback) | |
| } | |
| static func languageName(forFilePath path: String) -> String? { | |
| let ext = (path as NSString).pathExtension.lowercased() | |
| switch ext { | |
| case "sh", "bash", "zsh": return localizedLanguage("codeEditor.language.bash", "Bash") | |
| case "c", "h": return localizedLanguage("codeEditor.language.c", "C") | |
| case "cpp", "cc", "cxx", "hpp", "hxx": return localizedLanguage("codeEditor.language.cpp", "C++") | |
| case "cs": return localizedLanguage("codeEditor.language.csharp", "C#") | |
| case "css": return localizedLanguage("codeEditor.language.css", "CSS") | |
| case "dart": return localizedLanguage("codeEditor.language.dart", "Dart") | |
| case "dockerfile": return localizedLanguage("codeEditor.language.dockerfile", "Dockerfile") | |
| case "ex", "exs": return localizedLanguage("codeEditor.language.elixir", "Elixir") | |
| case "go": return localizedLanguage("codeEditor.language.go", "Go") | |
| case "hs": return localizedLanguage("codeEditor.language.haskell", "Haskell") | |
| case "html", "htm": return localizedLanguage("codeEditor.language.html", "HTML") | |
| case "java": return localizedLanguage("codeEditor.language.java", "Java") | |
| case "js", "mjs", "cjs": return localizedLanguage("codeEditor.language.javascript", "JavaScript") | |
| case "json", "jsonc": return localizedLanguage("codeEditor.language.json", "JSON") | |
| case "jsx": return localizedLanguage("codeEditor.language.jsx", "JSX") | |
| case "kt", "kts": return localizedLanguage("codeEditor.language.kotlin", "Kotlin") | |
| case "lua": return localizedLanguage("codeEditor.language.lua", "Lua") | |
| case "md", "markdown": return localizedLanguage("codeEditor.language.markdown", "Markdown") | |
| case "m": return localizedLanguage("codeEditor.language.objc", "Objective-C") | |
| case "ml", "mli": return localizedLanguage("codeEditor.language.ocaml", "OCaml") | |
| case "pl", "pm": return localizedLanguage("codeEditor.language.perl", "Perl") | |
| case "php": return localizedLanguage("codeEditor.language.php", "PHP") | |
| case "py", "pyw": return localizedLanguage("codeEditor.language.python", "Python") | |
| case "rb": return localizedLanguage("codeEditor.language.ruby", "Ruby") | |
| case "rs": return localizedLanguage("codeEditor.language.rust", "Rust") | |
| case "scala": return localizedLanguage("codeEditor.language.scala", "Scala") | |
| case "sql": return localizedLanguage("codeEditor.language.sql", "SQL") | |
| case "swift": return localizedLanguage("codeEditor.language.swift", "Swift") | |
| case "toml": return localizedLanguage("codeEditor.language.toml", "TOML") | |
| case "tsx": return localizedLanguage("codeEditor.language.tsx", "TSX") | |
| case "ts", "mts", "cts": return localizedLanguage("codeEditor.language.typescript", "TypeScript") | |
| case "yaml", "yml": return localizedLanguage("codeEditor.language.yaml", "YAML") | |
| case "zig": return localizedLanguage("codeEditor.language.zig", "Zig") | |
| case "xml", "svg", "plist": return localizedLanguage("codeEditor.language.xml", "XML") | |
| case "txt": return localizedLanguage("codeEditor.language.plainText", "Plain Text") | |
| case "cfg", "conf", "ini": return localizedLanguage("codeEditor.language.config", "Config") | |
| default: | |
| // Check filename-based detection | |
| let name = (path as NSString).lastPathComponent.lowercased() | |
| switch name { | |
| case "makefile", "gnumakefile": return localizedLanguage("codeEditor.language.makefile", "Makefile") | |
| case "dockerfile": return localizedLanguage("codeEditor.language.dockerfile", "Dockerfile") | |
| case ".gitignore", ".gitattributes": return localizedLanguage("codeEditor.language.gitConfig", "Git Config") | |
| case ".env": return localizedLanguage("codeEditor.language.env", "Env") | |
| default: return nil | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/Panels/LanguageDetection.swift` around lines 39 - 84, The switch in
languageName(forFilePath:) returns hard-coded English display names; update each
returned literal (e.g., "Bash", "C++", "Plain Text", "Makefile", "Dockerfile",
"Git Config", etc.) to localized forms by wrapping them with String(localized:
"key", defaultValue: "English text") using a clear key per label (e.g.,
"language.bash", "language.cpp", "language.plainText", "filename.makefile",
"filename.dockerfile", "filename.gitConfig"); ensure every return in both the
extension cases and the filename-based inner switch uses the localized
constructor so all user-facing names are localizable.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 9 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Sources/Panels/CodeEditorPanel.swift">
<violation number="1" location="Sources/Panels/CodeEditorPanel.swift:40">
P0: This converts read failures into empty content, which can cause destructive data loss: unreadable/non-UTF8 files are opened as `""` and a later save overwrites the file with an empty body. Preserve load failure state (or propagate the error) and prevent saving when the initial read did not succeed.</violation>
<violation number="2" location="Sources/Panels/CodeEditorPanel.swift:51">
P2: The async save clears `isDirty` unconditionally, so edits made while the background write is running can get marked clean even though they’re unsaved. Guard the dirty reset with a check that the current text still matches the saved snapshot.</violation>
</file>
<file name="Sources/Workspace.swift">
<violation number="1" location="Sources/Workspace.swift:2195">
P2: `await` is introduced between duplicate-check and panel insertion, which can create duplicate editor tabs for the same file under concurrent opens.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| static func load(workspaceId: UUID, filePath: String) async -> CodeEditorPanel { | ||
| let content = await Task.detached { | ||
| (try? String(contentsOfFile: filePath, encoding: .utf8)) ?? "" |
There was a problem hiding this comment.
P0: This converts read failures into empty content, which can cause destructive data loss: unreadable/non-UTF8 files are opened as "" and a later save overwrites the file with an empty body. Preserve load failure state (or propagate the error) and prevent saving when the initial read did not succeed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/Panels/CodeEditorPanel.swift, line 40:
<comment>This converts read failures into empty content, which can cause destructive data loss: unreadable/non-UTF8 files are opened as `""` and a later save overwrites the file with an empty body. Preserve load failure state (or propagate the error) and prevent saving when the initial read did not succeed.</comment>
<file context>
@@ -28,25 +28,30 @@ final class CodeEditorPanel: Panel, ObservableObject {
- }
+ static func load(workspaceId: UUID, filePath: String) async -> CodeEditorPanel {
+ let content = await Task.detached {
+ (try? String(contentsOfFile: filePath, encoding: .utf8)) ?? ""
+ }.value
+ return await CodeEditorPanel(workspaceId: workspaceId, filePath: filePath, content: content)
</file context>
|
|
||
| let shouldFocusNewTab = focus ?? (bonsplitController.focusedPaneId == paneId) | ||
|
|
||
| let editorPanel = await CodeEditorPanel.load(workspaceId: id, filePath: filePath) |
There was a problem hiding this comment.
P2: await is introduced between duplicate-check and panel insertion, which can create duplicate editor tabs for the same file under concurrent opens.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/Workspace.swift, line 2195:
<comment>`await` is introduced between duplicate-check and panel insertion, which can create duplicate editor tabs for the same file under concurrent opens.</comment>
<file context>
@@ -2189,7 +2192,7 @@ final class Workspace: Identifiable, ObservableObject {
let shouldFocusNewTab = focus ?? (bonsplitController.focusedPaneId == paneId)
- let editorPanel = CodeEditorPanel(workspaceId: id, filePath: filePath)
+ let editorPanel = await CodeEditorPanel.load(workspaceId: id, filePath: filePath)
panels[editorPanel.id] = editorPanel
panelTitles[editorPanel.id] = editorPanel.displayTitle
</file context>
| Task.detached { | ||
| do { | ||
| try text.write(toFile: path, atomically: true, encoding: .utf8) | ||
| await MainActor.run { self.isDirty = false } |
There was a problem hiding this comment.
P2: The async save clears isDirty unconditionally, so edits made while the background write is running can get marked clean even though they’re unsaved. Guard the dirty reset with a check that the current text still matches the saved snapshot.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/Panels/CodeEditorPanel.swift, line 51:
<comment>The async save clears `isDirty` unconditionally, so edits made while the background write is running can get marked clean even though they’re unsaved. Guard the dirty reset with a check that the current text still matches the saved snapshot.</comment>
<file context>
@@ -28,25 +28,30 @@ final class CodeEditorPanel: Panel, ObservableObject {
+ Task.detached {
+ do {
+ try text.write(toFile: path, atomically: true, encoding: .utf8)
+ await MainActor.run { self.isDirty = false }
+ } catch {
+ NSLog("[CodeEditorPanel] Failed to save \(path): \(error)")
</file context>
Suggested change
| await MainActor.run { self.isDirty = false } | |
| await MainActor.run { | |
| if self.currentTextProvider?() == text { | |
| self.isDirty = false | |
| } | |
| } |
Author
|
Closing this our since it's drifted pretty far |
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
2 participants
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.
Summary
Details
File tree sidebar:
Split sidebar layout:
Code editor panel:
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit