Skip to content

Commit 53b5870

Browse files
dahliaclaude
andcommitted
Fix help line overflow when option term exceeds termWidth
When a rendered option term is wider than termWidth (default: 26), the description column starts further right on the first output line than descColumnWidth assumes. formatMessage() was still given the full descColumnWidth budget, so descriptions, default values, and choices could produce lines that far exceeded maxWidth. The fix computes extraTermOffset = max(0, termVisibleWidth - termWidth) and passes it as startWidth to the formatMessage() call for the description, propagating the narrowed first-line budget through all subsequent appends (showDefault, showChoices). Once any content has wrapped to a continuation line the extra offset no longer applies and resets to zero. A second, independent fix reduces the maxWidth given to formatMessage() for default and choices content by suffix.length (1 char for "]" and ")") so that appending the closing suffix afterward can never push a fully-filled line 1 character over the column limit. Eight regression tests cover: the original reported case, wide term with showDefault only, wide term with both options, description wrapping due to the narrowed budget, colors-enabled visible-width accounting, the term-equals-termWidth boundary (no regression), the term-is-termWidth+1 boundary, and the suffix-overflow edge case. Fixes #132 Co-Authored-By: Claude <claude@anthropic.com>
1 parent a7e3105 commit 53b5870

File tree

3 files changed

+415
-11
lines changed

3 files changed

+415
-11
lines changed

CHANGES.md

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,27 @@ Version 0.10.6
88

99
To be released.
1010

11+
### @optique/core
12+
13+
- Fixed `formatDocPage()` to respect `maxWidth` when the rendered option
14+
term is wider than `termWidth` (default: 26). Previously, the description
15+
column width was calculated assuming the term occupied exactly `termWidth`
16+
characters. When the actual term was wider (e.g.,
17+
`-p, --package-manager PACKAGE_MANAGER` at 38 chars), the description
18+
column started further right on the first output line, but `formatMessage()`
19+
was still given the full `descColumnWidth` budget, allowing lines to exceed
20+
`maxWidth` by as much as the term's extra width. The fix passes the extra
21+
width as `startWidth` to `formatMessage()` so the first-line budget is
22+
correctly narrowed. The same correction is applied when appending default
23+
values (`showDefault`) and available choices (`showChoices`). [[#132]]
24+
25+
- Fixed `formatDocPage()` to account for the closing suffix (`]` for default
26+
values, `)` for choices) when word-wrapping content. Previously, the suffix
27+
was appended after `formatMessage()` had already filled the description
28+
column to capacity, producing lines that were one character too wide.
29+
30+
[#132]: https://github.com/dahlia/optique/issues/132
31+
1132

1233
Version 0.10.5
1334
--------------

packages/core/src/doc.test.ts

Lines changed: 343 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -858,7 +858,350 @@ describe("formatDocPage", () => {
858858
);
859859
});
860860

861+
// Helper: strip ANSI escape codes to measure visible line width
862+
// deno-lint-ignore no-control-regex
863+
const stripAnsi = (s: string) => s.replace(/\x1B\[[0-9;]*[a-zA-Z]/g, "");
864+
861865
describe("maxWidth with showDefault and showChoices", () => {
866+
// Issue #132: when a term is wider than termWidth (default: 26), the
867+
// description column starts further right than descColumnWidth assumes,
868+
// but formatMessage is still given the full descColumnWidth budget.
869+
// This causes the first line to overflow maxWidth.
870+
871+
it("should not exceed maxWidth when term is wider than termWidth with showChoices (issue #132)", () => {
872+
// "-p, --package-manager PACKAGE_MANAGER" is 38 chars > termWidth 26.
873+
// descColumnWidth = 100 - 2 - 26 - 2 = 70, but first-line budget is
874+
// only 100 - (2 + 38 + 2) = 58 chars. Without the fix, the combined
875+
// description + choices text fills 69 chars in the desc column, and the
876+
// full line becomes 2 + 38 + 2 + 69 = 111 chars (observed in the issue).
877+
const page: DocPage = {
878+
sections: [{
879+
title: "Options",
880+
entries: [{
881+
term: {
882+
type: "option",
883+
names: ["-p", "--package-manager"],
884+
metavar: "PACKAGE_MANAGER" as const,
885+
},
886+
description: [
887+
{
888+
type: "text",
889+
text: "The package manager to use for installing dependencies.",
890+
},
891+
],
892+
choices: valueSet(
893+
["deno", "pnpm", "bun", "yarn", "npm"],
894+
{ type: "unit" },
895+
),
896+
}],
897+
}],
898+
};
899+
900+
const result = formatDocPage("repro", page, {
901+
showChoices: true,
902+
maxWidth: 100,
903+
colors: false,
904+
});
905+
906+
for (const line of result.split("\n")) {
907+
assert.ok(
908+
line.length <= 100,
909+
`Line exceeds maxWidth 100: "${line}" (${line.length} chars)`,
910+
);
911+
}
912+
});
913+
914+
it("should not exceed maxWidth when term is wider than termWidth with showDefault", () => {
915+
// Same pattern as issue #132, but with showDefault instead of showChoices.
916+
// The defaultStartWidth calculation has the same bug: it uses
917+
// lastLineVisibleLength(description) without accounting for the extra
918+
// physical offset caused by the wide term.
919+
// Term "-p, --package-manager PACKAGE_MANAGER" is 38 chars.
920+
// descColumnWidth = 80 - 2 - 26 - 2 = 50.
921+
// First-line budget = 80 - (2 + 38 + 2) = 38 chars.
922+
// Without fix: "The package manager to use for your project." (44 chars)
923+
// fits in 50 but not 38, so the full line becomes 2+38+2+44 = 86 > 80.
924+
const page: DocPage = {
925+
sections: [{
926+
entries: [{
927+
term: {
928+
type: "option",
929+
names: ["-p", "--package-manager"],
930+
metavar: "PACKAGE_MANAGER" as const,
931+
},
932+
description: [
933+
{
934+
type: "text",
935+
text: "The package manager to use for your project.",
936+
},
937+
],
938+
default: [{ type: "text", text: "npm" }],
939+
}],
940+
}],
941+
};
942+
943+
const result = formatDocPage("repro", page, {
944+
showDefault: true,
945+
maxWidth: 80,
946+
colors: false,
947+
});
948+
949+
for (const line of result.split("\n")) {
950+
assert.ok(
951+
line.length <= 80,
952+
`Line exceeds maxWidth 80: "${line}" (${line.length} chars)`,
953+
);
954+
}
955+
});
956+
957+
it("should not exceed maxWidth when term is wider than termWidth with both showDefault and showChoices", () => {
958+
// Combined case: wide term + both options active. The accumulated
959+
// description + default + choices text on the first output line must
960+
// still respect maxWidth.
961+
// Term "-w, --web-framework WEB_FRAMEWORK" is 34 chars > 26.
962+
// descColumnWidth = 100 - 2 - 26 - 2 = 70.
963+
// First-line budget = 100 - (2 + 34 + 2) = 62 chars.
964+
const page: DocPage = {
965+
sections: [{
966+
entries: [{
967+
term: {
968+
type: "option",
969+
names: ["-w", "--web-framework"],
970+
metavar: "WEB_FRAMEWORK" as const,
971+
},
972+
description: [
973+
{ type: "text", text: "The web framework to integrate." },
974+
],
975+
default: [{ type: "text", text: "hono" }],
976+
choices: valueSet(
977+
["hono", "nitro", "next", "elysia", "express"],
978+
{ type: "unit" },
979+
),
980+
}],
981+
}],
982+
};
983+
984+
const result = formatDocPage("repro", page, {
985+
showDefault: true,
986+
showChoices: true,
987+
maxWidth: 100,
988+
colors: false,
989+
});
990+
991+
for (const line of result.split("\n")) {
992+
assert.ok(
993+
line.length <= 100,
994+
`Line exceeds maxWidth 100: "${line}" (${line.length} chars)`,
995+
);
996+
}
997+
});
998+
999+
it("should wrap description itself when it would overflow due to wide term", () => {
1000+
// When the description text itself is longer than the narrowed first-line
1001+
// budget (maxWidth - termIndent - actualTermWidth - 2), it must be wrapped
1002+
// within that budget, not within the wider descColumnWidth.
1003+
// Term is 38 chars, maxWidth = 80, first-line budget = 38 chars.
1004+
// Description "The package manager to use for your project." is 44 chars:
1005+
// - Without fix: fits in descColumnWidth=50, stays on one line → full
1006+
// line = 2+38+2+44 = 86 > 80.
1007+
// - With fix: wrapped at 38-char budget, first line ≤ 38 chars → fine.
1008+
const page: DocPage = {
1009+
sections: [{
1010+
entries: [{
1011+
term: {
1012+
type: "option",
1013+
names: ["-p", "--package-manager"],
1014+
metavar: "PACKAGE_MANAGER" as const,
1015+
},
1016+
description: [
1017+
{
1018+
type: "text",
1019+
text: "The package manager to use for your project.",
1020+
},
1021+
],
1022+
}],
1023+
}],
1024+
};
1025+
1026+
const result = formatDocPage("repro", page, {
1027+
maxWidth: 80,
1028+
colors: false,
1029+
});
1030+
1031+
for (const line of result.split("\n")) {
1032+
assert.ok(
1033+
line.length <= 80,
1034+
`Line exceeds maxWidth 80: "${line}" (${line.length} chars)`,
1035+
);
1036+
}
1037+
// Description must have wrapped onto a new line
1038+
assert.ok(result.includes("\n"), "Expected a wrapped line in output");
1039+
});
1040+
1041+
it("should not exceed maxWidth when term is wider than termWidth with colors enabled", () => {
1042+
// ANSI escape codes inflate the raw string length but must not be counted
1043+
// toward visible width. lastLineVisibleLength() strips them correctly,
1044+
// so the overflow logic should still trigger for wide terms even when
1045+
// colors: true adds ANSI codes to the term string.
1046+
const page: DocPage = {
1047+
sections: [{
1048+
entries: [{
1049+
term: {
1050+
type: "option",
1051+
names: ["-p", "--package-manager"],
1052+
metavar: "PACKAGE_MANAGER" as const,
1053+
},
1054+
description: [
1055+
{
1056+
type: "text",
1057+
text: "The package manager to use for installing dependencies.",
1058+
},
1059+
],
1060+
choices: valueSet(
1061+
["deno", "pnpm", "bun", "yarn", "npm"],
1062+
{ type: "unit" },
1063+
),
1064+
}],
1065+
}],
1066+
};
1067+
1068+
const result = formatDocPage("repro", page, {
1069+
showChoices: true,
1070+
maxWidth: 100,
1071+
colors: true,
1072+
});
1073+
1074+
for (const line of result.split("\n")) {
1075+
const visibleLength = stripAnsi(line).length;
1076+
assert.ok(
1077+
visibleLength <= 100,
1078+
`Line visible length exceeds maxWidth 100: ${visibleLength} chars`,
1079+
);
1080+
}
1081+
});
1082+
1083+
it("should not overflow when term is exactly termWidth wide (boundary)", () => {
1084+
// When term visible width equals termWidth (26), extraTermOffset = 0
1085+
// and behaviour should be identical to the pre-fix code path.
1086+
// "--verbose-mode VERBOSE_MOD" is exactly 26 chars.
1087+
const page: DocPage = {
1088+
sections: [{
1089+
entries: [{
1090+
term: {
1091+
type: "option",
1092+
names: ["--verbose-mode"],
1093+
metavar: "VERBOSE_MOD" as const,
1094+
},
1095+
description: [
1096+
{ type: "text", text: "Enable verbose mode output." },
1097+
],
1098+
choices: valueSet(
1099+
["trace", "debug", "info", "warn", "error"],
1100+
{ type: "unit" },
1101+
),
1102+
}],
1103+
}],
1104+
};
1105+
1106+
const result = formatDocPage("repro", page, {
1107+
showChoices: true,
1108+
maxWidth: 80,
1109+
colors: false,
1110+
});
1111+
1112+
for (const line of result.split("\n")) {
1113+
assert.ok(
1114+
line.length <= 80,
1115+
`Line exceeds maxWidth 80: "${line}" (${line.length} chars)`,
1116+
);
1117+
}
1118+
});
1119+
1120+
it("should not overflow when term is 1 char wider than termWidth (boundary)", () => {
1121+
// Minimal extra offset: term is just 1 char wider than termWidth.
1122+
// "--verbose-mode VERBOSE_MODE" is 27 chars (termWidth 26 + 1).
1123+
// descColumnWidth = 80 - 2 - 26 - 2 = 50. First-line budget = 49.
1124+
//
1125+
// "Enable verbose mode output for entire running sys." is exactly 50 chars:
1126+
// tokens = ["Enable "(7), "verbose "(8), "mode "(5), "output "(7),
1127+
// "for "(4), "entire "(7), "running "(8), "sys."(4)] = 50 total.
1128+
//
1129+
// Without fix (startWidth=0): running totals stay ≤ 50, no wrap.
1130+
// Full first line = 2+27+2+50 = 81 > 80 → OVERFLOW.
1131+
// With fix (startWidth=1): total hits 51 at "sys.", so it wraps.
1132+
// Full first line ≤ 80 ✓.
1133+
const page: DocPage = {
1134+
sections: [{
1135+
entries: [{
1136+
term: {
1137+
type: "option",
1138+
names: ["--verbose-mode"],
1139+
metavar: "VERBOSE_MODE" as const,
1140+
},
1141+
description: [
1142+
{
1143+
type: "text",
1144+
text: "Enable verbose mode output for entire running sys.",
1145+
},
1146+
],
1147+
}],
1148+
}],
1149+
};
1150+
1151+
const result = formatDocPage("repro", page, {
1152+
maxWidth: 80,
1153+
colors: false,
1154+
});
1155+
1156+
for (const line of result.split("\n")) {
1157+
assert.ok(
1158+
line.length <= 80,
1159+
`Line exceeds maxWidth 80: "${line}" (${line.length} chars)`,
1160+
);
1161+
}
1162+
// With the fix the description must have wrapped
1163+
assert.ok(result.includes("\n"), "Expected wrapped line");
1164+
});
1165+
1166+
it("should not overflow when choices suffix ) would push the last line over maxWidth", () => {
1167+
// The closing suffix ")" is appended outside of formatMessage, so it is
1168+
// not counted in the maxWidth budget. If the choices content is allowed
1169+
// to fill the description column to the very last char, adding ")"
1170+
// produces a line that is 1 char too wide.
1171+
//
1172+
// Setup (all default termIndent=2, termWidth=26):
1173+
// maxWidth = 50 → descColumnWidth = 50-2-26-2 = 20
1174+
// term "--option" (8 chars) fits in termWidth, no extra offset
1175+
// description = "" (empty)
1176+
// prefix = " (" (2), label = "choices: " (9) → prefixLabelLen = 11
1177+
// choicesStartWidth = 0 + 11 = 11
1178+
// value "aaaaaaaaa" (9 chars): 11+9 = 20 = descColumnWidth → no wrap
1179+
// choicesDisplay = "aaaaaaaaa"
1180+
// choicesText = " (choices: aaaaaaaaa)" = 21 chars
1181+
// full line = 2 + 26 + 2 + 21 = 51 > 50
1182+
const page: DocPage = {
1183+
sections: [{
1184+
entries: [{
1185+
term: { type: "option", names: ["--option"] },
1186+
choices: valueSet(["aaaaaaaaa"], { type: "unit" }),
1187+
}],
1188+
}],
1189+
};
1190+
1191+
const result = formatDocPage("repro", page, {
1192+
showChoices: true,
1193+
maxWidth: 50,
1194+
colors: false,
1195+
});
1196+
1197+
for (const line of result.split("\n")) {
1198+
assert.ok(
1199+
line.length <= 50,
1200+
`Line exceeds maxWidth 50: "${line}" (${line.length} chars)`,
1201+
);
1202+
}
1203+
});
1204+
8621205
it("should not exceed maxWidth when showDefault overflows the line", () => {
8631206
const page: DocPage = {
8641207
sections: [{

0 commit comments

Comments
 (0)