From 985fd44ec0d4c9aba664f61f6a49bd05fb8187c4 Mon Sep 17 00:00:00 2001
From: Fouad Matin <169186268+fouad-openai@users.noreply.github.com>
Date: Wed, 30 Apr 2025 17:17:13 -0700
Subject: [PATCH] fix: input keyboard shortcut opt+delete (#685)

---
 codex-cli/src/text-buffer.ts             | 50 +++++++++++++++++++++++-
 codex-cli/tests/text-buffer-word.test.ts | 25 +++++++++---
 2 files changed, 67 insertions(+), 8 deletions(-)

diff --git a/codex-cli/src/text-buffer.ts b/codex-cli/src/text-buffer.ts
index 4869b18c..057996dd 100644
--- a/codex-cli/src/text-buffer.ts
+++ b/codex-cli/src/text-buffer.ts
@@ -525,6 +525,22 @@ export default class TextBuffer {
       end++;
     }
 
+    /*
+     * After consuming the actual word we also want to swallow any immediate
+     * separator run that *follows* it so that a forward word-delete mirrors
+     * the behaviour of common shells/editors (and matches the expectations
+     * encoded in our test-suite).
+     *
+     * Example – given the text "foo bar baz" and the caret placed at the
+     * beginning of "bar" (index 4) we want Alt+Delete to turn the string
+     * into "foo␠baz" (single space).  Without this extra loop we would stop
+     * right before the separating space, producing "foo␠␠baz".
+     */
+
+    while (end < arr.length && !isWordChar(arr[end])) {
+      end++;
+    }
+
     this.lines[this.cursorRow] =
       cpSlice(line, 0, this.cursorCol) + cpSlice(line, end);
     // caret stays in place
@@ -859,12 +875,42 @@ export default class TextBuffer {
     // no `key.backspace` flag set.  Treat that byte exactly like an ordinary
     // Backspace for parity with textarea.rs and to make interactive tests
     // feedable through the simpler `(ch, {}, vp)` path.
+    // ------------------------------------------------------------------
+    //  Word-wise deletions
+    //
+    //  macOS (and many terminals on Linux/BSD) map the physical “Delete” key
+    //  to a *backspace* operation – emitting either the raw DEL (0x7f) byte
+    //  or setting `key.backspace = true` in Ink’s parsed event.  Holding the
+    //  Option/Alt modifier therefore *also* sends backspace semantics even
+    //  though users colloquially refer to the shortcut as “⌥+Delete”.
+    //
+    //  Historically we treated **modifier + Delete** as a *forward* word
+    //  deletion.  This behaviour, however, diverges from the default found
+    //  in shells (zsh, bash, fish, etc.) and native macOS text fields where
+    //  ⌥+Delete removes the word *to the left* of the caret.  Update the
+    //  mapping so that both
+    //
+    //    • ⌥/Alt/Meta + Backspace  and
+    //    • ⌥/Alt/Meta + Delete
+    //
+    //  perform a **backward** word deletion.  We keep the ability to delete
+    //  the *next* word by requiring an additional Shift modifier – a common
+    //  binding on full-size keyboards that expose a dedicated Forward Delete
+    //  key.
+    // ------------------------------------------------------------------
     else if (
+      // ⌥/Alt/Meta + (Backspace|Delete|DEL byte) → backward word delete
       (key["meta"] || key["ctrl"] || key["alt"]) &&
-      (key["backspace"] || input === "\x7f")
+      !key["shift"] &&
+      (key["backspace"] || input === "\x7f" || key["delete"])
     ) {
       this.deleteWordLeft();
-    } else if ((key["meta"] || key["ctrl"] || key["alt"]) && key["delete"]) {
+    } else if (
+      // ⇧+⌥/Alt/Meta + (Backspace|Delete|DEL byte) → forward word delete
+      (key["meta"] || key["ctrl"] || key["alt"]) &&
+      key["shift"] &&
+      (key["backspace"] || input === "\x7f" || key["delete"])
+    ) {
       this.deleteWordRight();
     } else if (
       key["backspace"] ||
diff --git a/codex-cli/tests/text-buffer-word.test.ts b/codex-cli/tests/text-buffer-word.test.ts
index f02ac045..a6ca9ebb 100644
--- a/codex-cli/tests/text-buffer-word.test.ts
+++ b/codex-cli/tests/text-buffer-word.test.ts
@@ -43,18 +43,17 @@ describe("TextBuffer – word‑wise navigation & deletion", () => {
     expect(tb.getText()).toBe("foo bar ");
   });
 
-  test("Option/Alt+Delete deletes next word", () => {
+  test("Option/Alt+Delete deletes previous word (matches shells)", () => {
     const tb = new TextBuffer("foo bar baz");
     const vp = { height: 10, width: 80 } as const;
 
-    // Move caret between first and second word (after space)
-    tb.move("wordRight"); // after foo
-    tb.move("right"); // skip space -> start of bar
+    // Place caret at end so we can test backward deletion.
+    tb.move("end");
 
-    // Option+Delete
+    // Simulate Option+Delete (parsed as alt-modified Delete on some terminals)
     tb.handleInput(undefined, { delete: true, alt: true }, vp);
 
-    expect(tb.getText()).toBe("foo  baz"); // note double space removed later maybe
+    expect(tb.getText()).toBe("foo bar ");
   });
 
   test("wordLeft eventually reaches column 0", () => {
@@ -121,4 +120,18 @@ describe("TextBuffer – word‑wise navigation & deletion", () => {
     const [, col] = tb.getCursor();
     expect(col).toBe(6);
   });
+
+  test("Shift+Option/Alt+Delete deletes next word", () => {
+    const tb = new TextBuffer("foo bar baz");
+    const vp = { height: 10, width: 80 } as const;
+
+    // Move caret between first and second word (after space)
+    tb.move("wordRight"); // after foo
+    tb.move("right"); // skip space -> start of bar
+
+    // Shift+Option+Delete should now remove "bar "
+    tb.handleInput(undefined, { delete: true, alt: true, shift: true }, vp);
+
+    expect(tb.getText()).toBe("foo baz");
+  });
 });