feat(macos): Add text editor with file tree sidebar#148
Merged
2witstudios merged 5 commits intomainfrom Mar 16, 2026
Merged
Conversation
Transform WorktreeDetailView into a split view with collapsible file tree sidebar and tabbed editor area. Users can now browse, open, edit, and save files directly in PurePoint without leaving the app. - FileNode/EditorTab models with language detection - FileTreeState with lazy directory scanning and gitignore filtering - EditorState with tab management, dirty tracking, and external change detection - FileIOService for async I/O with binary detection - FileTreeWatcher for multi-directory DispatchSource monitoring - NSOutlineView file tree with context menu (Reveal in Finder, Copy Path) - NSTextView editor with line numbers, Cmd+S save, tab-to-spaces - Tab bar with Changes tab (existing diffs) and file tabs - EditorTheme with syntax colors derived from terminal ANSI palette - SyntaxHighlightManager stub for future tree-sitter integration Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds a macOS code editor and file-tree subsystem: editor tab and language models, file-tree models and NSOutlineView bridge, async file I/O with binary detection, directory watcher, editor text view + ruler + SwiftUI bridge, syntax-highlight scaffold, adaptive editor theme, and state managers tying UI and filesystem events. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Editor UI
participant State as EditorState
participant FileIO as FileIOService
participant Watcher as FileTreeWatcher
User->>UI: open file (path, name)
UI->>State: openFile(path, name)
State->>FileIO: readFile(at: path)
FileIO-->>State: (content, isBinary)
State->>State: add/activate EditorTab (content, language, isBinary)
State->>Watcher: watchDirectory(path)
State-->>UI: present content / binary placeholder
User->>UI: edit content
UI->>State: updateContent(id, content)
State->>State: markDirty(id)
User->>UI: save (Cmd+S)
UI->>State: saveFile(id)
State->>FileIO: writeFile(content, to: path)
FileIO-->>State: success -> update lastModified, clear dirty
Watcher->>State: debounced onChange
State->>FileIO: fileModificationDate/readFile
alt file changed externally
State->>UI: show externalChangeAlert / reload or prompt
else no change or auto-reload
State->>UI: reload content
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
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)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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: 345c1d014c
ℹ️ 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
+159
to
+163
| Task { | ||
| do { | ||
| let result = try await FileIOService.readFile(at: tab.id) | ||
| openTabs[idx].content = result.content | ||
| openTabs[idx].lastModified = currentMod |
There was a problem hiding this comment.
checkForExternalChanges() captures idx from openTabs.enumerated() and then writes to openTabs[idx] after await FileIOService.readFile(...). If a tab is closed while that read is in flight, indices shift and this write can target the wrong tab or go out of bounds, which can crash the editor during normal filesystem-change refreshes. Re-look up the tab by tab.id after the await before mutating state.
Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment.
Fixed in d200951. checkForExternalChanges() now captures tabId before the Task and re-resolves the index via openTabs.firstIndex(where:) after the async read completes. Same fix applied to reloadFile().
Comment on lines
+105
to
+107
| guard let contents = try? fm.contentsOfDirectory(atPath: worktreePath) else { return } | ||
|
|
||
| let paths = contents.filter { !Self.hiddenNames.contains($0) && !$0.hasPrefix(".") } |
There was a problem hiding this comment.
loadGitIgnored only sends the immediate contentsOfDirectory(atPath: worktreePath) entries to git check-ignore --stdin, but later filtering compares against relative paths at any depth (for example pkg/node_modules). Because nested paths are never queried, ignored files inside subdirectories are shown in the tree even though the feature is expected to respect .gitignore; git check-ignore -h confirms it only evaluates the pathnames you provide on stdin.
Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment.
Fixed in d200951. Rewrote gitignore filtering: computeGitIgnored() is now a nonisolated static method that runs in Task.detached and takes the directory path + worktree root, computing full relative paths. It runs per-directory (root + each expanded directory) so nested ignored paths are properly caught.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (3)
apps/purepoint-macos/purepoint-macos/Theme/EditorTheme.swift (1)
24-34: Avoid duplicating adaptive color logic across theme types.This helper duplicates the appearance-switch logic already present in
Theme.swift(Theme.adaptive), which increases drift risk. Consider centralizing this in one shared helper and reusing it here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/purepoint-macos/purepoint-macos/Theme/EditorTheme.swift` around lines 24 - 34, The adaptive color logic in EditorTheme.swift (private static func adaptive) duplicates Theme.adaptive from Theme.swift; replace this local implementation by calling the shared Theme.adaptive helper instead: remove the private static func adaptive in EditorTheme and use Theme.adaptive(dark:light:) (or the exact shared function name in Theme.swift) wherever EditorTheme.adaptive was used so all appearance switching is centralized in Theme.adaptive.apps/purepoint-macos/purepoint-macos/Views/Editor/FileTreeOutlineView.swift (1)
7-27: Callbacks not updated when SwiftUI parent re-renders.The
onFileSelected,onNodeExpanded, andonNodeCollapsedcallbacks are wired only inmakeNSViewController. If the parent view supplies different closures on re-render (e.g., capturing updated state), those changes won't propagate to the existing controller.♻️ Proposed fix to update callbacks in updateNSViewController
func updateNSViewController(_ vc: FileTreeOutlineViewController, context: Context) { + vc.onFileSelected = { path, name in + onFileSelected(path, name) + } + vc.onNodeExpanded = { [fileTreeState] node in + fileTreeState.expandNode(node) + } + vc.onNodeCollapsed = { [fileTreeState] node in + fileTreeState.collapseNode(node) + } vc.updateNodes(fileTreeState.rootNodes) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/purepoint-macos/purepoint-macos/Views/Editor/FileTreeOutlineView.swift` around lines 7 - 27, The callbacks are only set in makeNSViewController so they won't reflect new closures when SwiftUI re-renders; in updateNSViewController reassign vc.onFileSelected, vc.onNodeExpanded, and vc.onNodeCollapsed to the current closures (using the current onFileSelected and fileTreeState) before calling vc.updateNodes(fileTreeState.rootNodes) so the controller always uses the latest handlers; reference the FileTreeOutlineViewController instance (vc) and the existing symbols makeNSViewController, updateNSViewController, onFileSelected, onNodeExpanded, onNodeCollapsed, and fileTreeState when making the change.apps/purepoint-macos/purepoint-macos/State/EditorState.swift (1)
86-91: Consider guarding against saving binary files.While the UI prevents editing binary files,
saveFileunconditionally writes content. If called programmatically on a binary tab (e.g., via keyboard shortcut while binary placeholder is shown), it would overwrite the file with empty content.🛡️ Proposed defensive guard
func saveFile(id: String) async throws { guard let idx = openTabs.firstIndex(where: { $0.id == id }) else { return } + guard !openTabs[idx].isBinary else { return } try await FileIOService.writeFile(content: openTabs[idx].content, to: id) openTabs[idx].isDirty = false openTabs[idx].lastModified = FileIOService.fileModificationDate(at: id) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/purepoint-macos/purepoint-macos/State/EditorState.swift` around lines 86 - 91, saveFile currently writes openTabs[idx].content unconditionally and can overwrite binary files; add a defensive guard at the start of saveFile to skip writing when the tab is a binary/placeholder tab (check the tab's binary indicator such as openTabs[idx].isBinary or similar flag used for binary placeholders) and return early so FileIOService.writeFile is not called; preserve existing isDirty and lastModified behavior only for non-binary tabs and avoid updating them when the save is skipped.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/purepoint-macos/purepoint-macos/Services/FileTreeWatcher.swift`:
- Around line 84-89: The deinit is reading and mutating sources without
synchronizing with the serial queue, causing a race; wrap the cleanup in a
synchronous dispatch to the instance's queue (use queue.sync) so you read and
modify sources and cancel each entry.source and debounceWork on the queue
thread, e.g. capture and clear the sources inside queue.sync then iterate and
cancel each entry.source and cancel debounceWork within that protected block;
reference deinit, sources, queue, debounceWork, and entry.source.cancel to
locate the code to change.
- Around line 32-39: The delete/rename branch in FileTreeWatcher (inside
watchDirectory handling) closes fd directly while the DispatchSource remains
active, risking a double-close in the source's cancelHandler; change the
sequence to retrieve and cancel the DispatchSource stored in self.sources for
the given path (call source.cancel()), remove it from self.sources, then
close(fd) and re-call watchDirectory(path:) and scheduleDebounce(); ensure you
reference the same source stored in self.sources and do not close fd twice (the
cancelHandler will run after cancel() so closing here is safe only after
canceling the source).
In `@apps/purepoint-macos/purepoint-macos/State/EditorState.swift`:
- Around line 93-104: reloadFile currently captures idx before awaiting an async
Task which can become stale; instead, inside the Task re-resolve the tab by id
(e.g., find index via openTabs.firstIndex(where: { $0.id == id }) or directly
mutate the tab by id) and only then apply result.content, isDirty = false, and
lastModified from FileIOService.fileModificationDate(at: id); also ensure
externalChangeAlert is cleared after the successful update or handle failures
consistently. Target symbols: function reloadFile, openTabs, idx,
FileIOService.readFile, FileIOService.fileModificationDate, externalChangeAlert.
- Around line 159-165: The async Task closes over the loop-local idx which can
become invalid if openTabs mutates before FileIOService.readFile completes;
capture the tab identity and then locate the tab inside the Task before mutating
state. Specifically, store let tabId = tab.id (or capture the tab object id)
before Task, await FileIOService.readFile(at: tabId), then find the current
index with openTabs.firstIndex(where: { $0.id == tabId }) and only update
openTabs[index].content and openTabs[index].lastModified if the index exists;
avoid using the original idx inside the Task and handle the missing-index case
(and consider logging the error rather than silently swallowing it).
In `@apps/purepoint-macos/purepoint-macos/State/FileTreeState.swift`:
- Around line 124-130: FileTreeState's loadGitIgnored() runs
Process.waitUntilExit() on the `@MainActor`, blocking the UI; move the git
check-ignore work off the main actor (e.g. wrap the Process
creation/run/waitUntilExit and reading of inputPipe/output in Task.detached or a
background queue) and then marshal the computed result back onto the main actor
(await MainActor.run { ... }) to assign state (e.g. update your gitIgnored
property). In practice: wrap the code that creates Process, sets up
inputPipe/outputPipe, calls try process.run(), writes to inputPipe, and calls
process.waitUntilExit() inside a background Task, compute the ignored paths
there, and only update FileTreeState on the main actor afterward.
In `@apps/purepoint-macos/purepoint-macos/Views/Detail/WorktreeDetailView.swift`:
- Around line 44-52: The editor state isn't cleared when the worktree changes;
update the .task(id: worktree.id) block (where
diffState.loadForWorktree(worktree) and fileTreeState.load(worktreePath:
worktree.path) are called) to also reset the editor state for the new worktree
by calling a clear/reset method on editorState (e.g., editorState.reset() or
editorState.clearOpenTabs()) before loading the new fileTree; if such a method
doesn't exist add one that closes tabs and clears references to files from the
previous worktree so editorState doesn't retain invalid tabs.
- Around line 119-123: Replace the silent try? in the onSave Task with explicit
error handling: call editorState.saveFile(id: tab.id) inside a do/catch so you
can catch and surface failures (for example by updating the view model state and
presenting an Alert or calling whatever user-facing error handler your app
uses), and only clear or update the dirty state after a successful save; in
other words, modify the onSave Task to await editorState.saveFile(id: tab.id)
inside a do block and handle errors in catch to notify the user and avoid
incorrectly mutating editorState on failure.
In `@apps/purepoint-macos/purepoint-macos/Views/Editor/EditorTabBar.swift`:
- Around line 68-95: The FileTabButton currently nests a close Button inside the
label of the selection Button (the outer Button that calls onSelect), which
causes gesture and accessibility issues; remove the outer Button and make the
tab selectable via a plain container with .contentShape(Rectangle()) and
.onTapGesture(perform: onSelect) while keeping the close control as an actual
Button that calls onClose (retain the Image(tab.language.icon), Text(tab.name),
dirty Circle, and the isActive/isHovered logic); ensure the container has the
same padding/frame/background/overlay styling (used for active state and
underline) and still updates isHovered with .onHover so visual and interaction
behavior remain unchanged.
In
`@apps/purepoint-macos/purepoint-macos/Views/Editor/FileTreeOutlineViewController.swift`:
- Around line 148-152: The NSImage initializer for systemSymbolName is
force-unwrapped using iconName (computed from node.isDirectory ? "folder.fill" :
fileIcon(for: node.name)), which can crash if the symbol is unsupported; change
the NSImage creation in the FileTreeOutlineViewController to safely unwrap and
provide a fallback symbol (e.g., "doc" or "questionmark")—compute the intended
iconName, try NSImage(systemSymbolName:iconName, accessibilityDescription:), and
if that returns nil use a fallback symbol image before constructing the
NSImageView and setting contentTintColor and symbolConfiguration.
In `@apps/purepoint-macos/purepoint-macos/Views/Editor/LineNumberRulerView.swift`:
- Around line 9-11: The initializer init(textView: NSTextView) for
LineNumberRulerView currently force-unwraps textView.enclosingScrollView which
can crash if the text view isn't yet embedded; update the initializer to safely
handle a nil enclosingScrollView by either requiring a scroll view parameter or
by using textView.enclosingScrollView ?? NSScrollView() (or creating/deferring
attachment) and initialize via super.init(scrollView: safeScrollView,
orientation: .verticalRuler), and ensure clientView = textView still runs; if
you choose to accept a scroll view param, adjust callers to pass the containing
NSScrollView and document the change.
---
Nitpick comments:
In `@apps/purepoint-macos/purepoint-macos/State/EditorState.swift`:
- Around line 86-91: saveFile currently writes openTabs[idx].content
unconditionally and can overwrite binary files; add a defensive guard at the
start of saveFile to skip writing when the tab is a binary/placeholder tab
(check the tab's binary indicator such as openTabs[idx].isBinary or similar flag
used for binary placeholders) and return early so FileIOService.writeFile is not
called; preserve existing isDirty and lastModified behavior only for non-binary
tabs and avoid updating them when the save is skipped.
In `@apps/purepoint-macos/purepoint-macos/Theme/EditorTheme.swift`:
- Around line 24-34: The adaptive color logic in EditorTheme.swift (private
static func adaptive) duplicates Theme.adaptive from Theme.swift; replace this
local implementation by calling the shared Theme.adaptive helper instead: remove
the private static func adaptive in EditorTheme and use
Theme.adaptive(dark:light:) (or the exact shared function name in Theme.swift)
wherever EditorTheme.adaptive was used so all appearance switching is
centralized in Theme.adaptive.
In `@apps/purepoint-macos/purepoint-macos/Views/Editor/FileTreeOutlineView.swift`:
- Around line 7-27: The callbacks are only set in makeNSViewController so they
won't reflect new closures when SwiftUI re-renders; in updateNSViewController
reassign vc.onFileSelected, vc.onNodeExpanded, and vc.onNodeCollapsed to the
current closures (using the current onFileSelected and fileTreeState) before
calling vc.updateNodes(fileTreeState.rootNodes) so the controller always uses
the latest handlers; reference the FileTreeOutlineViewController instance (vc)
and the existing symbols makeNSViewController, updateNSViewController,
onFileSelected, onNodeExpanded, onNodeCollapsed, and fileTreeState when making
the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 11c32c78-10a6-4a4d-b653-f5c9ab9417b9
📒 Files selected for processing (16)
apps/purepoint-macos/purepoint-macos/Models/EditorTab.swiftapps/purepoint-macos/purepoint-macos/Models/FileNode.swiftapps/purepoint-macos/purepoint-macos/Services/FileIOService.swiftapps/purepoint-macos/purepoint-macos/Services/FileTreeWatcher.swiftapps/purepoint-macos/purepoint-macos/Services/SyntaxHighlightManager.swiftapps/purepoint-macos/purepoint-macos/State/EditorState.swiftapps/purepoint-macos/purepoint-macos/State/FileTreeState.swiftapps/purepoint-macos/purepoint-macos/Theme/EditorTheme.swiftapps/purepoint-macos/purepoint-macos/Views/Detail/WorktreeDetailView.swiftapps/purepoint-macos/purepoint-macos/Views/Editor/EditorNSView.swiftapps/purepoint-macos/purepoint-macos/Views/Editor/EditorTabBar.swiftapps/purepoint-macos/purepoint-macos/Views/Editor/EditorTextView.swiftapps/purepoint-macos/purepoint-macos/Views/Editor/FileTreeOutlineView.swiftapps/purepoint-macos/purepoint-macos/Views/Editor/FileTreeOutlineViewController.swiftapps/purepoint-macos/purepoint-macos/Views/Editor/FileTreeSidebarView.swiftapps/purepoint-macos/purepoint-macos/Views/Editor/LineNumberRulerView.swift
apps/purepoint-macos/purepoint-macos/Views/Detail/WorktreeDetailView.swift
Show resolved
Hide resolved
apps/purepoint-macos/purepoint-macos/Views/Detail/WorktreeDetailView.swift
Show resolved
Hide resolved
apps/purepoint-macos/purepoint-macos/Views/Editor/EditorTabBar.swift
Outdated
Show resolved
Hide resolved
apps/purepoint-macos/purepoint-macos/Views/Editor/FileTreeOutlineViewController.swift
Show resolved
Hide resolved
apps/purepoint-macos/purepoint-macos/Views/Editor/LineNumberRulerView.swift
Show resolved
Hide resolved
- Fix stale index bugs in EditorState: re-resolve tab by id after async operations in reloadFile() and checkForExternalChanges() - Fix double-close of fd in FileTreeWatcher: cancel DispatchSource before re-watching on delete/rename events - Fix race condition in FileTreeWatcher deinit: use queue.sync for cleanup - Move git check-ignore off @mainactor: run Process in Task.detached to avoid blocking the main thread - Fix gitignore for nested paths: run check-ignore per expanded directory with full relative paths instead of root-only names - Reset EditorState when worktree changes to avoid stale tabs - Surface save errors via banner instead of silent try? - Fix nested Button in EditorTabBar: use onTapGesture + contentShape for tab selection, keep close as standalone Button - Add NSImage fallback for unsupported SF Symbol names - Guard against nil enclosingScrollView in LineNumberRulerView init Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (3)
apps/purepoint-macos/purepoint-macos/Views/Editor/LineNumberRulerView.swift (3)
74-81: Performance: O(n) line counting on every draw may cause lag in large files.The loop that counts lines before the visible range (lines 76-81) runs once per line from the beginning of the document to the visible area. For a large file scrolled to line 50,000, this executes 50,000
lineRangecalls on every scroll/draw event, potentially causing UI jank.Consider caching the line count or using a more efficient approach such as counting newline characters in the substring before the visible range.
♻️ Proposed optimization using substring newline count
var lineNumber = 1 // Count lines before visible range - var idx = 0 - while idx < visibleCharRange.location { - let lineRange = content.lineRange(for: NSRange(location: idx, length: 0)) - lineNumber += 1 - idx = NSMaxRange(lineRange) + if visibleCharRange.location > 0 { + let prefixRange = NSRange(location: 0, length: visibleCharRange.location) + let prefix = content.substring(with: prefixRange) + // Count newlines; each newline ends a line, so line number = newline count + 1 + lineNumber = prefix.filter { $0 == "\n" }.count + 1 }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/purepoint-macos/purepoint-macos/Views/Editor/LineNumberRulerView.swift` around lines 74 - 81, The current loop in LineNumberRulerView that increments lineNumber by iterating from idx = 0 to visibleCharRange.location using content.lineRange(for:) is O(n) per draw; replace it with a fast count of newline characters in the substring up to visibleCharRange.location (or maintain a cached line index updated on text edits) to compute the starting line number in O(m) where m is the visible prefix length or O(1) for cache lookups; locate the code around the lineNumber calculation in LineNumberRulerView (the block initializing var lineNumber = 1 and looping with lineRange) and change it to count "\n" occurrences in content up to visibleCharRange.location (or consult/maintain a lines cache keyed by document modifications) and use that result to set lineNumber.
18-18: Minor: Redundant optional chain whenscrollViewis already available.Line 10 already extracts
scrollViewfromtextView.enclosingScrollView. Reuse that local variable instead of re-accessing the optional.♻️ Proposed fix
- if let clipView = textView.enclosingScrollView?.contentView { + if let clipView = scrollView.contentView {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/purepoint-macos/purepoint-macos/Views/Editor/LineNumberRulerView.swift` at line 18, The code redundantly re-accesses textView.enclosingScrollView when a local scrollView is already available; replace the optional chain "textView.enclosingScrollView?.contentView" with the existing "scrollView?.contentView" (or safely unwrap the local scrollView) so the code uses the previously captured scrollView variable when assigning clipView in LineNumberRulerView (referencing textView, scrollView, clipView, enclosingScrollView).
4-6: Consider dynamic gutter width for files with many lines.The fixed
gutterWidthof 40 points may truncate line numbers in files exceeding ~9,999 lines (5+ digit numbers). Consider dynamically calculating width based on the total line count, or at minimum using a slightly larger default.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/purepoint-macos/purepoint-macos/Views/Editor/LineNumberRulerView.swift` around lines 4 - 6, The gutterWidth is fixed at 40 which can truncate large line numbers; replace the static gutterWidth with a computed property that measures the width needed for the maximum line number using the existing lineNumberFont: calculate maxLine = max(1, totalLineCount) (or derive from the text storage/line index method used in LineNumberRulerView), compute digits = floor(log10(maxLine)) + 1, create a sample string of that many "9" characters, measure its NSAttributedString size with lineNumberFont, add a small horizontal padding (e.g. 8–12 pts) and use that value as the gutter width; ensure you update any layout/drawing code that referenced the old gutterWidth (e.g., draw or layout methods in LineNumberRulerView) and call invalidateIntrinsicContentSize / setNeedsDisplay when totalLineCount changes so the ruler resizes dynamically.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/purepoint-macos/purepoint-macos/Views/Editor/LineNumberRulerView.swift`:
- Around line 74-81: The current loop in LineNumberRulerView that increments
lineNumber by iterating from idx = 0 to visibleCharRange.location using
content.lineRange(for:) is O(n) per draw; replace it with a fast count of
newline characters in the substring up to visibleCharRange.location (or maintain
a cached line index updated on text edits) to compute the starting line number
in O(m) where m is the visible prefix length or O(1) for cache lookups; locate
the code around the lineNumber calculation in LineNumberRulerView (the block
initializing var lineNumber = 1 and looping with lineRange) and change it to
count "\n" occurrences in content up to visibleCharRange.location (or
consult/maintain a lines cache keyed by document modifications) and use that
result to set lineNumber.
- Line 18: The code redundantly re-accesses textView.enclosingScrollView when a
local scrollView is already available; replace the optional chain
"textView.enclosingScrollView?.contentView" with the existing
"scrollView?.contentView" (or safely unwrap the local scrollView) so the code
uses the previously captured scrollView variable when assigning clipView in
LineNumberRulerView (referencing textView, scrollView, clipView,
enclosingScrollView).
- Around line 4-6: The gutterWidth is fixed at 40 which can truncate large line
numbers; replace the static gutterWidth with a computed property that measures
the width needed for the maximum line number using the existing lineNumberFont:
calculate maxLine = max(1, totalLineCount) (or derive from the text storage/line
index method used in LineNumberRulerView), compute digits =
floor(log10(maxLine)) + 1, create a sample string of that many "9" characters,
measure its NSAttributedString size with lineNumberFont, add a small horizontal
padding (e.g. 8–12 pts) and use that value as the gutter width; ensure you
update any layout/drawing code that referenced the old gutterWidth (e.g., draw
or layout methods in LineNumberRulerView) and call
invalidateIntrinsicContentSize / setNeedsDisplay when totalLineCount changes so
the ruler resizes dynamically.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7af96bba-61e7-49bc-9906-cb9f97f8ef35
📒 Files selected for processing (7)
apps/purepoint-macos/purepoint-macos/Services/FileTreeWatcher.swiftapps/purepoint-macos/purepoint-macos/State/EditorState.swiftapps/purepoint-macos/purepoint-macos/State/FileTreeState.swiftapps/purepoint-macos/purepoint-macos/Views/Detail/WorktreeDetailView.swiftapps/purepoint-macos/purepoint-macos/Views/Editor/EditorTabBar.swiftapps/purepoint-macos/purepoint-macos/Views/Editor/FileTreeOutlineViewController.swiftapps/purepoint-macos/purepoint-macos/Views/Editor/LineNumberRulerView.swift
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/purepoint-macos/purepoint-macos/Services/FileTreeWatcher.swift
- apps/purepoint-macos/purepoint-macos/Views/Editor/FileTreeOutlineViewController.swift
- apps/purepoint-macos/purepoint-macos/Views/Editor/EditorTabBar.swift
- apps/purepoint-macos/purepoint-macos/State/FileTreeState.swift
- apps/purepoint-macos/purepoint-macos/State/EditorState.swift
Add identity guard in updateNodes to skip reloadData when nodes haven't changed, and add children-loaded guard in outlineViewItemDidExpand to break the reloadItem → expand feedback loop. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add .clipped() to DraggableSplit first pane and EditorTabBar ScrollView, and set low compression resistance on file tree labels so long names truncate instead of bleeding into the editor area. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/purepoint-macos/purepoint-macos/Views/Editor/EditorTabBar.swift (1)
97-98: File tabs are not keyboard-accessible unlike the Changes tab.Using
.onTapGesturefor tab selection means file tabs cannot be focused or activated via keyboard, whereas the Changes tab (which usesButton) can. For consistent macOS accessibility, consider wrapping the selectable area in a focusable control or adding keyboard navigation support.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/purepoint-macos/purepoint-macos/Views/Editor/EditorTabBar.swift` around lines 97 - 98, The file tabs use .contentShape(Rectangle()) and .onTapGesture(perform: onSelect) which prevents keyboard focus/activation; replace the tappable area with a Button(action: onSelect) wrapping the same label content (or convert the view to a Button in EditorTabBar.swift), apply .buttonStyle(PlainButtonStyle()) to preserve visuals, and keep/restore .contentShape(Rectangle()) if needed so the hit area remains the same; this ensures the onSelect handler is keyboard-accessible and behaves like the existing Changes tab Button.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/purepoint-macos/purepoint-macos/Views/Editor/EditorTabBar.swift`:
- Around line 84-93: The close button currently calls onClose() unconditionally
which leads to editorState.closeTab(id:) removing tabs with unsaved changes;
update the close flow to guard dirty tabs: in EditorTabBar.swift where the
Button triggers onClose(), first check the tab.isDirty flag and if true present
a confirmation/save prompt (or call a new helper that shows a save/discard/
cancel dialog) and only call onClose() when the user confirms discard or after
save; alternatively implement the guard inside EditorState.closeTab(id:) to
detect dirty state and present the same confirmation/save dialog before removing
the tab (reference functions/values: the Button closure in EditorTabBar,
onClose(), tab.isDirty, WorktreeDetailView wiring, and
EditorState.closeTab(id:)).
In
`@apps/purepoint-macos/purepoint-macos/Views/Editor/FileTreeOutlineViewController.swift`:
- Around line 58-61: The identity-only short-circuit in updateNodes(_: ) can
skip real updates because FileTreeNode.children may be mutated in place by
FileTreeState.expandNode(_:), so change the guard that returns early to also
detect children mutations: either remove the identity-only early return, or
replace it with a deeper equality check that compares each node's children (at
least counts or child identities, or a recursive fingerprint) so content changes
trigger an update; refer to updateNodes, FileTreeNode.children and
FileTreeState.expandNode(_:) when implementing the fix.
---
Nitpick comments:
In `@apps/purepoint-macos/purepoint-macos/Views/Editor/EditorTabBar.swift`:
- Around line 97-98: The file tabs use .contentShape(Rectangle()) and
.onTapGesture(perform: onSelect) which prevents keyboard focus/activation;
replace the tappable area with a Button(action: onSelect) wrapping the same
label content (or convert the view to a Button in EditorTabBar.swift), apply
.buttonStyle(PlainButtonStyle()) to preserve visuals, and keep/restore
.contentShape(Rectangle()) if needed so the hit area remains the same; this
ensures the onSelect handler is keyboard-accessible and behaves like the
existing Changes tab Button.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 18788d95-4689-43bc-8fc3-5b709b647ee2
📒 Files selected for processing (3)
apps/purepoint-macos/purepoint-macos/Views/Editor/EditorTabBar.swiftapps/purepoint-macos/purepoint-macos/Views/Editor/FileTreeOutlineViewController.swiftapps/purepoint-macos/purepoint-macos/Views/PaneGrid/DraggableSplit.swift
Comment on lines
+84
to
+93
| if isActive || isHovered { | ||
| Button { | ||
| onClose() | ||
| } label: { | ||
| Image(systemName: "xmark") | ||
| .font(.system(size: 8, weight: .bold)) | ||
| .foregroundStyle(.secondary) | ||
| } | ||
| .buttonStyle(.plain) | ||
| } |
There was a problem hiding this comment.
Closing a dirty tab discards unsaved changes without confirmation.
The close button unconditionally calls onClose(), which per the wiring in WorktreeDetailView invokes editorState.closeTab(id:). That method removes the tab immediately without checking isDirty. Users can lose unsaved work with no warning.
Consider either:
- Prompting for confirmation when
tab.isDirtyis true before callingonClose(), or - Handling the guard in
EditorState.closeTab(id:)to present a save/discard dialog.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/purepoint-macos/purepoint-macos/Views/Editor/EditorTabBar.swift` around
lines 84 - 93, The close button currently calls onClose() unconditionally which
leads to editorState.closeTab(id:) removing tabs with unsaved changes; update
the close flow to guard dirty tabs: in EditorTabBar.swift where the Button
triggers onClose(), first check the tab.isDirty flag and if true present a
confirmation/save prompt (or call a new helper that shows a save/discard/ cancel
dialog) and only call onClose() when the user confirms discard or after save;
alternatively implement the guard inside EditorState.closeTab(id:) to detect
dirty state and present the same confirmation/save dialog before removing the
tab (reference functions/values: the Button closure in EditorTabBar, onClose(),
tab.isDirty, WorktreeDetailView wiring, and EditorState.closeTab(id:)).
Comment on lines
+58
to
+61
| func updateNodes(_ nodes: [FileTreeNode]) { | ||
| if nodes.count == rootNodes.count && zip(nodes, rootNodes).allSatisfy({ $0 === $1 }) { | ||
| return | ||
| } |
There was a problem hiding this comment.
updateNodes identity short-circuit can skip real tree changes
Line 59 returns early when node identities are unchanged, but FileTreeNode.children is mutated in place by FileTreeState.expandNode(_:) (including async git-ignore filtering). That means content can change while identities stay the same, and this guard can leave the outline stale.
💡 Proposed fix
func updateNodes(_ nodes: [FileTreeNode]) {
- if nodes.count == rootNodes.count && zip(nodes, rootNodes).allSatisfy({ $0 === $1 }) {
- return
- }
let scrollOrigin = scrollView.contentView.bounds.origin
rootNodes = nodes
outlineView.reloadData()
restoreExpansionState()
scrollView.contentView.scroll(to: scrollOrigin)
scrollView.reflectScrolledClipView(scrollView.contentView)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/purepoint-macos/purepoint-macos/Views/Editor/FileTreeOutlineViewController.swift`
around lines 58 - 61, The identity-only short-circuit in updateNodes(_: ) can
skip real updates because FileTreeNode.children may be mutated in place by
FileTreeState.expandNode(_:), so change the guard that returns early to also
detect children mutations: either remove the identity-only early return, or
replace it with a deeper equality check that compares each node's children (at
least counts or child identities, or a recursive fingerprint) so content changes
trigger an update; refer to updateNodes, FileTreeNode.children and
FileTreeState.expandNode(_:) when implementing the fix.
9 tasks
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
1 participant
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
DraggableSplitNew files (14)
Models/FileNode.swiftModels/EditorTab.swiftState/FileTreeState.swiftState/EditorState.swiftServices/FileIOService.swiftServices/FileTreeWatcher.swiftServices/SyntaxHighlightManager.swiftViews/Editor/FileTreeOutlineViewController.swiftViews/Editor/FileTreeOutlineView.swiftViews/Editor/FileTreeSidebarView.swiftViews/Editor/EditorTextView.swiftViews/Editor/LineNumberRulerView.swiftViews/Editor/EditorNSView.swiftViews/Editor/EditorTabBar.swiftTheme/EditorTheme.swiftReview fixes (d200951)
All 12 review comments addressed:
reloadFile()andcheckForExternalChanges()try?Before building
New files must be added to the Xcode project target. SPM packages for tree-sitter (Neon, SwiftTreeSitter, language grammars) are deferred to a follow-up.
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit