From 866626347b1b89ef091e3725490db8e549e36532 Mon Sep 17 00:00:00 2001 From: Thibault Sottiaux Date: Fri, 25 Apr 2025 09:39:24 -0700 Subject: [PATCH] fix: only allow going up in history when not already in history if input is empty (#654) \+ cleanup below input help to be "ctrl+c to exit | "/" to see commands | enter to send" now that we have command autocompletion \+ minor other drive-by code cleanups --------- Signed-off-by: Thibault Sottiaux --- .../components/chat/terminal-chat-input.tsx | 91 +++++++++---------- .../terminal-chat-input-multiline.test.tsx | 29 ------ 2 files changed, 45 insertions(+), 75 deletions(-) diff --git a/codex-cli/src/components/chat/terminal-chat-input.tsx b/codex-cli/src/components/chat/terminal-chat-input.tsx index fdf5e077..a18ee99a 100644 --- a/codex-cli/src/components/chat/terminal-chat-input.tsx +++ b/codex-cli/src/components/chat/terminal-chat-input.tsx @@ -245,30 +245,40 @@ export default function TerminalChatInput({ } if (_key.upArrow) { - // Only recall history when the caret was *already* on the very first + let moveThroughHistory = true; + + // Only use history when the caret was *already* on the very first // row *before* this key-press. const cursorRow = editorRef.current?.getRow?.() ?? 0; const wasAtFirstRow = (prevCursorRow.current ?? cursorRow) === 0; + if (!(cursorRow === 0 && wasAtFirstRow)) { + moveThroughHistory = false; + } - if (history.length > 0 && cursorRow === 0 && wasAtFirstRow) { + // Only use history if we are already in history mode or if the input is empty. + if (historyIndex == null && input.trim() !== "") { + moveThroughHistory = false; + } + + // Move through history. + if (history.length && moveThroughHistory) { + let newIndex: number; if (historyIndex == null) { const currentDraft = editorRef.current?.getText?.() ?? input; setDraftInput(currentDraft); - } - - let newIndex: number; - if (historyIndex == null) { newIndex = history.length - 1; } else { newIndex = Math.max(0, historyIndex - 1); } setHistoryIndex(newIndex); + setInput(history[newIndex]?.command ?? ""); // Re-mount the editor so it picks up the new initialText setEditorKey((k) => k + 1); - return; // we handled the key + return; // handled } - // Otherwise let the event propagate so the editor moves the caret + + // Otherwise let it propagate. } if (_key.downArrow) { @@ -339,73 +349,60 @@ export default function TerminalChatInput({ const onSubmit = useCallback( async (value: string) => { const inputValue = value.trim(); - // If the user only entered a slash, do not send a chat message + + // If the user only entered a slash, do not send a chat message. if (inputValue === "/") { setInput(""); return; } - // Skip this submit if we just autocompleted a slash command + + // Skip this submit if we just autocompleted a slash command. if (skipNextSubmit) { setSkipNextSubmit(false); return; } + if (!inputValue) { return; - } - - if (inputValue === "/history") { + } else if (inputValue === "/history") { setInput(""); openOverlay(); return; - } - - if (inputValue === "/help") { + } else if (inputValue === "/help") { setInput(""); openHelpOverlay(); return; - } - - if (inputValue === "/diff") { + } else if (inputValue === "/diff") { setInput(""); openDiffOverlay(); return; - } - - if (inputValue === "/compact") { + } else if (inputValue === "/compact") { setInput(""); onCompact(); return; - } - - if (inputValue.startsWith("/model")) { + } else if (inputValue.startsWith("/model")) { setInput(""); openModelOverlay(); return; - } - - if (inputValue.startsWith("/approval")) { + } else if (inputValue.startsWith("/approval")) { setInput(""); openApprovalOverlay(); return; - } - - if (inputValue === "q" || inputValue === ":q" || inputValue === "exit") { + } else if (inputValue === "exit") { setInput(""); - // wait one 60ms frame setTimeout(() => { app.exit(); onExit(); process.exit(0); - }, 60); + }, 60); // Wait one frame. return; } else if (inputValue === "/clear" || inputValue === "clear") { setInput(""); setSessionId(""); setLastResponseId(""); - // Clear the terminal screen (including scrollback) before resetting context - clearTerminal(); - // Emit a system notice in the chat; no raw console writes so Ink keeps control. + // Clear the terminal screen (including scrollback) before resetting context. + clearTerminal(); // Emit a system message to confirm the clear action. We *append* // it so Ink's treats it as new output and actually renders it. @@ -449,7 +446,7 @@ export default function TerminalChatInput({ await clearCommandHistory(); setHistory([]); - // Emit a system message to confirm the history clear action + // Emit a system message to confirm the history clear action. setItems((prev) => [ ...prev, { @@ -466,7 +463,7 @@ export default function TerminalChatInput({ return; } else if (inputValue === "/bug") { - // Generate a GitHub bug report URL pre‑filled with session details + // Generate a GitHub bug report URL pre‑filled with session details. setInput(""); try { @@ -519,10 +516,10 @@ export default function TerminalChatInput({ return; } else if (inputValue.startsWith("/")) { - // Handle invalid/unrecognized commands. - // Only single-word inputs starting with '/' (e.g., /command) that are not recognized are caught here. - // Any other input, including those starting with '/' but containing spaces - // (e.g., "/command arg"), will fall through and be treated as a regular prompt. + // Handle invalid/unrecognized commands. Only single-word inputs starting with '/' + // (e.g., /command) that are not recognized are caught here. Any other input, including + // those starting with '/' but containing spaces (e.g., "/command arg"), will fall through + // and be treated as a regular prompt. const trimmed = inputValue.trim(); if (/^\/\S+$/.test(trimmed)) { @@ -549,11 +546,13 @@ export default function TerminalChatInput({ // detect image file paths for dynamic inclusion const images: Array = []; let text = inputValue; + // markdown-style image syntax: ![alt](path) text = text.replace(/!\[[^\]]*?\]\(([^)]+)\)/g, (_m, p1: string) => { images.push(p1.startsWith("file://") ? fileURLToPath(p1) : p1); return ""; }); + // quoted file paths ending with common image extensions (e.g. '/path/to/img.png') text = text.replace( /['"]([^'"]+?\.(?:png|jpe?g|gif|bmp|webp|svg))['"]/gi, @@ -562,6 +561,7 @@ export default function TerminalChatInput({ return ""; }, ); + // bare file paths ending with common image extensions text = text.replace( // eslint-disable-next-line no-useless-escape @@ -578,10 +578,10 @@ export default function TerminalChatInput({ const inputItem = await createInputItem(text, images); submitInput([inputItem]); - // Get config for history persistence + // Get config for history persistence. const config = loadConfig(); - // Add to history and update state + // Add to history and update state. const updatedHistory = await addToHistory(value, history, { maxSize: config.history?.maxSize ?? 1000, saveHistory: config.history?.saveHistory ?? true, @@ -723,8 +723,7 @@ export default function TerminalChatInput({ /> ) : ( - send q or ctrl+c to exit | send "/clear" to reset | send "/help" for - commands | press enter to send | shift+enter for new line + ctrl+c to exit | "/" to see commands | enter to send {contextLeftPercent > 25 && ( <> {" — "} diff --git a/codex-cli/tests/terminal-chat-input-multiline.test.tsx b/codex-cli/tests/terminal-chat-input-multiline.test.tsx index b992cd08..ff95e5e8 100644 --- a/codex-cli/tests/terminal-chat-input-multiline.test.tsx +++ b/codex-cli/tests/terminal-chat-input-multiline.test.tsx @@ -24,35 +24,6 @@ vi.mock("../src/utils/input-utils.js", () => ({ })); describe("TerminalChatInput multiline functionality", () => { - it("renders the multiline editor component", async () => { - const props: ComponentProps = { - isNew: false, - loading: false, - submitInput: () => {}, - confirmationPrompt: null, - explanation: undefined, - submitConfirmation: () => {}, - setLastResponseId: () => {}, - setItems: () => {}, - contextLeftPercent: 50, - openOverlay: () => {}, - openDiffOverlay: () => {}, - openModelOverlay: () => {}, - openApprovalOverlay: () => {}, - openHelpOverlay: () => {}, - onCompact: () => {}, - interruptAgent: () => {}, - active: true, - thinkingSeconds: 0, - }; - - const { lastFrameStripped } = renderTui(); - const frame = lastFrameStripped(); - - // Check that the help text mentions shift+enter for new line - expect(frame).toContain("shift+enter for new line"); - }); - it("allows multiline input with shift+enter", async () => { const submitInput = vi.fn();