Add open-in-editor feature with Cmd+O shortcut#2
5 unresolved threads · Jump to file
- apps/renderer/src/components/ChatView.tsx:183–191 · _⚠️ Potential issue_ | _🟡 Minor_ **Only prevent default when the action will run.** Right now Cm
- apps/desktop/src/main.ts:135–153 · _⚠️ Potential issue_ | _🟠 Major_ <details> <summary>🧩 Analysis chain</summary> 🏁 Script execute
- apps/desktop/src/main.ts:143–145 · _⚠️ Potential issue_ | _🟡 Minor_ **Formatter failure in CI — please run the formatter.** CI repo
- apps/renderer/src/components/ChatView.tsx:66–69 · _⚠️ Potential issue_ | _🟡 Minor_ **Guard against stale `lastEditor` values in localStorage.** If
- apps/desktop/src/main.ts:152–152 · `src/main.ts:152` The spawned `child` process lacks an `error` event listener. If the command isn't
apps/renderer/src/components/ChatView.tsx · Earlier revision 3303d9a7 · right 183–191
Original diff
@@ -144,6 +161,47 @@ export default function ChatView() {
};
}, [isModelMenuOpen]);
+ useEffect(() => {
+ if (!isEditorMenuOpen) return;
+
+ const handleClickOutside = (event: MouseEvent) => {
+ if (!editorMenuRef.current) return;
+ if (
+ event.target instanceof Node &&
+ !editorMenuRef.current.contains(event.target)
+ ) {
+ setIsEditorMenuOpen(false);
+ }
+ };
+
+ window.addEventListener("mousedown", handleClickOutside);
+ return () => {
+ window.removeEventListener("mousedown", handleClickOutside);
+ };
+ }, [isEditorMenuOpen]);
+
+ // Cmd+O / Ctrl+O to open in last-used editor
+ useEffect(() => {
+ const handler = (e: globalThis.KeyboardEvent) => {
+ if (e.key === "o" && (e.metaKey || e.ctrlKey) && !e.shiftKey) {
+ e.preventDefault();
+ if (api && activeProject) {
+ void api.shell.openInEditor(activeProject.cwd, lastEditor);
+ }
+ }apps/renderer/src/components/ChatView.tsx · right 183–191
⚠️ Potential issue | 🟡 Minor
Only prevent default when the action will run.
Right now Cmd/Ctrl+O is swallowed even when no active project exists. Gate preventDefault() behind the same condition as the action.
✅ Suggested tweak
| 1 | const handler = (e: globalThis.KeyboardEvent) => { |
| 2 | if (e.key === "o" && (e.metaKey || e.ctrlKey) && !e.shiftKey) { |
| 3 | - e.preventDefault(); |
| 4 | - if (api && activeProject) { |
| 5 | - void api.shell.openInEditor(activeProject.cwd, lastEditor); |
| 6 | - } |
| 7 | + if (api && activeProject) { |
| 8 | + e.preventDefault(); |
| 9 | + void api.shell.openInEditor(activeProject.cwd, lastEditor); |
| 10 | + } |
| 11 | } |
| 12 | }; |
📝 Committable suggestion
‼️ IMPORTANT Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 1 | // Cmd+O / Ctrl+O to open in last-used editor |
| 2 | useEffect(() => { |
| 3 | const handler = (e: globalThis.KeyboardEvent) => { |
| 4 | if (e.key === "o" && (e.metaKey || e.ctrlKey) && !e.shiftKey) { |
| 5 | if (api && activeProject) { |
| 6 | e.preventDefault(); |
| 7 | void api.shell.openInEditor(activeProject.cwd, lastEditor); |
| 8 | } |
| 9 | } |
🤖 Prompt for AI Agents
| 1 | In `@apps/renderer/src/components/ChatView.tsx` around lines 183 - 191, The |
| 2 | keydown handler currently calls e.preventDefault() unconditionally for |
| 3 | Cmd/Ctrl+O even when the action won't run; update the handler in the useEffect |
| 4 | (function handler used for the KeyboardEvent) to only call e.preventDefault() |
| 5 | when api and activeProject are truthy and you are about to call |
| 6 | api.shell.openInEditor(activeProject.cwd, lastEditor), i.e. move or guard |
| 7 | preventDefault() behind the same condition that checks api and activeProject so |
| 8 | the key event is not swallowed when no activeProject exists. |
✅ Addressed in commit 84b71e0
apps/desktop/src/main.ts · Earlier revision 3303d9a7 · right 135–153
Original diff
@@ -125,6 +132,27 @@ function registerIpcHandlers(): void {
}),
);
+ // Shell handlers
+ ipcMain.handle(
+ IPC_CHANNELS.shellOpenInEditor,
+ async (_event, cwd: string, editor: string) => {
+ if (editor === "file-manager") {
+ await shell.openPath(cwd);
+ return;
+ }
+ const EDITOR_COMMANDS: Record<string, { command: string; args: (cwd: string) => string[] }> = {
+ cursor: { command: "cursor", args: (p) => [p] },
+ };
+ const entry = EDITOR_COMMANDS[editor];
+ if (!entry) throw new Error(`Unknown editor: ${editor}`);
+ const child = spawn(entry.command, entry.args(cwd), {
+ detached: true,
+ stdio: "ignore",
+ });
+ child.unref();
+ },apps/desktop/src/main.ts · right 135–153
⚠️ Potential issue | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
| 1 | cat -n apps/desktop/src/main.ts | sed -n '135,153p' |
Repository: pingdotgg/codething-mvp
Length of output: 848
🌐 Web query:
Electron shell.openPath() return value error handling
💡 Result:
shell.openPath(path) returns a Promise<string> that resolves (not rejects) to:
""(empty string) on success- a non-empty error message string on failure (electronjs.org)
So error handling is typically:
| 1 | import { shell } from "electron"; |
| 2 | |
| 3 | async function openFile(p: string) { |
| 4 | try { |
| 5 | const err = await shell.openPath(p); |
| 6 | if (err) { |
| 7 | // open failed; Electron returns an error message string |
| 8 | console.error("openPath failed:", err); |
| 9 | // show UI / log / fallback here |
| 10 | } |
| 11 | } catch (e) { |
| 12 | // for unexpected exceptions (e.g., bad argument types, other runtime issues) |
| 13 | console.error("openPath threw:", e); |
| 14 | } |
| 15 | } |
Key point: check the resolved string—don’t rely only on catch. (electronjs.org)
Citations:
- 1: https://www.electronjs.org/docs/latest/api/shell?utm_source=openai
- 2: https://www.electronjs.org/docs/latest/api/shell?utm_source=openai
🌐 Web query:
Node.js child_process spawn error event handling
💡 Result:
In Node.js, child_process.spawn() can fail asynchronously, so you should always attach an 'error' handler to the returned ChildProcess:
'error'event is emitted when the process could not be spawned (e.g.,ENOENT,EACCES), could not be killed, or IPC send failed. The'exit'event may or may not fire after an'error', so don’t rely on'exit'alone. [1]'close'event fires after the process ends and all stdio streams are closed; it will emit after'exit', or after'error'if the child failed to spawn. [2]
A common safe pattern is to funnel both 'error' and 'close' into a single “done once” handler:
| 1 | import { spawn } from 'node:child_process'; |
| 2 | |
| 3 | function spawnChecked(cmd, args, opts) { |
| 4 | const child = spawn(cmd, args, opts); |
| 5 | |
| 6 | let settled = false; |
| 7 | const done = (err, result) => { |
| 8 | if (settled) return; |
| 9 | settled = true; |
| 10 | if (err) throw err; // or reject a Promise / call cb(err) |
| 11 | return result; |
| 12 | }; |
| 13 | |
| 14 | child.once('error', (err) => done(err)); // spawn/kill/IPC failure |
| 15 | child.once('close', (code, signal) => { // stdio fully closed |
| 16 | if (code === 0) return done(null, { code, signal }); |
| 17 | done(Object.assign(new Error(`exit ${code ?? 'null'} (${signal ?? 'no-signal'})`), { code, signal })); |
| 18 | }); |
| 19 | |
| 20 | return child; |
| 21 | } |
Sources: Node.js ChildProcess event docs for 'error' and 'close'. [1] [2]
References
[1] Node.js docs — ChildProcess event: 'error' (nodejs.org)
[2] Node.js v22 docs — ChildProcess event: 'close' (nodejs.org)
Citations:
- 1: https://nodejs.org/download/release/v7.7.0/docs/api/child_process.html?utm_source=openai
- 2: https://nodejs.org/download/release/v22.19.0/docs/api/child_process.html?utm_source=openai
Add error handling for shell.openPath() and spawn() to prevent crashes and silent failures.shell.openPath() returns a Promise that resolves to an error string (not thrown), and spawn() emits an 'error' event asynchronously when the process cannot be spawned (e.g., missing CLI). Without handling these, the IPC handler silently fails or crashes. Also guard against empty cwd.
🛠️ Suggested fix
| 1 | - async (_event, cwd: string, editor: string) => { |
| 2 | - if (editor === "file-manager") { |
| 3 | - await shell.openPath(cwd); |
| 4 | - return; |
| 5 | - } |
| 6 | + async (_event, cwd: string, editor: string) => { |
| 7 | + if (!cwd) { |
| 8 | + throw new Error("cwd is required"); |
| 9 | + } |
| 10 | + if (editor === "file-manager") { |
| 11 | + const error = await shell.openPath(cwd); |
| 12 | + if (error) { |
| 13 | + throw new Error(error); |
| 14 | + } |
| 15 | + return; |
| 16 | + } |
| 17 | const EDITOR_COMMANDS: Record<string, { command: string; args: (cwd: string) => string[] }> = { |
| 18 | cursor: { command: "cursor", args: (p) => [p] }, |
| 19 | }; |
| 20 | const entry = EDITOR_COMMANDS[editor]; |
| 21 | if (!entry) throw new Error(`Unknown editor: ${editor}`); |
| 22 | - const child = spawn(entry.command, entry.args(cwd), { |
| 23 | - detached: true, |
| 24 | - stdio: "ignore", |
| 25 | - }); |
| 26 | - child.unref(); |
| 27 | + await new Promise<void>((resolve, reject) => { |
| 28 | + const child = spawn(entry.command, entry.args(cwd), { |
| 29 | + detached: true, |
| 30 | + stdio: "ignore", |
| 31 | + }); |
| 32 | + child.once("error", reject); |
| 33 | + child.once("spawn", () => { |
| 34 | + child.unref(); |
| 35 | + resolve(); |
| 36 | + }); |
| 37 | + }); |
| 38 | }, |
| 39 | ); |
📝 Committable suggestion
‼️ IMPORTANT Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 1 | // Shell handlers |
| 2 | ipcMain.handle( |
| 3 | IPC_CHANNELS.shellOpenInEditor, |
| 4 | async (_event, cwd: string, editor: string) => { |
| 5 | if (!cwd) { |
| 6 | throw new Error("cwd is required"); |
| 7 | } |
| 8 | if (editor === "file-manager") { |
| 9 | const error = await shell.openPath(cwd); |
| 10 | if (error) { |
| 11 | throw new Error(error); |
| 12 | } |
| 13 | return; |
| 14 | } |
| 15 | const EDITOR_COMMANDS: Record<string, { command: string; args: (cwd: string) => string[] }> = { |
| 16 | cursor: { command: "cursor", args: (p) => [p] }, |
| 17 | }; |
| 18 | const entry = EDITOR_COMMANDS[editor]; |
| 19 | if (!entry) throw new Error(`Unknown editor: ${editor}`); |
| 20 | await new Promise<void>((resolve, reject) => { |
| 21 | const child = spawn(entry.command, entry.args(cwd), { |
| 22 | detached: true, |
| 23 | stdio: "ignore", |
| 24 | }); |
| 25 | child.once("error", reject); |
| 26 | child.once("spawn", () => { |
| 27 | child.unref(); |
| 28 | resolve(); |
| 29 | }); |
| 30 | }); |
| 31 | }, |
| 32 | ); |
🤖 Prompt for AI Agents
| 1 | In `@apps/desktop/src/main.ts` around lines 135 - 153, The IPC handler for |
| 2 | ipcMain.handle should validate cwd is non-empty, await shell.openPath(cwd) and |
| 3 | check its returned string for an error (log/throw when non-empty) instead of |
| 4 | assuming success, and protect the spawn path by wrapping the spawn call in |
| 5 | try/catch and attaching an 'error' listener to the returned child (e.g., |
| 6 | child.on('error', ...) to log/throw when the CLI cannot be spawned); reference |
| 7 | EDITOR_COMMANDS and entry (from editor) to locate the spawn logic, and ensure |
| 8 | you handle and surface errors for both shell.openPath and spawn so the handler |
| 9 | doesn't crash or fail silently. |
apps/desktop/src/main.ts · Earlier revision 3303d9a7 · right 143–145
Original diff
@@ -125,6 +132,27 @@ function registerIpcHandlers(): void {
}),
);
+ // Shell handlers
+ ipcMain.handle(
+ IPC_CHANNELS.shellOpenInEditor,
+ async (_event, cwd: string, editor: string) => {
+ if (editor === "file-manager") {
+ await shell.openPath(cwd);
+ return;
+ }
+ const EDITOR_COMMANDS: Record<string, { command: string; args: (cwd: string) => string[] }> = {
+ cursor: { command: "cursor", args: (p) => [p] },
+ };apps/desktop/src/main.ts · right 143–145
⚠️ Potential issue | 🟡 Minor
Formatter failure in CI — please run the formatter.
CI reports the formatter would reformat this section. Run bun run lint after applying changes.
✨ Example formatting adjustment
| 1 | - const EDITOR_COMMANDS: Record<string, { command: string; args: (cwd: string) => string[] }> = { |
| 2 | + const EDITOR_COMMANDS: Record< |
| 3 | + string, |
| 4 | + { command: string; args: (cwd: string) => string[] } |
| 5 | + > = { |
| 6 | cursor: { command: "cursor", args: (p) => [p] }, |
| 7 | }; |
📝 Committable suggestion
‼️ IMPORTANT Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 1 | const EDITOR_COMMANDS: Record< |
| 2 | string, |
| 3 | { command: string; args: (cwd: string) => string[] } |
| 4 | > = { |
| 5 | cursor: { command: "cursor", args: (p) => [p] }, |
| 6 | }; |
🤖 Prompt for AI Agents
| 1 | In `@apps/desktop/src/main.ts` around lines 143 - 145, The CI formatter flagged |
| 2 | the EDITOR_COMMANDS block — run the project formatter (e.g., run the formatter |
| 3 | or execute `bun run lint`) to reformat the object literal; specifically ensure |
| 4 | the EDITOR_COMMANDS constant and its entry for "cursor" follow the project's |
| 5 | formatting rules (spacing, trailing commas, and indentation) so the object shape |
| 6 | Record<string, { command: string; args: (cwd: string) => string[] }> and the |
| 7 | cursor entry { command: "cursor", args: (p) => [p] } are formatted exactly as |
| 8 | the linter expects. |
✅ Addressed in commit 6204eaf
apps/renderer/src/components/ChatView.tsx · Earlier revision 3303d9a7 · right 66–69
Original diff
@@ -51,12 +63,17 @@ export default function ChatView() {
const [isSending, setIsSending] = useState(false);
const [isConnecting, setIsConnecting] = useState(false);
const [isModelMenuOpen, setIsModelMenuOpen] = useState(false);
+ const [isEditorMenuOpen, setIsEditorMenuOpen] = useState(false);
+ const [lastEditor, setLastEditor] = useState(
+ () => localStorage.getItem(LAST_EDITOR_KEY) ?? EDITORS[0].id,
+ );apps/renderer/src/components/ChatView.tsx · right 66–69
⚠️ Potential issue | 🟡 Minor
Guard against stale lastEditor values in localStorage.
If localStorage contains an unknown editor id (older versions, manual edits), the IPC handler will throw. Prefer validating against EDITORS and falling back.
🧩 Suggested fix
| 1 | - const [lastEditor, setLastEditor] = useState( |
| 2 | - () => localStorage.getItem(LAST_EDITOR_KEY) ?? EDITORS[0].id, |
| 3 | - ); |
| 4 | + const [lastEditor, setLastEditor] = useState(() => { |
| 5 | + const stored = localStorage.getItem(LAST_EDITOR_KEY); |
| 6 | + return EDITORS.some((editor) => editor.id === stored) |
| 7 | + ? stored |
| 8 | + : EDITORS[0].id; |
| 9 | + }); |
📝 Committable suggestion
‼️ IMPORTANT Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 1 | const [isEditorMenuOpen, setIsEditorMenuOpen] = useState(false); |
| 2 | const [lastEditor, setLastEditor] = useState(() => { |
| 3 | const stored = localStorage.getItem(LAST_EDITOR_KEY); |
| 4 | return EDITORS.some((editor) => editor.id === stored) |
| 5 | ? stored |
| 6 | : EDITORS[0].id; |
| 7 | }); |
🤖 Prompt for AI Agents
| 1 | In `@apps/renderer/src/components/ChatView.tsx` around lines 66 - 69, Validate the |
| 2 | value read from localStorage under LAST_EDITOR_KEY against the known EDITORS |
| 3 | list before using it as the initial state for lastEditor; replace the current |
| 4 | initializer (() => localStorage.getItem(LAST_EDITOR_KEY) ?? EDITORS[0].id) with |
| 5 | logic that reads localStorage, checks EDITORS.some(e => e.id === stored), and |
| 6 | falls back to EDITORS[0].id when the stored id is missing or not found; also |
| 7 | ensure any places that write to lastEditor (setLastEditor) persist only |
| 8 | validated ids to localStorage. |
apps/desktop/src/main.ts · Earlier revision 3303d9a7 · right 152–152
Original diff
@@ -125,6 +132,27 @@ function registerIpcHandlers(): void {
}),
);
+ // Shell handlers
+ ipcMain.handle(
+ IPC_CHANNELS.shellOpenInEditor,
+ async (_event, cwd: string, editor: string) => {
+ if (editor === "file-manager") {
+ await shell.openPath(cwd);
+ return;
+ }
+ const EDITOR_COMMANDS: Record<string, { command: string; args: (cwd: string) => string[] }> = {
+ cursor: { command: "cursor", args: (p) => [p] },
+ };
+ const entry = EDITOR_COMMANDS[editor];
+ if (!entry) throw new Error(`Unknown editor: ${editor}`);
+ const child = spawn(entry.command, entry.args(cwd), {
+ detached: true,
+ stdio: "ignore",
+ });
+ child.unref();apps/desktop/src/main.ts · right 152–152
src/main.ts:152 The spawned child process lacks an error event listener. If the command isn't found (ENOENT), the unhandled error will crash the Electron main process. Consider adding child.on('error', () => {}) or wrapping in try-catch.
✅ Addressed in 84b71e0a016acc71cf12e375a2b151ee20bc11b3