Skip to content

Commit c14c70c

Browse files
os-litantclaude
andauthored
fix(cli): make i18n extract --check's regenerate hint the invocation it was given (#16470)
`os i18n extract --check` printed a "Regenerate and commit" command assembled at the print site from four things that happened to be in scope: the config argument, the emitted locales minus the default one, `--fill` and `--out`. Every other flag the operator passed was not in the expression, so it was not in the advice. Driven on the reported invocation against a stack whose `i18n.defaultLocale` is `zh-CN`: $ os i18n extract objectstack.config.ts --locales=zh-CN --no-metadata-forms --no-objects-only --filter=kpi_ --out=OUT --check missing: ../../../../../tmp/os-i18n-repro-jNrZ/zh-CN.objects.generated.ts Translation bundles have drifted from the schema. Regenerate and commit: os i18n extract objectstack.config.ts --locales= --fill=empty --out=OUT Three defects, each measured before the repair: * `--locales=` came out EMPTY. The echo drops the default locale on the grounds that `--locales` always re-adds it, and here the only locale asked for WAS the default one. The `Skeleton summary` two lines up names `zh-CN`, so one code path had the locale and the other did not. * `--no-metadata-forms`, `--no-objects-only` and `--filter=kpi_` were never candidates for the line. Running what it printed wrote 775 keys across two files where the operator's own command writes 2 across one — including a `metadata-forms` companion they had explicitly switched off. * `missing:` printed a `../../../../../…` walk out of the cwd for a directory the operator had just typed in full. The second one is what it costs. A failing `--check` is self-healable in one step; following the printed advice emitted a different key set, so the next `--check` failed AGAIN — on `out of date:` rather than `missing:` — and printed the same wrong command. Both halves driven end to end. The hint is now a DELETION, never an assembly: this run's own argv (`this.argv`, verified to hold the arguments as typed with the command id stripped) with the `--check` token removed, shell-quoted so it survives a copy, and `--` honoured so a positional `--check` is left alone. Nothing enumerates flags, so a flag added to this command later is echoed without anyone remembering this print site — which is the property the assembled expression could not have. When `--check` is not in the argv the command cannot point at what it removed and prints "re-run the same command without `--check`" instead, the degraded line the report asked for; today's flag surface has no other way to set `--check` (no `env`, no default, no `allowNo`), so that branch is defence rather than a reachable path, and it is what keeps an approximation from ever becoming the fallback. Diagnostic paths are printed absolute once the cwd cannot reach them downwards, and keep the short relative form otherwise. That covers `Wrote` as well as `missing:` / `out of date:`: it is the same `path.relative(process.cwd(), file)` expression, in the same function, with the same symptom — the repro's write leg printed `Wrote ../../../../../tmp/os-i18n-repro-jNrZ/…` — and repairing the `--check` half alone would have left the two lines disagreeing. All nine of this repo's extract configs write in-tree and their output is unchanged. The pin drives the real CLI. Its central case asserts the WHOLE token list rather than probing for `--filter` and `--no-objects-only` by name, because a name-probing pin inherits exactly the blind spot being closed: it would be green for the next flag added. Its second case is the loop itself — the printed line is executed VERBATIM through `sh -c`, with an `os` shim on `PATH` in front of the source entry point, and the original `--check` must then pass. No approximate command can satisfy that one, and quoting is exercised for real by a case whose `--filter` value carries a `|`. Fixes #14895 Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N Co-authored-by: Claude <noreply@anthropic.com>
1 parent ce8caba commit c14c70c

3 files changed

Lines changed: 415 additions & 8 deletions

File tree

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
---
2+
"@objectstack/cli": patch
3+
---
4+
5+
`os i18n extract --check` now prints the invocation it was given, minus `--check`, as its "Regenerate and commit" hint — instead of a command assembled from four of the flags.
6+
7+
The hint used to be built at the print site from the config argument, the emitted locales minus the default one, `--fill` and `--out`. Everything else was absent from the expression, so it was absent from the advice. Driven on the reported invocation against a stack whose `i18n.defaultLocale` is `zh-CN`:
8+
9+
```
10+
$ os i18n extract objectstack.config.ts --locales=zh-CN --no-metadata-forms \
11+
--no-objects-only --filter=kpi_ --out=OUT --check
12+
✗ missing: ../../../../../tmp/os-i18n-repro/zh-CN.objects.generated.ts
13+
✗ Translation bundles have drifted from the schema. Regenerate and commit:
14+
os i18n extract objectstack.config.ts --locales= --fill=empty --out=OUT
15+
```
16+
17+
`--locales=` came out empty because the only locale asked for was the default one, and the echo dropped the default locale on the grounds that `--locales` always re-adds it; `--no-metadata-forms`, `--no-objects-only` and `--filter=kpi_` were never candidates for the line. Running what it printed wrote 775 keys across two files where the operator's own command writes 2 across one — a `metadata-forms` companion they had explicitly switched off, and an unfiltered key set. The next `--check` then failed again, on `out of date:` instead of `missing:`, and printed the same wrong command. A failure that heals itself in one step became a loop, and the loop was the printed advice.
18+
19+
The hint is now a deletion rather than an assembly: this run's own argv with the `--check` token removed, shell-quoted so it can be copied, `--` honoured so a positional `--check` is left alone. Nothing enumerates flags, so a flag added to this command later is echoed without anyone remembering this print site. When `--check` is not in the argv the command cannot say what it removed, and prints "re-run the same command without `--check`" rather than guessing.
20+
21+
Diagnostic paths are also no longer walks. `missing:`, `out of date:` and `Wrote` printed a bare path relative to the working directory, which for an `--out` outside the project produced `../../../../../tmp/i18n-out/zh-CN.objects.generated.ts` for a directory the operator had just typed in full. A path the working directory cannot reach downwards is now printed absolute; an in-tree `--out` — what all nine of this repo's extract configs use — keeps the short relative form it has always had.

packages/cli/src/commands/i18n/extract.ts

Lines changed: 114 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,107 @@ import {
3131

3232
const FILL_STRATEGIES: FillStrategy[] = ['empty', 'default', 'todo'];
3333

34+
/**
35+
* A path for one of this command's output lines: relative to the cwd while that
36+
* is still a NAME for the file, absolute once it stops being one.
37+
*
38+
* Every path this command printed used to be a bare `path.relative(cwd, file)`,
39+
* and for an `--out` outside the project that is not a name — it is a walk.
40+
* Driven from `packages/cli` with `--out=/tmp/os-i18n-repro-jNrZ`, the `--check`
41+
* failure reported
42+
* `missing: ../../../../../tmp/os-i18n-repro-jNrZ/zh-CN.objects.generated.ts`
43+
* for a directory the operator had just typed in full (#14895). Nothing in that
44+
* string is recognisable as what they wrote, and it only resolves against a cwd
45+
* the line does not state.
46+
*
47+
* The threshold is "does the relative form still descend from here", not a
48+
* length: an in-tree `--out` — which is what all nine of this repo's extract
49+
* configs use — keeps the short form it has always had, and only a path that
50+
* has to climb out of the cwd is printed absolute.
51+
*/
52+
function displayPath(file: string): string {
53+
const rel = path.relative(process.cwd(), file);
54+
// `path.relative` answers with an ABSOLUTE path across Windows drive roots,
55+
// where no relative form exists at all; that is already the answer wanted.
56+
if (!rel || path.isAbsolute(rel)) return file;
57+
return rel === '..' || rel.startsWith(`..${path.sep}`) ? file : rel;
58+
}
59+
60+
/** One argv token, spelled so a POSIX shell hands it back byte-for-byte. */
61+
function shellToken(token: string): string {
62+
if (/^[A-Za-z0-9_@%+=:,./-]+$/.test(token)) return token;
63+
// Close the quote, emit an escaped quote, reopen — the only way a literal
64+
// `'` survives single quoting.
65+
return `'${token.split("'").join("'\\''")}'`;
66+
}
67+
68+
/**
69+
* This run's own invocation with `--check` taken out of it — the ONLY command
70+
* the `--check` failure hint may print.
71+
*
72+
* ## Why a deletion and never an assembly
73+
*
74+
* The hint used to be BUILT, from four things this file happened to have in
75+
* scope: the config arg, the emitted locales minus the default one, `--fill`
76+
* and `--out`. Everything else the operator passed was simply not in the
77+
* expression, so it was not in the advice either. Driven on the reporter's
78+
* invocation (#14895):
79+
*
80+
* $ os i18n extract stack.config.ts --locales=zh-CN --no-metadata-forms
81+
* --no-objects-only --filter=kpi_ --out=OUT --check
82+
* ✗ Translation bundles have drifted from the schema. Regenerate and commit:
83+
* os i18n extract stack.config.ts --locales= --fill=empty --out=OUT
84+
*
85+
* `--locales=` came out EMPTY — the emitted locale was the default locale, and
86+
* the filter that drops the default one from the echo then drops the only
87+
* locale there was — while `--no-metadata-forms`, `--no-objects-only` and
88+
* `--filter=kpi_` were never candidates for the line to begin with. Running
89+
* what it printed emitted 775 keys across two files instead of 2 across one,
90+
* including a `metadata-forms` companion the operator had explicitly switched
91+
* off; the next `--check` then failed AGAIN, on `out of date:` instead of
92+
* `missing:`, and printed the same wrong command. That loop is the defect: the
93+
* failure is self-healable and the advice is what stops it healing.
94+
*
95+
* An assembled command is wrong in exactly one way and it is unbounded — every
96+
* flag that exists now, and every flag added later, has to be remembered at
97+
* this print site or it silently goes missing. So this does not enumerate
98+
* flags at all. It takes the argv oclif was handed and removes one token from
99+
* it, which makes the echo correct for flags this file has never heard of.
100+
*
101+
* ⛔ It also never GUESSES. If `--check` is not in the argv the flag was not
102+
* spelled there, this function cannot point at what it removed, and the caller
103+
* prints "re-run the same command without `--check`" instead — the degraded
104+
* line the report itself asked for, on the grounds that a correct vague
105+
* sentence beats a complete-looking wrong command. Today's flag surface has no
106+
* other way to set `--check` (no `env`, no default, no `allowNo`), so that is
107+
* defence rather than a path a user can reach; it is what keeps "assemble an
108+
* approximation" from ever becoming the fallback.
109+
*
110+
* `--` is honoured because it changes what a token MEANS: after it, `--check`
111+
* is a positional argument and removing it would rewrite the invocation rather
112+
* than trim it.
113+
*
114+
* @param bin `config.bin` — `os`, the name the command is installed under
115+
* @param id `this.id` — `i18n:extract`, oclif's colon spelling of the path
116+
* @param argv `this.argv` — the arguments as typed, the command id stripped
117+
* @returns the command to print, or `undefined` when it cannot be built
118+
*/
119+
function rerunWithoutCheck(bin: string, id: string | undefined, argv: readonly string[]): string | undefined {
120+
const kept: string[] = [];
121+
let dropped = 0;
122+
let afterTerminator = false;
123+
for (const token of argv) {
124+
if (!afterTerminator && token === '--') afterTerminator = true;
125+
else if (!afterTerminator && (token === '--check' || token.startsWith('--check='))) {
126+
dropped += 1;
127+
continue;
128+
}
129+
kept.push(token);
130+
}
131+
if (dropped === 0) return undefined;
132+
return [bin, ...(id ?? 'i18n:extract').split(':'), ...kept.map(shellToken)].join(' ');
133+
}
134+
34135
/**
35136
* `os i18n extract` — scaffold translation skeletons.
36137
*
@@ -471,22 +572,27 @@ export default class I18nExtract extends Command {
471572
const stale: string[] = [];
472573
const missing: string[] = [];
473574
for (const { file, content } of emitted) {
474-
const rel = path.relative(process.cwd(), file);
475-
if (!fs.existsSync(file)) missing.push(rel);
476-
else if (fs.readFileSync(file, 'utf8') !== content) stale.push(rel);
575+
const shown = displayPath(file);
576+
if (!fs.existsSync(file)) missing.push(shown);
577+
else if (fs.readFileSync(file, 'utf8') !== content) stale.push(shown);
477578
}
478579
if (missing.length === 0 && stale.length === 0) {
479580
console.log('');
480581
printSuccess(`${emitted.length} bundle(s) are in sync with the schema ${chalk.dim(`(${timer.display()})`)}`);
481582
return;
482583
}
483-
for (const rel of missing) printError(`missing: ${rel}`);
484-
for (const rel of stale) printError(`out of date: ${rel}`);
584+
for (const shown of missing) printError(`missing: ${shown}`);
585+
for (const shown of stale) printError(`out of date: ${shown}`);
485586
console.log('');
587+
// The command that regenerates these bytes is THIS run without
588+
// `--check` — the two branches share the `emitted` list above, so the
589+
// write path cannot produce anything other than what was just
590+
// compared. {@link rerunWithoutCheck} says why it is spelled as a
591+
// deletion and what the degraded line is for.
592+
const rerun = rerunWithoutCheck(this.config.bin, this.id, this.argv);
486593
printError(
487594
'Translation bundles have drifted from the schema. Regenerate and commit:\n' +
488-
` os i18n extract ${args.config ?? ''} --locales=${localesEmitted.filter((l) => l !== defaultLocale).join(',')} ` +
489-
`--fill=${flags.fill} --out=${flags.out}`.replace(/\s+/g, ' '),
595+
(rerun ? ` ${rerun}` : ' re-run the same command without `--check`'),
490596
);
491597
process.exit(1);
492598
}
@@ -496,7 +602,7 @@ export default class I18nExtract extends Command {
496602
for (const { file, content, keys } of emitted) {
497603
fs.writeFileSync(file, content, 'utf8');
498604
written += 1;
499-
printInfo(`Wrote ${chalk.white(path.relative(process.cwd(), file))} (${keys} keys)`);
605+
printInfo(`Wrote ${chalk.white(displayPath(file))} (${keys} keys)`);
500606
}
501607
if (!anyMetadataForms) {
502608
printInfo('(no metadataForms keys discovered for these locales)');

0 commit comments

Comments
 (0)