Repository navigation
Add TreeSitter syntax highlighting and markdown preview to file preview panel - #4259
shishirsharma wants to merge 16 commits into
Conversation
|
@shishirsharma is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughIntegrates CodeEdit packages, adds a SyntaxLanguageDetector gating highlightability, enables a Markdown-specific "Open Preview" control, replaces text preview routing with a highlighted-editor router and bridge-based CodeEdit integration, and adds an opt-in CI flag to skip package plugin validation. ChangesCodeEdit Integration & Markdown Preview with Syntax-Highlighted Editor
Sequence DiagramsequenceDiagram
participant Router as HighlightedFilePreviewRouter
participant Cache as FileLanguageCache
participant Detector as SyntaxLanguageDetector
participant Highlighted as HighlightedFilePreviewEditor
participant Bridge as HighlightedEditorBridge
participant Plain as PlainFilePreviewEditor
Router->>Cache: check cached language for panel.fileURL
alt language detected
Cache->>Detector: request language(for: url)
Detector->>Detector: check extension allowlist
Detector->>Detector: check file size < maxHighlightBytes
Detector-->>Cache: return CodeLanguage?
Cache-->>Router: cached language available
Router->>Highlighted: render highlighted path
Highlighted->>Bridge: initialize and sync content/theme
Bridge->>Bridge: publish font size and foreground/background
else no language or unsupported
Cache-->>Router: no language
Router->>Plain: render plain fallback
end
🎯 3 (Moderate) | ⏱️ ~25 minutes
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (3 errors, 1 warning)
✅ Passed checks (12 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Greptile SummaryThis PR adds TreeSitter-based syntax highlighting to the file preview panel via
Confidence Score: 5/5Safe to merge; the highlighted editor path is well-isolated and falls back to the unchanged plain editor for unsupported files. All previously flagged issues have been addressed: the zoom monitor is removed synchronously in dismantleNSView, actor isolation uses Task @mainactor throughout, the nil cache sentinel is stored correctly with updateValue, and SyntaxLanguageDetector is extracted to its own file. The only remaining finding is that the static language cache has no eviction policy, which is a long-session memory concern rather than a functional defect. SyntaxLanguageDetector.swift — static cache grows without bound; no other files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["FilePreviewPanelView (.text mode)"] --> B["HighlightedFilePreviewRouter"]
B --> C{"FileLanguageCache (StateObject, runs once)"}
C -->|"language detected"| D["HighlightedFilePreviewEditor (NSViewRepresentable)"]
C -->|"nil: unsupported ext or file > 500 KB"| E["FilePreviewTextEditor (plain NSTextView fallback)"]
D --> F["makeNSView: HighlightedEditorContainerView"]
F --> G["NSHostingView → HighlightedSourceEditorCore → SourceEditor"]
D --> I["HighlightedEditorBridge (TextViewCoordinator)"]
I -->|"prepareCoordinator Task @MainActor"| J["installZoomMonitor via NSLock"]
I -->|"textViewDidChangeText Task @MainActor"| K["panel.updateTextContent"]
I -->|"dismantleNSView"| L["removeZoomMonitor synchronous"]
K --> M["FilePreviewPanel @Published textContent"]
M -->|"SwiftUI updateNSView"| N["bridge.setContent (no-op if unchanged)"]
I --> O["registerFocusIfReady"]
O --> P["panel.attachTextInsertionTarget (TextView)"]
P --> Q["saveTextContent via filePreviewCurrentText"]
Reviews (14): Last reviewed commit: "fix: cache unsupported preview language ..." | Re-trigger Greptile |
There was a problem hiding this comment.
1 issue found across 6 files
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Re-trigger cubic
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@GhosttyTabs.xcodeproj/project.pbxproj`:
- Around line 2392-2407: The package version constraints for the
XCRemoteSwiftPackageReference entries "CodeEditSourceEditor" and
"CodeEditLanguages" are asymmetric and may break when CodeEditSourceEditor is
bumped; update the two XCRemoteSwiftPackageReference objects so they use
matching requirement kinds—either pin both to known-compatible exact versions
(e.g., set both to the exact versions that are known to work together) or make
both use upToNextMajorVersion with the same minimumVersion to allow coordinated
minor/patch updates; modify the entries for "CodeEditSourceEditor" and
"CodeEditLanguages" accordingly so their requirement.kind and
version/minimumVersion are consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8ff59953-0824-4fa6-985c-b343b146eb15
📒 Files selected for processing (4)
GhosttyTabs.xcodeproj/project.pbxprojSources/Panels/FilePreviewPanel.swiftSources/Panels/FilePreviewTextEditor.swiftSources/Panels/SyntaxLanguageDetector.swift
d47e8b3 to
050b33e
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/Panels/FilePreviewPanel.swift`:
- Around line 1269-1274: The markdown fallback currently calls
NSWorkspace.shared.open(fileURL) in FilePreviewPanel's action, which bypasses
the self-filtering in FileExternalOpenAction; replace that direct open with
invoking the shared external-open helper (use FileExternalOpenAction or its
public API) so the same self-filtering and external-launch logic runs for the
markdown fallback — e.g. call the shared action with the panel (panel.id,
panel.filePath, fileURL and/or owningWorkspace()) instead of
NSWorkspace.shared.open(fileURL).
In `@Sources/Panels/FilePreviewTextEditor.swift`:
- Around line 58-66: The highlighted branch in FilePreviewTextEditor ignores the
panel's drawsContentBackground setting causing opaque themes when syntax
highlighting is enabled; update the HighlightedFilePreviewEditor initializer
call to pass the panel.drawsContentBackground (or a derived drawsBackground
Bool) and then consume that value inside HighlightedFilePreviewEditor to set the
editor theme's background alpha/transparent color (or set the
NSTextView/NSTextField drawsBackground accordingly), so the highlighted path
respects drawsContentBackground just like the plain TextEditor path; reference
FilePreviewTextEditor, HighlightedFilePreviewEditor,
drawsContentBackground/drawsBackground and languageCache.language to locate the
change.
- Around line 201-205: registerFocusIfReady currently attaches focus to the
panel but never registers the highlighted text view with the panel, leaving
FilePreviewPanel.textView nil and causing handleDroppedFileURLsAsText(_:) to
bail; update registerFocusIfReady to call panel.attachTextView(textView) (or the
appropriate attachTextView(_) API) before calling panel.attachPreviewFocus(...)
so the panel's textView is populated and subsequent drops/hotpaths that rely on
FilePreviewPanel.textView work correctly.
In `@Sources/Panels/SyntaxLanguageDetector.swift`:
- Around line 51-53: The allowlist check currently compares filename ==
"Dockerfile" and misses lowercase variants; change the normalization so the
Dockerfile basename is lowercased before checking — e.g., compute a lowercased
basename (or use filename.lowercased()) and update the guard to check
supportedExtensions.contains(ext) || lowercasedBasename == "dockerfile" so both
"Dockerfile" and "dockerfile" are accepted; ensure you update references to
ext/filename as needed in SyntaxLanguageDetector so the comparison is
case-insensitive.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: fc33692e-9537-4a75-bf2d-f93363bf0758
📒 Files selected for processing (6)
CLAUDE.mdSources/Panels/FilePreviewPanel.swiftSources/Panels/FilePreviewTextEditor.swiftSources/Panels/SyntaxLanguageDetector.swiftcmux.xcodeproj/project.pbxprojcmux.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
Sources/Panels/FilePreviewTextEditor.swift (1)
204-210:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftHighlighted previews still lose file-drop insertion.
Line 209 only registers focus.
FilePreviewPanel.handleDroppedFileURLsAsText(_:)still requirespanel.textView, so every syntax-highlighted file drops the existing file-URL insertion path that the plain editor supports. Please keep a shared insertion path here, or fall back to the plain editor until the highlighted path can service drops too.Based on learnings: "When a behavior is exposed through multiple entrypoints (keyboard shortcut, command palette, context menu, CLI, settings, debug menu), implement one shared action/model path and verify every entrypoint that should invoke it."
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/Panels/FilePreviewPanel.swift`:
- Around line 1263-1268: The Markdown preview button currently uses
PanelHeaderIconButton with its isDisabled computed from panel.isDirty ||
panel.isSaving but misses panel.isFileUnavailable; update the disabled guard to
also include panel.isFileUnavailable so the button is disabled when the file is
unavailable (matching the logic used in FileExternalOpenMenu), i.e., add
panel.isFileUnavailable to the boolean expression guarding the
PanelHeaderIconButton.
In `@Sources/Panels/FilePreviewTextEditor.swift`:
- Around line 333-336: In makeSyntaxTheme(), compute isDark from
bridge.themeBackground regardless of bridge.drawsBackground so transparent
panels use the correct dark/light token palette; replace the current conditional
isDark assignment with one that calls backgroundIsDark(bridge.themeBackground)
unconditionally (keeping bg = bridge.drawsBackground ? bridge.themeBackground :
.clear and still using fg = bridge.themeForeground) so the syntax theme
selection uses bridge.themeBackground to pick the dark palette even when
drawsBackground is false.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9875b11c-158f-4d97-bfd5-01b2f615ea5d
📒 Files selected for processing (4)
Sources/Panels/FilePreviewPanel.swiftSources/Panels/FilePreviewTextEditor.swiftSources/Panels/SyntaxLanguageDetector.swiftcmux.xcodeproj/project.pbxproj
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/Panels/FilePreviewPanel.swift`:
- Around line 927-930: The markdown extension alias list is duplicated:
isMarkdownFile recognizes .mkd, .mkdn, .mdwn, and .mdown but
FilePreviewKindResolver.textExtensions does not, causing inconsistent
quick-preview behavior; update the code so both surfaces share a single source
of truth—either extract the markdown extensions into a shared constant/allowlist
used by isMarkdownFile and FilePreviewKindResolver.textExtensions, or add the
missing aliases (.mkd, .mkdn, .mdwn, .mdown) to
FilePreviewKindResolver.textExtensions; ensure you reference and update the
symbols isMarkdownFile and FilePreviewKindResolver.textExtensions (or the new
shared constant) so all preview entrypoints use the same list.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9d26122c-94b9-484e-b5cd-a13fda9ebf60
📒 Files selected for processing (2)
Sources/Panels/FilePreviewPanel.swiftSources/Panels/FilePreviewTextEditor.swift
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…ew panel Integrates CodeEditSourceEditor (v0.15.2) for TreeSitter-based syntax highlighting in the file preview panel, and adds a one-click markdown preview button that opens the existing MarkdownPanelView in a split. - Routes 40+ file extensions (Swift, JS/TS, Python, Go, Rust, JSON, YAML, Markdown, and more) through HighlightedFilePreviewEditor backed by CodeEditSourceEditor/TreeSitter; unsupported extensions fall back to the existing PlainFilePreviewEditor unchanged - Files over 500 KB fall back to plain editor to avoid TreeSitter latency - Language detection cached in @StateObject — runs once per panel, not on every keystroke - Dark/light syntax theme picked automatically by background luminance (VS Code-inspired palette) - Save shortcut (including chord shortcuts from KeyboardShortcutSettings) forwarded through HighlightedEditorContainerView in the AppKit responder chain - Pinch/scroll zoom handled via a local NSEvent monitor anchored to the inner scroll view; monitor tied to isVisibleInUI so inactive panels cannot steal gesture events - Focus registration via TextViewCoordinator.prepareCoordinator gives the file preview panel a real first responder for keyboard-driven panel switching - NSTextStorage bridge (HighlightedEditorBridge) keeps text in sync both directions: panel → editor on async file load, editor → panel on each edit; saves work correctly with panel.saveTextContent() - doc.richtext icon in the file preview header for .md files, next to the existing revert/save buttons - Disabled while the buffer is dirty (preview is disk-backed) - Calls workspace.openOrFocusMarkdownSplit to open/focus MarkdownPanelView in a horizontal split; falls back to NSWorkspace.shared.open on failure - CodeEditSourceEditor and CodeEditLanguages added as Swift Package deps - reload.sh documents Xcode trust path and CMUX_SKIP_PLUGIN_VALIDATION=1 opt-in for CI/headless builds (SwiftLintPlugin transitive dep)
- Add missing `mkdn` extension to `isMarkdownFile` so .mkdn files get
the markdown preview button consistently with syntax highlighting
- Replace `nonisolated(unsafe)` + `DispatchQueue.main.async` with
`Task { @mainactor [weak self] in }` for Swift-concurrency-correct
actor isolation in HighlightedEditorBridge
- Convert `FileLanguageCache` from `ObservableObject`/`@StateObject` to
a plain struct with `@State` — it never publishes changes so the class
added no value
- Extract `SyntaxLanguageDetector` to its own file to keep
FilePreviewTextEditor.swift within the single-responsibility threshold
- textViewDidChangeText: read controller.textView.string inside Task { @mainactor }
to avoid reading AppKit main-thread property before the actor hop (greptile P1)
- drawsBackground propagated into HighlightedFilePreviewEditor and theme:
transparent-background appearances now work in highlighted path (coderabbit)
- FileExternalOpenAction.openDefault used in markdown preview fallback instead
of NSWorkspace.shared.open, consistent with existing cmd-click route (coderabbit)
- Dockerfile filename check lowercased to match case-insensitively (coderabbit)
- CodeEditLanguages version constraint changed from exact to upToNextMajorVersion
to match CodeEditSourceEditor, avoiding future incompatibility when CESE updates
(coderabbit)
- @preconcurrency NSTextStorageDelegate on HighlightedEditorBridge to satisfy
Swift actor isolation while the conformance methods are all @mainactor (greptile)
- Note: CodeEditTextView.TextView is not NSTextView so panel.attachTextView cannot
be called in the highlighted path; focus registration uses attachPreviewFocus
and file-drop via NSTextView APIs remains unavailable in that path
- Zoom monitor leak: NSEvent.removeMonitor was deferred via Task { @mainactor }
with [weak self], but SwiftUI releases the coordinator synchronously when
destroy() is called, so self was already nil and the monitor leaked
permanently. Now removed synchronously in destroy(); zoomEventMonitor is
nonisolated(unsafe) because NSEvent.removeMonitor is thread-safe and the
field is written only on the main actor. (greptile P1)
- isDark now always derived from bridge.themeBackground, not the resolved
background (which is .clear when drawsBackground=false). Otherwise transparent
dark editors would pick the light syntax palette. (coderabbit + cubic)
- Markdown preview button also disabled when panel.isFileUnavailable, mirroring
the existing dirty/saving guards. (coderabbit)
- Serialize zoomEventMonitor access through NSLock so destroy() (called nonisolated) and @mainactor install/remove paths can never double-call NSEvent.removeMonitor with the same token (cubic P1). - Add markdown aliases (mkd, mkdn, mdwn, mdown) to FilePreviewKindResolver.textExtensions so they take the text path on first resolution instead of needing a sniff hop (coderabbit).
Extract FilePreviewKindResolver.markdownExtensions as the single source of truth and consume it from both FilePreviewKindResolver.textExtensions (via .union) and FilePreviewPanel.isMarkdownFile. Keeps the text-mode allowlist and the markdown-preview button in lockstep so neither can drift.
72f0290 to
c0669f8
Compare
|
@lawrencecchen — when you have a moment, would appreciate a maintainer pass on this. It's been through several rounds of bot review (CodeRabbit, cubic, greptile, Socket) all green on the current head, rebased onto latest |
manaflow-ai#4285 routes all .md file opens through Workspace.openFileSurfaces and sends them directly to MarkdownPanel (the rendered viewer), so a .md file no longer lands in FilePreviewPanel from any current entry point. The in-header markdown preview button is therefore unreachable in normal flows and would only fire on legacy session restores. Remove the button, isMarkdownFile, and owningWorkspace() helper. Keep the .mkd/.mkdn/.mdwn/.mdown additions in textExtensions inline, since upstream MarkdownPanelFileLinkResolver.isMarkdownPathLike only covers md/markdown/mkd/mdx — the long-tail aliases still benefit text-mode detection if they ever reach FilePreviewPanel. PR scope is now syntax highlighting only.
|
Scope reduction: dropped the in-header markdown preview button (commit Now that #4285 routes all This PR is now syntax highlighting only, which keeps the change tighter and easier to review. |
- Memoize SyntaxLanguageDetector.language(for:) with a static URL-keyed cache so the resourceValues file stat runs once per file instead of once per parent re-render. State(wrappedValue:) is not @autoclosure, so the FileLanguageCache constructor was firing on every keystroke even though SwiftUI discards the value after the first init (greptile P1, line 55). - Fix misleading "runs exactly once per panel lifetime" comment on FileLanguageCache. - Wrap NSEvent.addLocalMonitorForEvents handler body in MainActor.assumeIsolated. The closure accesses @MainActor-isolated properties; NSEvent local monitors fire on the main thread by contract, so this is safe at runtime and clears Swift 6 strict- concurrency complaints (greptile P1, line 193). Greptile's third P1 (missing .xcstrings entries for filePreview.openMarkdownPreview*) was stale — those strings were removed in ba58f7c with the markdown button.
|
@lawrencecchen pls take a look, don't let the bots hinder this good PR, syntax highligh is definitely a gap need to be filled. |
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
All inline findings from this automated CodeRabbit review have been addressed or resolved, and the current CodeRabbit status check is passing on the PR head.
|
cmux-reconcile: useful Usefulness verdict: Keep the syntax-highlighting/editor delta; remove stale Markdown claims from the title/linked scope. The current PR body says the Markdown preview button was removed after #4285. Its remaining contribution is TreeSitter/CodeEditSourceEditor syntax highlighting beyond the existing editable NSTextView preview. That can still be useful for #137. Do not merge or close based on the outdated title suggesting it provides Markdown viewing, which main already has. Dependency/build impact still needs normal review. Reviewed patch head: Older issue/PR tracking index — remaining scope and competing implementations are recorded there. |
Summary
NSTextViewpath unchanged.Scope change (2026-05-18)
The markdown preview button that originally shipped in this PR has been removed in light of #4285, which now routes all
.mdfile opens throughWorkspace.openFileSurfaces→MarkdownPanel(the rendered viewer). After that change,.mdfiles no longer land inFilePreviewPanelfrom any current entry point, so the in-header preview button became unreachable in normal flows. This PR is now syntax highlighting only.Related issues
Relates to:
Architecture
HighlightedFilePreviewRouter(new router, replaces the directFilePreviewTextEditorcall in the.textpreview mode):SyntaxLanguageDetector(cached in@State, not re-run on every keystroke)HighlightedFilePreviewEditor(CodeEditSourceEditor) or falls back to the existing genericFilePreviewTextEditorfor unsupported typesHighlightedEditorBridge(@MainActor, implementsTextViewCoordinator):NSTextStorageshared with CodeEditSourceEditor so async file loads are reflected immediatelytextViewDidChangeText(controller:)—NSTextStorageDelegatecannot be used becauseCodeEditTextView.setTextStorage()replaces the delegateTextViewwithFilePreviewFocusCoordinatorfor keyboard-driven panel switchingNSEventmonitor for zoom gestures (pinch/scroll/smart-magnify), anchored to the innerNSScrollViewand tied toisVisibleInUIso hidden panels cannot steal events. Token access is serialized throughNSLocksodestroy()(called nonisolated) and@MainActorinstall/remove paths can never double-callNSEvent.removeMonitor.HighlightedEditorContainerView(NSView):performKeyEquivalent(save shortcut, including chord shortcuts fromKeyboardShortcutSettings)layout()to keep the hosted SwiftUI content filling boundsSyntaxLanguageDetector(extracted to its own file per single-responsibility):CodeLanguageviadetectLanguageFrom(url:)Build notes
CodeEditSourceEditortransitively brings inSwiftLintPluginas a build-tool plugin. Two paths:IDEPackagePluginTrustTable.plist, no flag needed afterwardsCMUX_SKIP_PLUGIN_VALIDATION=1before runningreload.shAddressed review feedback
nonisolated(unsafe)+DispatchQueue.main.asyncwithTask { @MainActor [weak self] in }for correct Swift concurrency isolation inTextViewCoordinatorcallbacks.zoomEventMonitoraccess throughNSLockso the synchronous nonisolateddestroy()and the@MainActorinstall/remove paths can never double-callNSEvent.removeMonitorwith the same token.FileLanguageCachefromObservableObject/@StateObjectto a plainstructwith@State(never publishes changes).SyntaxLanguageDetectorto its own file (Sources/Panels/SyntaxLanguageDetector.swift); Dockerfile basename check is case-insensitive.CodeEditTextView.setTextStorage()replacestextStorage.delegatesoNSTextStorageDelegatecallbacks never fired; switched toTextViewCoordinator.textViewDidChangeText.drawsContentBackgroundis threaded through the highlighted path;isDarkfor the syntax palette is always derived fromthemeBackgroundso transparent dark panels keep the dark token colors.Test plan
.swift,.ts,.py, and.jsonfile — syntax colours visible, matching dark/light terminal theme