Skip to content

Commit 1ff3e14

Browse files
authored
fix: patch in #366 and #367 for marked-terminal (#916)
This PR uses [`pnpm patch`](https://www.petermekhaeil.com/til/pnpm-patch/) to pull in the following proposed fixes for `marked-terminal`: * mikaelbr/marked-terminal#366 * mikaelbr/marked-terminal#367 This adds a substantial test to `codex-cli/tests/markdown.test.tsx` to verify the new behavior. Note that one of the tests shows two citations being split across a line even though the rendered version would fit comfortably on one line. Changing this likely requires a subtle fix to `marked-terminal` to account for "rendered length" when determining line breaks.
1 parent dd354e2 commit 1ff3e14

File tree

5 files changed

+141
-11
lines changed

5 files changed

+141
-11
lines changed

codex-cli/tests/markdown.test.tsx

Lines changed: 100 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,17 @@ import React from "react";
66
import { describe, afterEach, beforeEach, it, expect, vi } from "vitest";
77
import chalk from "chalk";
88

9+
const BOLD = "\x1B[1m";
10+
const BOLD_OFF = "\x1B[22m";
11+
const ITALIC = "\x1B[3m";
12+
const ITALIC_OFF = "\x1B[23m";
13+
const LINK_ON = "\x1B[4m";
14+
const LINK_OFF = "\x1B[24m";
15+
const BLUE = "\x1B[34m";
16+
const GREEN = "\x1B[32m";
17+
const YELLOW = "\x1B[33m";
18+
const COLOR_OFF = "\x1B[39m";
19+
920
/** Simple sanity check that the Markdown component renders bold/italic text.
1021
* We strip ANSI codes, so the output should contain the raw words. */
1122
it("renders basic markdown", () => {
@@ -44,25 +55,105 @@ describe("ensure <Markdown> produces content with correct ANSI escape codes", ()
4455
);
4556

4657
const frame = lastFrame();
47-
const BOLD = "\x1B[1m";
48-
const BOLD_OFF = "\x1B[22m";
49-
const ITALIC = "\x1B[3m";
50-
const ITALIC_OFF = "\x1B[23m";
5158
expect(frame).toBe(`${BOLD}bold${BOLD_OFF} ${ITALIC}italic${ITALIC_OFF}`);
5259
});
5360

61+
// We had to patch in https://github.com/mikaelbr/marked-terminal/pull/366 to
62+
// make this work.
63+
it("bold test in a bullet should be rendered correctly", () => {
64+
const { lastFrame } = renderTui(
65+
<Markdown fileOpener={undefined}>* **bold** text</Markdown>,
66+
);
67+
68+
const outputWithAnsi = lastFrame();
69+
expect(outputWithAnsi).toBe(`* ${BOLD}bold${BOLD_OFF} text`);
70+
});
71+
72+
it("ensure simple nested list works as expected", () => {
73+
// Empirically, if there is no text at all before the first list item,
74+
// it gets indented.
75+
const nestedList = `\
76+
Paragraph before bulleted list.
77+
78+
* item 1
79+
* subitem 1
80+
* subitem 2
81+
* item 2
82+
`;
83+
const { lastFrame } = renderTui(
84+
<Markdown fileOpener={undefined}>{nestedList}</Markdown>,
85+
);
86+
87+
const outputWithAnsi = lastFrame();
88+
const i4 = " ".repeat(4);
89+
const expectedNestedList = `\
90+
Paragraph before bulleted list.
91+
92+
${i4}* item 1
93+
${i4}${i4}* subitem 1
94+
${i4}${i4}* subitem 2
95+
${i4}* item 2`;
96+
expect(outputWithAnsi).toBe(expectedNestedList);
97+
});
98+
99+
// We had to patch in https://github.com/mikaelbr/marked-terminal/pull/367 to
100+
// make this work.
101+
it("ensure sequential subitems with styling to do not get extra newlines", () => {
102+
// This is a real-world example that exhibits many of the Markdown features
103+
// we care about. Though the original issue fix this was intended to verify
104+
// was that even though there is a single newline between the two subitems,
105+
// the stock version of marked-terminal@7.3.0 was adding an extra newline
106+
// in the output.
107+
const nestedList = `\
108+
## 🛠 Core CLI Logic
109+
110+
All of the TypeScript/React code lives under \`src/\`. The main entrypoint for argument parsing and orchestration is:
111+
112+
### \`src/cli.tsx\`
113+
- Uses **meow** for flags/subcommands and prints the built-in help/usage:
114+
【F:src/cli.tsx†L49-L53】【F:src/cli.tsx†L55-L60】
115+
- Handles special subcommands (e.g. \`codex completion …\`), \`--config\`, API-key validation, then either:
116+
- Spawns the **AgentLoop** for the normal multi-step prompting/edits flow, or
117+
- Runs **single-pass** mode if \`--full-context\` is set.
118+
119+
`;
120+
const { lastFrame } = renderTui(
121+
<Markdown fileOpener={"vscode"} cwd="/home/user/codex">
122+
{nestedList}
123+
</Markdown>,
124+
);
125+
126+
const outputWithAnsi = lastFrame();
127+
128+
// Note that the line with two citations gets split across two lines.
129+
// While the underlying ANSI content is long such that the split appears to
130+
// be merited, the rendered output is considerably shorter and ideally it
131+
// would be a single line.
132+
const expectedNestedList = `${GREEN}${BOLD}## 🛠 Core CLI Logic${BOLD_OFF}${COLOR_OFF}
133+
134+
All of the TypeScript/React code lives under ${YELLOW}src/${COLOR_OFF}. The main entrypoint for argument parsing and
135+
orchestration is:
136+
137+
${GREEN}${BOLD}### ${YELLOW}src/cli.tsx${COLOR_OFF}${BOLD_OFF}
138+
139+
* Uses ${BOLD}meow${BOLD_OFF} for flags/subcommands and prints the built-in help/usage:
140+
${BLUE}src/cli.tsx (${LINK_ON}vscode://file/home/user/codex/src/cli.tsx:49${LINK_OFF})src/cli.tsx ${COLOR_OFF}
141+
${BLUE}(${LINK_ON}vscode://file/home/user/codex/src/cli.tsx:55${LINK_OFF})${COLOR_OFF}
142+
* Handles special subcommands (e.g. ${YELLOW}codex completion …${COLOR_OFF}), ${YELLOW}--config${COLOR_OFF}, API-key validation, then
143+
either:
144+
* Spawns the ${BOLD}AgentLoop${BOLD_OFF} for the normal multi-step prompting/edits flow, or
145+
* Runs ${BOLD}single-pass${BOLD_OFF} mode if ${YELLOW}--full-context${COLOR_OFF} is set.`;
146+
147+
expect(outputWithAnsi).toBe(expectedNestedList);
148+
});
149+
54150
it("citations should get converted to hyperlinks when stdout supports them", () => {
55151
const { lastFrame } = renderTui(
56152
<Markdown fileOpener={"vscode"} cwd="/foo/bar">
57153
File with TODO: 【F:src/approvals.ts†L40】
58154
</Markdown>,
59155
);
60156

61-
const BLUE = "\x1B[34m";
62-
const LINK_ON = "\x1B[4m";
63-
const LINK_OFF = "\x1B[24m";
64-
const COLOR_OFF = "\x1B[39m";
65-
66157
const expected = `File with TODO: ${BLUE}src/approvals.ts (${LINK_ON}vscode://file/foo/bar/src/approvals.ts:40${LINK_OFF})${COLOR_OFF}`;
67158
const outputWithAnsi = lastFrame();
68159
expect(outputWithAnsi).toBe(expected);

package.json

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,11 @@
2929
"overrides": {
3030
"punycode": "^2.3.1"
3131
},
32+
"pnpm": {
33+
"patchedDependencies": {
34+
"marked-terminal@7.3.0": "patches/marked-terminal@7.3.0.patch"
35+
}
36+
},
3237
"engines": {
3338
"node": ">=22",
3439
"pnpm": ">=9.0.0"
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
diff --git a/index.js b/index.js
2+
index 5e2d4b4f212a7c614ebcd5cba8c4928fa3e0d2d0..24dba3560bee4f88dac9106911ef204f37babebe 100644
3+
--- a/index.js
4+
+++ b/index.js
5+
@@ -83,7 +83,7 @@ Renderer.prototype.space = function () {
6+
7+
Renderer.prototype.text = function (text) {
8+
if (typeof text === 'object') {
9+
- text = text.text;
10+
+ text = text.tokens ? this.parser.parseInline(text.tokens) : text.text;
11+
}
12+
return this.o.text(text);
13+
};
14+
@@ -185,10 +185,10 @@ Renderer.prototype.listitem = function (text) {
15+
}
16+
var transform = compose(this.o.listitem, this.transform);
17+
var isNested = text.indexOf('\n') !== -1;
18+
- if (isNested) text = text.trim();
19+
+ if (!isNested) text = transform(text);
20+
21+
// Use BULLET_POINT as a marker for ordered or unordered list item
22+
- return '\n' + BULLET_POINT + transform(text);
23+
+ return '\n' + BULLET_POINT + text;
24+
};
25+
26+
Renderer.prototype.checkbox = function (checked) {

pnpm-lock.yaml

Lines changed: 7 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pnpm-workspace.yaml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,6 @@ packages:
55

66
ignoredBuiltDependencies:
77
- esbuild
8+
9+
patchedDependencies:
10+
marked-terminal@7.3.0: patches/marked-terminal@7.3.0.patch

0 commit comments

Comments
 (0)