Skip to content

Commit 90e0f90

Browse files
dahliaclaude
andcommitted
Respect maxWidth in showDefault and showChoices
formatDocPage() did not apply maxWidth word-wrapping to the default value and choices annotations appended by showDefault and showChoices. Both were concatenated directly onto the already-wrapped description string with no awareness of how many characters were already on the current line, so lines could far exceed maxWidth. Fixed by tracking the visible width of the last line of the description and passing it as an internal startWidth hint to formatMessage(), which now initializes its line-width counter from that value instead of always starting from zero. startWidth is kept out of the public MessageFormatOptions type via a function overload: only the implementation signature accepts it. Regression tests added. Fixes #129 Co-Authored-By: Claude <claude@anthropic.com>
1 parent 0f99cc8 commit 90e0f90

File tree

4 files changed

+180
-15
lines changed

4 files changed

+180
-15
lines changed

CHANGES.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,17 @@ Version 0.10.5
88

99
To be released.
1010

11+
### @optique/core
12+
13+
- Fixed `formatDocPage()` to respect `maxWidth` when appending default values
14+
and available choices via `showDefault` and `showChoices`. Previously,
15+
the appended text was concatenated onto the description string after
16+
word-wrapping had already been applied, with no awareness of how much of
17+
the current line was already occupied, so lines could far exceed `maxWidth`.
18+
[[#129]]
19+
20+
[#129]: https://github.com/dahlia/optique/issues/129
21+
1122

1223
Version 0.10.4
1324
--------------

packages/core/src/doc.test.ts

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -857,4 +857,86 @@ describe("formatDocPage", () => {
857857
"commas should be between value color sequences",
858858
);
859859
});
860+
861+
describe("maxWidth with showDefault and showChoices", () => {
862+
it("should not exceed maxWidth when showDefault overflows the line", () => {
863+
const page: DocPage = {
864+
sections: [{
865+
entries: [{
866+
term: { type: "option", names: ["--opt"] },
867+
description: [{ type: "text", text: "Some option" }],
868+
default: [{ type: "text", text: "a very long default value" }],
869+
}],
870+
}],
871+
};
872+
873+
const result = formatDocPage("myapp", page, {
874+
showDefault: true,
875+
maxWidth: 50,
876+
});
877+
878+
for (const line of result.split("\n")) {
879+
assert.ok(
880+
line.length <= 50,
881+
`Line exceeds maxWidth 50: "${line}" (${line.length} chars)`,
882+
);
883+
}
884+
});
885+
886+
it("should not exceed maxWidth when showChoices overflows the line", () => {
887+
const page: DocPage = {
888+
sections: [{
889+
entries: [{
890+
term: { type: "option", names: ["-f", "--format"] },
891+
description: [{ type: "text", text: "Output format" }],
892+
choices: valueSet(
893+
["json", "yaml", "toml", "xml", "csv", "tsv", "html", "markdown"],
894+
{ type: "unit" },
895+
),
896+
}],
897+
}],
898+
};
899+
900+
const result = formatDocPage("myapp", page, {
901+
showChoices: true,
902+
maxWidth: 60,
903+
});
904+
905+
for (const line of result.split("\n")) {
906+
assert.ok(
907+
line.length <= 60,
908+
`Line exceeds maxWidth 60: "${line}" (${line.length} chars)`,
909+
);
910+
}
911+
});
912+
913+
it("should not exceed maxWidth when both showDefault and showChoices overflow", () => {
914+
const page: DocPage = {
915+
sections: [{
916+
entries: [{
917+
term: { type: "option", names: ["-f", "--format"] },
918+
description: [{ type: "text", text: "Output format" }],
919+
default: [{ type: "text", text: "json" }],
920+
choices: valueSet(
921+
["json", "yaml", "toml", "xml", "csv", "tsv", "html", "markdown"],
922+
{ type: "unit" },
923+
),
924+
}],
925+
}],
926+
};
927+
928+
const result = formatDocPage("myapp", page, {
929+
showDefault: true,
930+
showChoices: true,
931+
maxWidth: 60,
932+
});
933+
934+
for (const line of result.split("\n")) {
935+
assert.ok(
936+
line.length <= 60,
937+
`Line exceeds maxWidth 60: "${line}" (${line.length} chars)`,
938+
);
939+
}
940+
});
941+
});
860942
});

packages/core/src/doc.ts

Lines changed: 75 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import {
22
formatMessage,
33
type Message,
4+
type MessageFormatOptions,
45
type MessageTerm,
56
text,
67
} from "./message.ts";
@@ -349,14 +350,16 @@ export function formatDocPage(
349350
: options.maxWidth - termIndent,
350351
});
351352

353+
const descColumnWidth = options.maxWidth == null
354+
? undefined
355+
: options.maxWidth - termIndent - termWidth - 2;
356+
352357
let description = entry.description == null
353358
? ""
354359
: formatMessage(entry.description, {
355360
colors: options.colors,
356361
quotes: !options.colors,
357-
maxWidth: options.maxWidth == null
358-
? undefined
359-
: options.maxWidth - termIndent - termWidth - 2,
362+
maxWidth: descColumnWidth,
360363
});
361364

362365
// Append default value if showDefault is enabled and default exists
@@ -367,12 +370,39 @@ export function formatDocPage(
367370
const suffix = typeof options.showDefault === "object"
368371
? options.showDefault.suffix ?? "]"
369372
: "]";
370-
const defaultText = `${prefix}${
371-
formatMessage(entry.default, {
372-
colors: options.colors ? { resetSuffix: "\x1b[2m" } : false,
373-
quotes: !options.colors,
374-
})
375-
}${suffix}`;
373+
374+
// Determine startWidth so that word-wrapping in the default value
375+
// continues correctly from the current line position.
376+
let defaultStartWidth: number | undefined;
377+
if (descColumnWidth != null) {
378+
const lastW = lastLineVisibleLength(description);
379+
if (lastW + prefix.length >= descColumnWidth) {
380+
description += "\n";
381+
defaultStartWidth = prefix.length;
382+
} else {
383+
defaultStartWidth = lastW + prefix.length;
384+
}
385+
}
386+
387+
// `startWidth` is accepted by the formatMessage() implementation but
388+
// is absent from the public MessageFormatOptions type. The inline
389+
// intersection type makes TypeScript accept the field here while
390+
// keeping it out of the public API. Because the intersection type is
391+
// a subtype of MessageFormatOptions, the call below remains
392+
// type-safe.
393+
const defaultFormatOptions: MessageFormatOptions & {
394+
readonly startWidth?: number;
395+
} = {
396+
colors: options.colors ? { resetSuffix: "\x1b[2m" } : false,
397+
quotes: !options.colors,
398+
maxWidth: descColumnWidth,
399+
startWidth: defaultStartWidth,
400+
};
401+
const defaultContent = formatMessage(
402+
entry.default,
403+
defaultFormatOptions,
404+
);
405+
const defaultText = `${prefix}${defaultContent}${suffix}`;
376406
const formattedDefault = options.colors
377407
? `\x1b[2m${defaultText}\x1b[0m`
378408
: defaultText;
@@ -418,10 +448,35 @@ export function formatDocPage(
418448
];
419449
}
420450
}
421-
const choicesDisplay = formatMessage(truncatedTerms, {
451+
// Determine startWidth so that word-wrapping in the choices list
452+
// continues correctly from the current line position.
453+
let choicesStartWidth: number | undefined;
454+
if (descColumnWidth != null) {
455+
const lastW = lastLineVisibleLength(description);
456+
const prefixLabelLen = prefix.length + label.length;
457+
if (lastW + prefixLabelLen >= descColumnWidth) {
458+
description += "\n";
459+
choicesStartWidth = prefixLabelLen;
460+
} else {
461+
choicesStartWidth = lastW + prefixLabelLen;
462+
}
463+
}
464+
465+
// See the comment above the defaultFormatOptions variable for why
466+
// startWidth is passed via a typed variable rather than an inline
467+
// object literal.
468+
const choicesFormatOptions: MessageFormatOptions & {
469+
readonly startWidth?: number;
470+
} = {
422471
colors: options.colors ? { resetSuffix: "\x1b[2m" } : false,
423472
quotes: false,
424-
});
473+
maxWidth: descColumnWidth,
474+
startWidth: choicesStartWidth,
475+
};
476+
const choicesDisplay = formatMessage(
477+
truncatedTerms,
478+
choicesFormatOptions,
479+
);
425480
const choicesText = `${prefix}${label}${choicesDisplay}${suffix}`;
426481
const formattedChoices = options.colors
427482
? `\x1b[2m${choicesText}\x1b[0m`
@@ -494,16 +549,23 @@ function indentLines(text: string, indent: number): string {
494549
return text.split("\n").join("\n" + " ".repeat(indent));
495550
}
496551

552+
// deno-lint-ignore no-control-regex
553+
const ansiEscapeCodeRegex = /\x1B\[[0-9;]*[a-zA-Z]/g;
554+
497555
function ansiAwareRightPad(
498556
text: string,
499557
length: number,
500558
char: string = " ",
501559
): string {
502-
// deno-lint-ignore no-control-regex
503-
const ansiEscapeCodeRegex = /\x1B\[[0-9;]*[a-zA-Z]/g;
504560
const strippedText = text.replace(ansiEscapeCodeRegex, "");
505561
if (strippedText.length >= length) {
506562
return text;
507563
}
508564
return text + char.repeat(length - strippedText.length);
509565
}
566+
567+
function lastLineVisibleLength(text: string): number {
568+
const lastNewline = text.lastIndexOf("\n");
569+
const lastLine = lastNewline === -1 ? text : text.slice(lastNewline + 1);
570+
return lastLine.replace(ansiEscapeCodeRegex, "").length;
571+
}

packages/core/src/message.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -496,7 +496,17 @@ export interface MessageFormatOptions {
496496
*/
497497
export function formatMessage(
498498
msg: Message,
499-
options: MessageFormatOptions = {},
499+
options?: MessageFormatOptions,
500+
): string;
501+
// The implementation accepts an additional `startWidth` field that is not
502+
// part of the public API. Other formatters within this package (e.g.,
503+
// formatDocPage() in doc.ts) pass it as a plain object variable — not as
504+
// an inline object literal — so TypeScript's excess-property check does not
505+
// apply and the field reaches the implementation without being part of the
506+
// declared public type.
507+
export function formatMessage(
508+
msg: Message,
509+
options: MessageFormatOptions & { readonly startWidth?: number } = {},
500510
): string {
501511
// Apply defaults
502512
const colorConfig = options.colors ?? false;
@@ -671,7 +681,7 @@ export function formatMessage(
671681
}
672682

673683
let output = "";
674-
let totalWidth = 0;
684+
let totalWidth = options.startWidth ?? 0;
675685
for (const { text, width } of stream()) {
676686
// Handle hard line breaks (marked with width -1)
677687
if (width === -1) {

0 commit comments

Comments
 (0)