Replace Monaco with CodeMirror in desktop editor#2140
Merged
Conversation
Contributor
|
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:
📝 WalkthroughWalkthroughRemoves Monaco integration and theming; adds a CodeMirror 6-based CodeEditor, CodeEditorAdapter types, language loaders, theme/styling utilities, and diffs.com-based LightDiffViewer for read-only diffs. Updates save and editor-action flows to be adapter-based and updates dependency/configuration to include CodeMirror packages and remove monaco packages. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as FileViewer / FileDiffSection
participant Editor as CodeEditor (CodeMirror)
participant Adapter as CodeEditorAdapter
participant SaveHook as useFileSave
participant Backend as Electron/TRPC
User->>UI: open file / start edit
UI->>Editor: mount(value, language, readOnly)
Editor->>Adapter: expose adapter methods via editorRef
User->>Editor: edit content
Editor->>UI: onChange(updated)
UI->>SaveHook: handleSaveRaw(updated)
SaveHook->>Adapter: getValue()
SaveHook->>Backend: persist content
Backend-->>SaveHook: success
SaveHook-->>UI: onSave resolved
UI-->>User: show saved state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/components/EditorContextMenu/useEditorActions.ts (1)
13-20: Update comment to reflect CodeMirror migration.The comment still references Monaco and language service features. Consider updating it to be editor-agnostic or mention CodeMirror.
📝 Suggested update
/** * Hook that creates all editor action handlers for the context menu. * Shared between FileEditorContextMenu and DiffViewer. * - * Note: Standalone Monaco editor doesn't include language service features - * like Go to Definition, References, Rename, etc. Those require language - * providers to be registered. We only expose actions that are actually available. + * Note: The editor abstraction (CodeEditorAdapter) exposes a subset of actions + * that are commonly available. Advanced language features like Go to Definition, + * References, Rename, etc. are not included as they require additional setup. */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/components/EditorContextMenu/useEditorActions.ts` around lines 13 - 20, Update the top-of-file comment in useEditorActions.ts to remove Monaco-specific wording and either mention CodeMirror or make it editor-agnostic; replace lines referencing "Standalone Monaco editor" and "language service features like Go to Definition, References, Rename" with a brief note that language features depend on registered language providers in the host editor (e.g., CodeMirror) or that this hook only exposes actions actually available from the current editor implementation, so consumers know the hook is not tied to Monaco-specific capabilities.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/FileViewerContent/FileViewerContent.tsx (1)
148-163: Refs in dependency array are stable and unnecessary.
draftContentRefandoriginalContentRefare refs created withuseRefand their identity is stable across renders. Including them in the dependency array doesn't cause bugs but is unnecessary since refs don't trigger re-renders.♻️ Optional cleanup
useEffect(() => { if (viewMode !== "raw") return; if (isLoadingRaw) return; if (!rawFileData?.ok) return; if (draftContentRef.current) return; originalContentRef.current = rawFileData.content; setIsDirty(false); -}, [ - viewMode, - isLoadingRaw, - rawFileData, - draftContentRef, - originalContentRef, - setIsDirty, -]); + // eslint-disable-next-line react-hooks/exhaustive-deps +}, [viewMode, isLoadingRaw, rawFileData, setIsDirty]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/FileViewerContent/FileViewerContent.tsx` around lines 148 - 163, The useEffect dependency array includes stable ref objects draftContentRef and originalContentRef unnecessarily; remove draftContentRef and originalContentRef from the dependency array in the useEffect that sets originalContentRef.current (the effect that checks viewMode, isLoadingRaw, rawFileData, and draftContentRef.current) so only reactive values (viewMode, isLoadingRaw, rawFileData, setIsDirty) remain, keeping the logic inside the effect unchanged and relying on the stable identity of the refs.apps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/loadLanguageSupport.ts (1)
6-14: Consider supporting explicittsx/jsxlanguage identifiers.The current implementation enables JSX for both
typescriptandjavascript, which is generally fine. However, file detection utilities might returntsxorjsxas distinct language identifiers. Consider adding explicit case handlers:case "typescript": +case "tsx": case "javascript": +case "jsx": {This ensures consistency with language detection that may distinguish between
.ts/.tsxand.js/.jsxfiles.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/loadLanguageSupport.ts` around lines 6 - 14, The switch branch handling language (the switch on variable `language`) only covers "typescript" and "javascript"; update it to also handle "tsx" and "jsx" as explicit cases so file detectors returning those identifiers map correctly to the same support — for "tsx" treat as typescript with jsx enabled, for "jsx" treat as javascript with jsx enabled; modify the case labels around the existing import of `@codemirror/lang-javascript` and the call to `javascript({ typescript: language === "typescript", jsx: true })` so it also returns the correct config when `language` is "tsx" or "jsx".apps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/CodeEditor.tsx (1)
89-123: Clipboard operations silently swallow errors.The
cut()andpaste()methods usevoid navigator.clipboard...then()which means any clipboard access failures (e.g., permissions denied) will result in unhandled promise rejections.🛡️ Suggested improvement with error handling
cut() { if (view.state.readOnly) return; const selection = view.state.selection.main; if (selection.empty) return; const text = view.state.sliceDoc(selection.from, selection.to); - void navigator.clipboard.writeText(text).then(() => { + navigator.clipboard.writeText(text).then(() => { view.dispatch({ changes: { from: selection.from, to: selection.to, insert: "" }, }); - }); + }).catch((err) => { + console.error("Failed to cut to clipboard:", err); + }); },Similar pattern for
paste().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/CodeEditor.tsx` around lines 89 - 123, The clipboard operations (cut, copy, paste) swallow promise errors via `void ...then()` and can produce unhandled rejections; update `cut()`, `copy()`, and `paste()` to chain a `.catch()` (or use async/await with try/catch) on `navigator.clipboard.writeText` / `navigator.clipboard.readText` to handle permission/IO failures, log the error (e.g., console.error or the component logger), and ensure `cut()` only dispatches the deletion on successful write and `paste()` only inserts on successful read to avoid corrupting editor state; keep references to the existing methods (`cut`, `copy`, `paste`) and `navigator.clipboard.writeText` / `navigator.clipboard.readText` when making the changes.
🤖 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/desktop/docs/CODE_EDITOR_MIGRATION_PLAN.md`:
- Around line 107-115: Update the migration plan to make the rollout gate
measurable by adding explicit thresholds and capture methods for each baseline
metric (desktop app startup time, time to first file open, memory after app
launch, memory after opening a large file, typing latency in the raw editor) and
for the "lazy-loads Monaco" experiment: specify how each metric is measured
(tooling e.g., performance marks, profilers, sample size, OS/hardware cohorts),
the exact numeric pass/fail thresholds or percent-change targets for Phase 5,
the measurement window and aggregation method (median/95th percentile), and
where results are recorded; apply the same explicitmetric-and-threshold wording
to the Phase 5 section and the duplicated guidance referenced around lines
220-229 so rollback decisions are unambiguous.
- Around line 82-98: The CodeEditorAdapter interface lacks a disposal contract
so CodeMirror-based implementations cannot call EditorView.destroy() to clean up
view plugins, global handlers, and observers; add a dispose(): void method to
CodeEditorAdapter and update all adapter implementations (the CodeMirror-backed
adapter in particular) to call EditorView.destroy() from their dispose() to
ensure proper teardown and prevent memory leaks, and update any consumers to
call adapter.dispose() when unmounting or replacing editor instances.
In `@apps/desktop/package.json`:
- Around line 42-62: The package.json has a mismatched CodeMirror core version
(package "@codemirror/core" pinned to ^6.0.2) versus newer ecosystem packages
like "@codemirror/view", "@codemirror/commands", "@codemirror/language",
"@codemirror/state", and "@codemirror/search"; update the "@codemirror/core"
dependency to a compatible release (e.g. ^6.65.7) to align versions, run your
package manager (npm/yarn/pnpm install) to update lockfile, and run the
app/tests to verify there are no runtime or typing regressions.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ChangesContent/components/FileDiffSection/FileDiffSection.tsx`:
- Around line 302-305: detectLanguage(file.path) returns values like
"plaintext", "shell", "dockerfile", "makefile", "toml", and "graphql" but
loadLanguageSupport currently returns null for those, causing CodeEditor (where
value is set via editedContent ?? diffData.modified) to lose syntax
highlighting; update loadLanguageSupport to map those detectLanguage outputs to
the appropriate CodeMirror language support modules (or sensible fallbacks) so
that when CodeEditor is rendered it receives a non-null language mode for the
symbols returned by detectLanguage; specifically add mappings for "plaintext",
"shell", "dockerfile", "makefile", "toml", and "graphql" in the
loadLanguageSupport implementation and ensure the function exports/returns the
matching support so CodeEditor can apply highlighting.
- Around line 233-236: The useEffect currently resets editedContent whenever
diffData changes, discarding in-progress edits; change the initialization to run
only once when entering edit mode or when switching files: update the effect to
depend on isEditing plus a stable file identifier from diffData (e.g.,
diffData.filePath or diffData.id) or use a local ref/flag that marks the edit
session, and call setEditedContent(diffData.modified) only when isEditing
becomes true or the file identifier changes, not on every diffData update; keep
the effect targeting the existing useEffect, isEditing, diffData,
setEditedContent and editedContent logic so in-session edits are preserved.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/CodeEditor.tsx`:
- Around line 89-123: The clipboard operations (cut, copy, paste) swallow
promise errors via `void ...then()` and can produce unhandled rejections; update
`cut()`, `copy()`, and `paste()` to chain a `.catch()` (or use async/await with
try/catch) on `navigator.clipboard.writeText` / `navigator.clipboard.readText`
to handle permission/IO failures, log the error (e.g., console.error or the
component logger), and ensure `cut()` only dispatches the deletion on successful
write and `paste()` only inserts on successful read to avoid corrupting editor
state; keep references to the existing methods (`cut`, `copy`, `paste`) and
`navigator.clipboard.writeText` / `navigator.clipboard.readText` when making the
changes.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/loadLanguageSupport.ts`:
- Around line 6-14: The switch branch handling language (the switch on variable
`language`) only covers "typescript" and "javascript"; update it to also handle
"tsx" and "jsx" as explicit cases so file detectors returning those identifiers
map correctly to the same support — for "tsx" treat as typescript with jsx
enabled, for "jsx" treat as javascript with jsx enabled; modify the case labels
around the existing import of `@codemirror/lang-javascript` and the call to
`javascript({ typescript: language === "typescript", jsx: true })` so it also
returns the correct config when `language` is "tsx" or "jsx".
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/components/EditorContextMenu/useEditorActions.ts`:
- Around line 13-20: Update the top-of-file comment in useEditorActions.ts to
remove Monaco-specific wording and either mention CodeMirror or make it
editor-agnostic; replace lines referencing "Standalone Monaco editor" and
"language service features like Go to Definition, References, Rename" with a
brief note that language features depend on registered language providers in the
host editor (e.g., CodeMirror) or that this hook only exposes actions actually
available from the current editor implementation, so consumers know the hook is
not tied to Monaco-specific capabilities.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/FileViewerContent/FileViewerContent.tsx`:
- Around line 148-163: The useEffect dependency array includes stable ref
objects draftContentRef and originalContentRef unnecessarily; remove
draftContentRef and originalContentRef from the dependency array in the
useEffect that sets originalContentRef.current (the effect that checks viewMode,
isLoadingRaw, rawFileData, and draftContentRef.current) so only reactive values
(viewMode, isLoadingRaw, rawFileData, setIsDirty) remain, keeping the logic
inside the effect unchanged and relying on the stable identity of the refs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d7f40fd4-e349-4fe1-84cc-a31c5ac9e18f
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (29)
apps/desktop/docs/CODE_EDITOR_MIGRATION_PLAN.mdapps/desktop/electron.vite.config.tsapps/desktop/package.jsonapps/desktop/src/renderer/providers/MonacoProvider/MonacoProvider.tsxapps/desktop/src/renderer/providers/MonacoProvider/index.tsapps/desktop/src/renderer/routes/-layout.tsxapps/desktop/src/renderer/routes/_authenticated/settings/appearance/components/AppearanceSettings/components/FontSettingSection/FontSettingSection.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ChangesContent/components/DiffViewer/DiffViewer.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ChangesContent/components/DiffViewer/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ChangesContent/components/FileDiffSection/FileDiffSection.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ChangesContent/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/FileViewerPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/FileEditorContextMenu/FileEditorContextMenu.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/FileViewerContent/FileViewerContent.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/hooks/useFileSave/useFileSave.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/components/CodeEditorAdapter/CodeEditorAdapter.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/components/CodeEditorAdapter/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/components/EditorContextMenu/editor-actions.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/components/EditorContextMenu/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/components/EditorContextMenu/useEditorActions.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/components/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/CodeEditor.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/createCodeMirrorTheme.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/loadLanguageSupport.tsapps/desktop/src/renderer/stores/theme/index.tsapps/desktop/src/renderer/stores/theme/store.tsapps/desktop/src/renderer/stores/theme/utils/index.tsapps/desktop/src/renderer/stores/theme/utils/monaco-theme.ts
💤 Files with no reviewable changes (11)
- apps/desktop/electron.vite.config.ts
- apps/desktop/src/renderer/stores/theme/utils/index.ts
- apps/desktop/src/renderer/stores/theme/utils/monaco-theme.ts
- apps/desktop/src/renderer/stores/theme/index.ts
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ChangesContent/components/DiffViewer/DiffViewer.tsx
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/components/EditorContextMenu/editor-actions.ts
- apps/desktop/src/renderer/providers/MonacoProvider/index.ts
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ChangesContent/index.ts
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ChangesContent/components/DiffViewer/index.ts
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/components/EditorContextMenu/index.ts
- apps/desktop/src/renderer/providers/MonacoProvider/MonacoProvider.tsx
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
6 issues found across 30 files
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="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ChangesContent/components/FileDiffSection/FileDiffSection.tsx">
<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ChangesContent/components/FileDiffSection/FileDiffSection.tsx:233">
P2: This effect overwrites `editedContent` on every `diffData` refetch while editing, which can clobber in-progress edits when `getFileContents` is invalidated elsewhere. Only initialize the editor content when entering edit mode or when no edits exist, and clear it when leaving edit mode to avoid losing user changes.
(Based on your team's feedback about narrowing React effect dependencies to required fields.) [FEEDBACK_USED]</violation>
</file>
<file name="apps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/loadLanguageSupport.ts">
<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/loadLanguageSupport.ts:71">
P2: The language loader currently falls back to `null` for several language IDs emitted by `detectLanguage`, which causes a user-visible syntax-highlighting regression for those file types. Add support (or normalization) for the missing language keys before defaulting to `null`.</violation>
</file>
<file name="apps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/CodeEditor.tsx">
<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/CodeEditor.tsx:95">
P1: Delete the text synchronously before writing to the clipboard to prevent a race condition, and add a catch block for the promise.
(Based on your team's feedback about handling async calls with proper await and catch.) [FEEDBACK_USED]</violation>
<violation number="2" location="apps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/CodeEditor.tsx:105">
P2: Add a catch block to handle potential promise rejections from clipboard operations.
(Based on your team's feedback about handling async calls with proper await and catch.) [FEEDBACK_USED]</violation>
<violation number="3" location="apps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/CodeEditor.tsx:112">
P2: Add a catch block to handle potential promise rejections from clipboard read operations.
(Based on your team's feedback about handling async calls with proper await and catch.) [FEEDBACK_USED]</violation>
<violation number="4" location="apps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/CodeEditor.tsx:333">
P2: Add a catch block to handle potential promise rejections from the async language support loader.
(Based on your team's feedback about handling async calls with proper await and catch.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/FileViewerContent/FileViewerContent.tsx (1)
14-16: Switch these new imports to the configured alias.This file already uses
renderer/...andshared/...aliases. Keeping the newWorkspaceViewimports on the alias avoids brittle../../../../../../paths.As per coding guidelines `apps/desktop/**/*.{ts,tsx}: Use alias as defined in tsconfig.json when possible`.♻️ Suggested cleanup
-import { LightDiffViewer } from "../../../../../../ChangesContent/components/LightDiffViewer"; -import { CodeEditor } from "../../../../../../components/CodeEditor"; -import type { CodeEditorAdapter } from "../../../../../components"; +import { LightDiffViewer } from "renderer/screens/main/components/WorkspaceView/ChangesContent/components/LightDiffViewer"; +import { CodeEditor } from "renderer/screens/main/components/WorkspaceView/components/CodeEditor"; +import type { CodeEditorAdapter } from "renderer/screens/main/components/WorkspaceView/ContentView/components";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/FileViewerContent/FileViewerContent.tsx` around lines 14 - 16, The imports for LightDiffViewer, CodeEditor and CodeEditorAdapter use long relative paths; switch them to the project's configured path aliases (e.g., use the existing renderer/... or shared/... aliases used elsewhere) so the imports for LightDiffViewer, CodeEditor and CodeEditorAdapter reference their modules via the tsconfig aliases instead of "../../../../../../" relative paths, updating the import specifiers accordingly and keeping the same named imports.
🤖 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/desktop/src/renderer/screens/main/components/WorkspaceView/ChangesContent/components/LightDiffViewer/LightDiffViewer.tsx`:
- Around line 37-40: The persisted editorFontSize may be a string with units, so
normalize it before passing to getDiffViewerStyle: read
fontSettings?.editorFontSize, parse/strip non-numeric content (e.g. parseFloat
after trimming units) and only pass a numeric value (or undefined if parsing
fails) into getDiffViewerStyle; update the diffStyle construction that uses
fontSettings?.editorFontSize so malformed strings won't produce invalid CSS
variables for --diffs-font-size / --diffs-line-height.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/CodeEditor.tsx`:
- Around line 84-118: The clipboard operations in the cut, copy, and paste
methods may reject and currently create unhandled promise rejections; update the
cut, copy, and paste functions in CodeEditor.tsx to handle promise rejections
from navigator.clipboard.writeText and navigator.clipboard.readText by adding
.catch handlers (or using try/catch with async/await) that log the error via the
existing logger or view/console and gracefully avoid dispatching changes on
failure (for cut/paste ensure you only call view.dispatch after successful
clipboard write/read); references: cut(), copy(), paste(), view.dispatch(),
EditorSelection.cursor().
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/FileViewerContent/FileViewerContent.tsx`:
- Around line 148-155: The effect currently treats a falsy empty-string draft as
"no draft" due to the truthy guard on draftContentRef.current; change that guard
to only check for null/undefined so an empty string is treated as a real draft
(i.e., replace the truthy check on draftContentRef.current with a null/undefined
check such as using != null). Update the effect around viewMode, isLoadingRaw,
rawFileData, draftContentRef, originalContentRef, and setIsDirty so the effect
returns when a draft exists even if it's an empty string.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/FileViewerContent/FileViewerContent.tsx`:
- Around line 14-16: The imports for LightDiffViewer, CodeEditor and
CodeEditorAdapter use long relative paths; switch them to the project's
configured path aliases (e.g., use the existing renderer/... or shared/...
aliases used elsewhere) so the imports for LightDiffViewer, CodeEditor and
CodeEditorAdapter reference their modules via the tsconfig aliases instead of
"../../../../../../" relative paths, updating the import specifiers accordingly
and keeping the same named imports.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c4886e62-4232-407f-b73e-f5b5ee31a03c
📒 Files selected for processing (4)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ChangesContent/components/LightDiffViewer/LightDiffViewer.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/FileViewerContent/FileViewerContent.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/CodeEditor.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/utils/code-theme.ts
Contributor
There was a problem hiding this comment.
1 issue found across 4 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="apps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/createCodeMirrorTheme.ts">
<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/createCodeMirrorTheme.ts:12">
P3: The midnight palette is duplicated here even though the same color set already exists in utils/code-theme.ts. This adds a second source of truth and risks the editor and diff viewer getting out of sync when colors change. Consider exporting a shared palette and reusing it.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ChangesContent/components/LightDiffViewer/LightDiffViewer.tsx (1)
32-35:⚠️ Potential issue | 🟡 MinorNormalize
editorFontSizebefore buildingdiffStyle.Line 34 still forwards the persisted setting verbatim. If
editorFontSizeis stored as a string or includes units,getDiffViewerStylecan end up generating invalid CSS values.💡 Suggested change
+ const parsedEditorFontSize = + fontSettings?.editorFontSize == null + ? undefined + : Number.parseFloat(String(fontSettings.editorFontSize)); const diffStyle = getDiffViewerStyle({ fontFamily: fontSettings?.editorFontFamily ?? undefined, - fontSize: fontSettings?.editorFontSize ?? undefined, + fontSize: + parsedEditorFontSize != null && Number.isFinite(parsedEditorFontSize) + ? parsedEditorFontSize + : undefined, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ChangesContent/components/LightDiffViewer/LightDiffViewer.tsx` around lines 32 - 35, Normalize fontSettings?.editorFontSize before calling getDiffViewerStyle: parse/coerce fontSettings?.editorFontSize (handle numbers, numeric strings, and strings with units) into a valid CSS value (e.g., a number or a "NNpx" string) with a sensible fallback if parsing fails, then pass that normalized value into getDiffViewerStyle when constructing diffStyle; update the diffStyle initialization that currently uses fontSettings?.editorFontSize verbatim to use the normalized value instead.apps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/CodeEditor.tsx (1)
83-117:⚠️ Potential issue | 🟡 MinorHandle clipboard failures before mutating the document.
navigator.clipboard.*can reject or be unavailable, andvoiddoes not consume those failures. That leaves unhandled rejections in the renderer, andcut/pasteshould only dispatch after a successful clipboard operation.Proposed fix
cut() { if (view.state.readOnly) return; + const clipboard = navigator.clipboard; + if (!clipboard) return; const selection = view.state.selection.main; if (selection.empty) return; const text = view.state.sliceDoc(selection.from, selection.to); - void navigator.clipboard.writeText(text).then(() => { - view.dispatch({ - changes: { from: selection.from, to: selection.to, insert: "" }, - }); - }); + void clipboard + .writeText(text) + .then(() => { + view.dispatch({ + changes: { from: selection.from, to: selection.to, insert: "" }, + }); + }) + .catch((error) => { + console.error("Failed to write to clipboard", error); + }); }, copy() { + const clipboard = navigator.clipboard; + if (!clipboard) return; const selection = view.state.selection.main; if (selection.empty) return; - void navigator.clipboard.writeText( - view.state.sliceDoc(selection.from, selection.to), - ); + void clipboard + .writeText(view.state.sliceDoc(selection.from, selection.to)) + .catch((error) => { + console.error("Failed to write to clipboard", error); + }); }, paste() { if (view.state.readOnly) return; + const clipboard = navigator.clipboard; + if (!clipboard) return; - void navigator.clipboard.readText().then((text) => { - const selection = view.state.selection.main; - view.dispatch({ - changes: { - from: selection.from, - to: selection.to, - insert: text, - }, - selection: EditorSelection.cursor(selection.from + text.length), - }); - }); + void clipboard + .readText() + .then((text) => { + const selection = view.state.selection.main; + view.dispatch({ + changes: { + from: selection.from, + to: selection.to, + insert: text, + }, + selection: EditorSelection.cursor(selection.from + text.length), + }); + }) + .catch((error) => { + console.error("Failed to read from clipboard", error); + }); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/CodeEditor.tsx` around lines 83 - 117, The cut/copy/paste handlers (cut, copy, paste) currently call navigator.clipboard.* and mutate the editor without handling rejections; change them to first verify navigator.clipboard is available and wait for the clipboard promise to resolve successfully (use async/await or .then/.catch) before calling view.dispatch (for cut/paste) or returning (for copy); on failure do not modify the document and surface/log the error (e.g., process to console or a logger) and ensure EditorSelection.cursor is only applied after a successful readText resolution.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ChangesContent/components/LightDiffViewer/LightDiffViewer.tsx (1)
6-6: Use therenderer/...alias forcode-theme.This import is still using a brittle relative path even though the file already uses the
renderer/...alias style on Line 4. Switching this one too will make the import more stable when this component moves.♻️ Suggested change
-import { getDiffsTheme, getDiffViewerStyle } from "../../../utils/code-theme"; +import { getDiffsTheme, getDiffViewerStyle } from "renderer/screens/main/components/WorkspaceView/utils/code-theme";As per coding guidelines,
apps/desktop/**/*.{ts,tsx}: Use alias as defined intsconfig.jsonwhen possible.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ChangesContent/components/LightDiffViewer/LightDiffViewer.tsx` at line 6, Replace the brittle relative import of getDiffsTheme and getDiffViewerStyle with the project alias import used elsewhere in this module (use the renderer/... alias instead of "../../../utils/code-theme"); update the import statement that references getDiffsTheme and getDiffViewerStyle to import from the aliased module path so the component uses the renderer alias for code-theme.
🤖 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/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/CodeEditor.tsx`:
- Around line 285-296: Handle rejections from loadLanguageSupport by catching
errors, logging the failure, and forcing the language compartment to be
reconfigured to an empty array so highlighting is not left stale; specifically,
update the useEffect where loadLanguageSupport(language) is awaited (the call to
loadLanguageSupport) to add a .catch() (or wrap in try/catch for the async flow)
that calls process/data logger with the error and then, using viewRef.current
and languageCompartment, dispatch languageCompartment.reconfigure([]) (same
dispatch path used in the success branch) to clear the extension on failure.
---
Duplicate comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ChangesContent/components/LightDiffViewer/LightDiffViewer.tsx`:
- Around line 32-35: Normalize fontSettings?.editorFontSize before calling
getDiffViewerStyle: parse/coerce fontSettings?.editorFontSize (handle numbers,
numeric strings, and strings with units) into a valid CSS value (e.g., a number
or a "NNpx" string) with a sensible fallback if parsing fails, then pass that
normalized value into getDiffViewerStyle when constructing diffStyle; update the
diffStyle initialization that currently uses fontSettings?.editorFontSize
verbatim to use the normalized value instead.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/CodeEditor.tsx`:
- Around line 83-117: The cut/copy/paste handlers (cut, copy, paste) currently
call navigator.clipboard.* and mutate the editor without handling rejections;
change them to first verify navigator.clipboard is available and wait for the
clipboard promise to resolve successfully (use async/await or .then/.catch)
before calling view.dispatch (for cut/paste) or returning (for copy); on failure
do not modify the document and surface/log the error (e.g., process to console
or a logger) and ensure EditorSelection.cursor is only applied after a
successful readText resolution.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ChangesContent/components/LightDiffViewer/LightDiffViewer.tsx`:
- Line 6: Replace the brittle relative import of getDiffsTheme and
getDiffViewerStyle with the project alias import used elsewhere in this module
(use the renderer/... alias instead of "../../../utils/code-theme"); update the
import statement that references getDiffsTheme and getDiffViewerStyle to import
from the aliased module path so the component uses the renderer alias for
code-theme.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 607030ec-b5ba-45a4-98f9-558ba8179e0f
📒 Files selected for processing (4)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ChangesContent/components/LightDiffViewer/LightDiffViewer.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/CodeEditor.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/createCodeMirrorTheme.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/utils/code-theme.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/utils/code-theme.ts
Contributor
There was a problem hiding this comment.
1 issue found across 2 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="apps/desktop/src/renderer/screens/main/components/WorkspaceView/utils/code-theme.ts">
<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceView/utils/code-theme.ts:37">
P2: `oneDark` is the full One Dark **theme** (editor chrome + syntax highlighting), not just syntax colors. It will inject competing styles for background, gutters, selection, cursor, etc. that conflict with `createCodeMirrorTheme()` applied in the same compartment. Use `syntaxHighlighting(oneDarkHighlightStyle)` to get only the token colors without the editor-chrome overrides.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
1 issue found across 10 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="apps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/streamLanguages.ts">
<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/streamLanguages.ts:174">
P2: The Makefile directive keywords are duplicated—once in the `MAKEFILE_DIRECTIVES` Set and again as a hard-coded regex literal inside the `stream.sol()` branch. Build the regex from the Set so the list is maintained in one place.
(Based on your team's feedback about avoiding duplicating business logic in multiple components.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ChangesContent/components/LightDiffViewer/LightDiffViewer.tsx (1)
35-38:⚠️ Potential issue | 🟡 MinorNormalize
editorFontSizebefore buildingdiffStyle.This still passes the persisted font size through verbatim. If the setting is stored as a string,
getDiffViewerStylecan emit invalid--diffs-font-size/--diffs-line-heightvalues here; parse it to a finite number first.Suggested fix
+ const parsedEditorFontSize = + typeof fontSettings?.editorFontSize === "number" + ? fontSettings.editorFontSize + : typeof fontSettings?.editorFontSize === "string" + ? Number.parseFloat(fontSettings.editorFontSize) + : Number.NaN; const diffStyle = getDiffViewerStyle({ fontFamily: fontSettings?.editorFontFamily ?? undefined, - fontSize: fontSettings?.editorFontSize ?? undefined, + fontSize: Number.isFinite(parsedEditorFontSize) + ? parsedEditorFontSize + : undefined, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ChangesContent/components/LightDiffViewer/LightDiffViewer.tsx` around lines 35 - 38, The current call to getDiffViewerStyle passes fontSettings?.editorFontSize verbatim, which can be a string; normalize it first by parsing fontSettings?.editorFontSize to a Number (e.g., parseFloat/Number) and verify Number.isFinite on the result, then pass the finite numeric value (or undefined fallback) into getDiffViewerStyle so diffStyle and CSS variables like --diffs-font-size/--diffs-line-height receive valid numeric values; update the code that builds diffStyle (the diffStyle variable and its call to getDiffViewerStyle) to use the normalized value.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/CodeEditor.tsx (1)
29-30: Userenderer/...aliases for these cross-workspace imports.These deep relative imports are harder to maintain and don't match the alias-based import style already used in this file.
Suggested refactor
-import type { CodeEditorAdapter } from "../../ContentView/components"; -import { getCodeSyntaxHighlighting } from "../../utils/code-theme"; +import type { CodeEditorAdapter } from "renderer/screens/main/components/WorkspaceView/ContentView/components"; +import { getCodeSyntaxHighlighting } from "renderer/screens/main/components/WorkspaceView/utils/code-theme";As per coding guidelines "
apps/desktop/**/*.{ts,tsx}: Use alias as defined intsconfig.jsonwhen possible".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/CodeEditor.tsx` around lines 29 - 30, The imports in CodeEditor.tsx use deep relative paths; update the two imports so they use the renderer alias form instead of "../../...". Replace the import of CodeEditorAdapter (currently from "../../ContentView/components") with the alias import from "renderer/ContentView/components" and replace getCodeSyntaxHighlighting (currently from "../../utils/code-theme") with "renderer/utils/code-theme" so the file follows the tsconfig.json alias conventions and keeps cross-workspace imports consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ChangesContent/components/LightDiffViewer/LightDiffViewer.tsx`:
- Around line 35-38: The current call to getDiffViewerStyle passes
fontSettings?.editorFontSize verbatim, which can be a string; normalize it first
by parsing fontSettings?.editorFontSize to a Number (e.g., parseFloat/Number)
and verify Number.isFinite on the result, then pass the finite numeric value (or
undefined fallback) into getDiffViewerStyle so diffStyle and CSS variables like
--diffs-font-size/--diffs-line-height receive valid numeric values; update the
code that builds diffStyle (the diffStyle variable and its call to
getDiffViewerStyle) to use the normalized value.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/CodeEditor.tsx`:
- Around line 29-30: The imports in CodeEditor.tsx use deep relative paths;
update the two imports so they use the renderer alias form instead of
"../../...". Replace the import of CodeEditorAdapter (currently from
"../../ContentView/components") with the alias import from
"renderer/ContentView/components" and replace getCodeSyntaxHighlighting
(currently from "../../utils/code-theme") with "renderer/utils/code-theme" so
the file follows the tsconfig.json alias conventions and keeps cross-workspace
imports consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9dddc2d8-0919-4321-b7a3-162900f203a8
📒 Files selected for processing (10)
apps/desktop/src/renderer/routes/_authenticated/settings/appearance/components/AppearanceSettings/components/FontSettingSection/FontSettingSection.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ChangesContent/components/FileDiffSection/FileDiffSection.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ChangesContent/components/LightDiffViewer/LightDiffViewer.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/FileViewerContent/FileViewerContent.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/CodeEditor.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/constants.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/createCodeMirrorTheme.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/loadLanguageSupport.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/streamLanguages.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/utils/code-theme.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/desktop/src/renderer/routes/_authenticated/settings/appearance/components/AppearanceSettings/components/FontSettingSection/FontSettingSection.tsx
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/loadLanguageSupport.ts
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/utils/code-theme.ts
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/createCodeMirrorTheme.ts
This was referenced Mar 12, 2026
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
Verification
bun run generate:routesbun x biome check --writeon touched filesbun x tsc --noEmit --pretty false -p apps/desktop/tsconfig.json(still fails on a pre-existing unrelated error inapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/ChangesView.tsx:511)Summary by cubic
Replaced Monaco with CodeMirror 6 for raw editing and moved diffs to our diffs.com viewer to cut startup cost, memory, and typing latency. Unified syntax to One Dark on the Midnight theme with darker changed regions, tighter selections, and editor/diff fonts following Appearance settings.
New Features
Bug Fixes
Written for commit cea6205. Summary will update on new commits.
Summary by CodeRabbit
New Features
Documentation
Changes
Removed