Skip to content

Translate patch and pull request apply failures - #645

Merged
ryanwelcher merged 5 commits into
trunkfrom
fix/628-apply-failure-strings
Oct 7, 2026
Merged

ryanwelcher merged 5 commits into
trunkfrom
fix/628-apply-failure-strings

Conversation

@ryanwelcher

@ryanwelcher ryanwelcher commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator

Why

When a patch or a pull request will not go on, the apply card explains why: which files missed, how many of the changes, and what to do next. In any language but English that whole explanation is still English, and most of it was built by splicing words into sentences (`change${s}`, a where clause, 'includes' : 'include'), so it could not be translated as it was. #628 is the batch of #622 that covers it.

What changes

  • The failure breakdown (apply-conflict.cjs), the applied-patch notice and the preview's "whose work is this" sentences (applied-layer.cjs), and a pull request's preview, checkout notice and refusals (pr-checkout.cjs) are wrapped, along with the hook's own errors in use-apply-patch.jsx.
  • Main's side is wrapped too: the per-file refusals and the terminal lines in patch-apply.js, the parse errors in patch-plan.cjs, the pasted-URL errors in patch-sources.cjs, the Trac download error in trac-view.js, and the git:apply-patch and git:preview-patch handlers in main.js.
  • Spliced sentences are written whole: one string per case and per file count, _n on the count each sentence is about, numbered placeholders, and separate strings for an unnamed patch and for an issue against a ticket. About 127 strings in all.
  • REASONS/REVERT_REASONS and DISPOSABLE_EXIT/SLOT_HELD are now functions, so they are translated when shown. The applied notice's title is now one sentence with the patch's name inside it, not the name followed by the rest of the sentence.
  • English is unchanged. A throwaway script compared trunk's three renderer modules with this branch's across about 32,000 inputs and found the English output identical.

How to test this

The journey covers this: the apply card is fully translatable when a patch or a pull request will not go on, and once a patch is applied in tests/e2e/journeys/i18n.spec.js. Run it from the repository root (either platform) with npx playwright test --project=journeys -g "will not go on" after npm run build:once. It fails on trunk's src/ (checked by reverting src/ and rebuilding). It runs real patches through the real engine in the pseudo-locale and scans the preview of a patch that lands on the contributor's own edits, the failure notice when that patch does not fit, the refusal of a non-GitHub pull request URL, a pull request's preview (with git:preview-pr stubbed), and the notice once a patch is applied.

What the journey cannot reach, and is covered in the unit suite in the pseudo-locale instead:

  • The pull-request framings of a failure (closed, stale, the contributor's own work in the file, a file the applied patch brought): they need a real pull request that fails against a real checkout. tests/unit/apply-conflict.test.cjs reads each one against the exact string it should choose.
  • Main's own sentences, which the window shows only when they are not replaced by a breakdown: patch-apply.integration, patch-plan, patch-sources, trac-view and ipc-wiring each check theirs.
  • A checked-out pull request's notice, whose headline belongs to another batch (Translate the Terminal and Logs output #627).

For a look by hand, either platform, current head: run npx electron . --lang=en-XA from the repository root, open a Core site, edit src/wp-login.php, then on Apply a patch or PR choose Diff and pick a .diff that changes the line you edited.

  1. The preview says you have your own edits to src/wp-login.php, bracketed and accented.
  2. Click Apply and rebuild. The card says how many of the patch's changes no longer fit, and lists the file. Every sentence is bracketed, apart from the path and the lines of code.
  3. On Pull request, paste https://gitlab.com/a/b/pull/1 and click Apply PR. The refusal is bracketed (with a full stop after the bracket, see Risks).

What must not have happened: in English, nothing on the card reads differently from trunk. The English journeys for the card (apply-card, pr-checkout, gutenberg-site, watch-during-changes, ticket-card, site-header) still pass unchanged.

Risks and limitations

Related

Fixes #628. Part of #622.


Design decisions and alternatives considered
  • Two strings where a sentence names a file count as well as a change count. _n takes one number. The plural follows the change count, as Translate patch and pull request apply failures #628 asks, and the file count picks between a one-file and a many-file string. The alternative, a nested _n for "in %d files" spliced in, is the splice Wrap the rest of the app's strings for translation #622 rules out.
  • Joined sentences. The pull-request headlines that say "does not fit" and then name whose files failed are built from whole sentences joined with a space, not one string per combination. Each piece is a complete sentence, and four cases times two file counts would have meant sixteen near-duplicate strings for translators.
  • reverting: '' for an unnamed patch. describeApplyFailure used to get 'That patch' as the label. Now it gets '' and words the unnamed case as its own sentence, so "is this a revert" is read from the type of the option (typeof reverting === 'string') rather than from the label being truthy.
  • No scanner change. Nothing in this batch puts an element inside a sentence, so the unwrapped() hunk from Translate the pull request destination and GitHub sign-in #643 is not needed here. The "Near %s" line is cut at the code by design, and the journey leaves its two pieces out by name.
Review outcome (required — see AGENTS.md)
  • Review: completed. Reviewer: a fresh-context Explore subagent, given only the diff and .github/instructions/code-review.instructions.md. Reviewed head aa3bd889ba64565bfc773fdb690dbb72c0d19c2c, base 9fd167e6c370837bd832dbe87a46abf78ec55053. Lint and the unit suite were clean before the pass. Outcome: 1 [fix here] · 2 [follow-up]. Security, performance and cross-platform: no findings.
    • 🟡 [fix here], architecture: patch-apply.js put __('writing %s') around the app's own translated fragments (git apply exited %s, could not verify %1$s after git apply: %2$s). Fixed: each case is now one whole string, and writing %s wraps only Git's own stderr line. The English is unchanged.
    • 🔵 [follow-up]: '(unnamed binary file)' in planApply's skip list (patch-plan.cjs) was not wrapped, so the preview showed it in English inside a translated sentence. Fixed here (one line).
    • 🔵 [follow-up]: the Trac attachment URL refusal in trac-view.js was not wrapped. Fixed here (one line). The rest of that finding is prSubmissionRefusal/prSubmissionBlocked, which belong to Translate the Review & submit dialog #635, and the 'Site is not registered' guard, which is left as it is (see Risks).
    • Style notes acted on: the apply-card.cjs header comment no longer claims watch-activity.cjs and update-plan.cjs are translated, and the pull request preview's install note reuses the apply card's existing msgid instead of adding a second one. Not acted on: a context for the generic '%1$s and %2$s' (nothing else uses it yet), and the awkward "which you applied" wording, which keeps the English as it was.
    • The reviewer confirmed that no English changed, that the one equality match (failures to conflicts) is the same translated string on both sides, that nothing is translated at module load, and that every new test fails without the wrapping.
  • Since review: aa3bd88 → 729dd12. The author checked the fixes above against the same standard: lint, npm test and npm run i18n:pot are clean, and the i18n, apply-card, patch-apply and pr-checkout journeys pass. No new findings. Then 729dd12 → 46c2cd4: a rebase onto trunk e94f507 after Translate the Review & submit dialog #635 merged, with two conflicts. In pr-checkout.cjs there is now one @wordpress/i18n import, Translate the Review & submit dialog #635's wrapping of prSubmissionRefusal/prSubmissionBlocked is kept, and this PR's wrapping of the rest is kept. In i18n.spec.js both appended tests are kept in full, and this PR's journey now uses trunk's top-level write/LOGIN import. The author checked the rebase but it was not re-reviewed: npm ci, build, lint, npm run i18n:pot, npm test (2041 pass, 2 skipped, 0 fail) are clean. The full journey suite passed 131 of 131 on the third full run. The first two full runs each had 2 timeouts, in toasts.spec.js:114 (the Dismiss tooltip) and one site-header.spec.js menu hover (:212, then :145). Each passed when rerun alone, and both specs passed three times on trunk's src/ and three times on this branch, but no trunk run was made under the same machine load, so a slower settle caused by this branch is not ruled out. CI's journeys are the check.
Implementation notes
  • prCheckoutRefusal runs in main as well as in the renderer: main words the refusal for the terminal and the done event, and the renderer passes an unknown code's error through as main worded it.
  • apply-conflict.cjs pairs a failure with its breakdown by string equality. Both come from the same conflictSentence call in main, so the pair still matches when translated, and patch-apply.integration asserts it in the pseudo-locale.
  • Checks run from the repository root: npm run build:once, npm run lint, npm test (2040 passed, 2 skipped, 0 failures), npm run i18n:pot (every new string with a placeholder has its translators: comment in the .pot). The full journey suite: 125 passed at aa3bd88, and the four journeys touched by the follow-up commit passed again at 729dd12.
Screenshots or recording

No layout changes: the same notices show the same English. The pseudo-locale journey above is the visual check.

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: Repository: WordPress/contributor-toolkit/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: bfda6362-9b30-4798-ab4e-bceff307b5ed

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Wrap what the apply card says when a patch or a pull request will not go
on, and what it says once one is applied: the failure breakdown and its
pull-request framings (apply-conflict.cjs), the applied-patch notice and the
preview's attribution (applied-layer.cjs), a pull request's preview,
checkout and refusals (pr-checkout.cjs), the hook's own errors, and main's
side of it (patch-apply.js, patch-plan.cjs, patch-sources.cjs, trac-view.js
and the git:apply-patch handler).

Sentences that were spliced are written whole: one per file count and per
case, _n on the count they are about, separate strings for an unnamed patch
and for an issue against a ticket. REASONS and REVERT_REASONS become
functions. English is unchanged.
… the review found

The review found 'writing %s' wrapping the app's own fragments; each case is
now one string. Also wraps the unnamed binary file in the preview's skip list
and the Trac attachment URL refusal, and reuses the apply card's existing
package-lock.json sentence rather than adding a second msgid.
@ryanwelcher
ryanwelcher force-pushed the fix/628-apply-failure-strings branch from 729dd12 to 46c2cd4 Compare October 6, 2026 19:09
ryanwelcher added a commit that referenced this pull request Oct 6, 2026
## Why

The Open a pull request card, its GitHub sign-in, and the errors main
sends it were plain English in a translated window. This is the #625
batch of #622, which needed the catalog in main from #634.

## What changes

- **The card** (`pull-request-destination.jsx`) wraps every string.
Sentences with a link or code in them use `createInterpolateElement`,
with logins, branches and repository names passed in as elements so they
are never read as markup. The failure messages are a function instead of
a module-level constant.
- **Per-project text** in `project-type.cjs` (`cards.*`) is getters,
read when the card renders. Main's refusal with nothing linked used to
splice the work item's label into a sentence; it is now
`cards.prNeedsWorkItem`, one sentence per project type.
- **Main process:** the sign-in errors in `github-auth.cjs` and the pull
request errors in `github-pr.cjs` are wrapped, including the step labels
passed to `failure()`, which the issue did not list. The "Trunk has
moved" message uses `_n`, and a GitHub failure is composed with `%1$s:
%2$s%3$s` so a translator can change the punctuation. GitHub's own
reason and the bracketed diagnostic for GitHub support stay as they are.

Not wrapped, per #622: the pull request body and the default pull
request title, because they leave the app.

## How to test this

The new journeys cover the card in the pseudo-locale on both projects.
Run them from the repository root on either platform:

```
npx playwright test --project=journeys tests/e2e/journeys/i18n.spec.js -g "pull request card"
```

Both fail without this change. The Core journey walks every state #625
lists: signed out, "Not now" and "Show this again", the device code,
signed in with the form and its fold, a `rate-limited` failure, a dry
run, and a pull request opened from a checkout behind trunk. The
Gutenberg one walks signed out, signed in with the fold open, and the
opened pull request, checking the words that are Gutenberg's own.

What the journeys cannot reach is a real GitHub. They stub every
`github:*` handler, so the main-process errors never reach the screen in
them. Unit tests in `github-auth`, `github-pr` and `pr-stage` cover
those strings in the pseudo-locale instead. To see one on screen by hand
(either platform, current head):

1. From the repository root, run `npx electron . --lang=en-XA`.
2. Open a Core site with a linked ticket and an edit, and click **Review
& submit changes**.
3. In **Open a pull request**, click **Sign in with GitHub** and finish
in the browser.
- Expected: every line of the card is accented and in brackets, apart
from your login, your fork's name, the code and the branch.

**What must not have happened:** no English an English speaker sees has
changed. The English journeys in `open-pull-request.spec.js`,
`review-changes.spec.js` and `pr-checkout.spec.js` still pass with their
exact strings.

## Risks and limitations

- The other open batches (#636, #644, #645, #646) also append tests to
the end of `i18n.spec.js`, so whichever merges later resolves that by
keeping both.
- One finding is deferred: `prFailureMessage` decides between five
user-facing strings inside a `.jsx` file the suite cannot load. It
predates this PR (it was the old constant map), so moving it to a `.cjs`
module with a test is a follow-up.

## Related

Fixes #625. Part of #622. Follows #634.

---

<details>
<summary>Design decisions and alternatives considered</summary>

- **Not stacked on #635.** #622 asks for batches that review on their
own. Now that #635 has merged, its `unwrapped()` scanner change and the
`@wordpress/element` dependency, which this PR also needed, come from
trunk.
- **The textarea placeholder** is three strings joined with a newline. A
single `__()` with `\n` in it is refused by
`@wordpress/i18n-no-collapsible-whitespace`.
- **"Signed in as …"** is wrapped in its own `<span>`, so the sentence
and the **Sign out** button beside it are separate elements, as in #635.

</details>

<details>
<summary>Review outcome (required — see AGENTS.md)</summary>

2 [fix here] · 1 [follow-up]. Both [fix here] findings are fixed; the
follow-up is deferred (see Risks).

- 🟡 The Gutenberg journey linked no issue, so it never showed the form,
the fold or the loop-back. Fixed: it links one in the record and walks
those states.
- 🔵 No test showed the main-process strings or the stage labels come out
translated. Fixed: pseudo-locale unit tests for the scope refusal, a
composed `failure()`, the plural stale message and `prStageLabel`.
- Style: three dead entries in the journey's `THEIRS` filter were
removed.

- **Review:** completed. Fresh agent context, head `11ef362` / base
`c34aa45`. `npm run lint` clean, `npm test` 2002 pass / 0 fail, i18n and
PR journeys 25 pass.
- **Since review:** `11ef362 → ef457fd`. The only change is the test
fixes above, checked by the authoring session, not re-reviewed by a
fresh context. `src` and `tests` lint clean, unit suite 2005 pass / 0
fail, i18n journeys 12 pass. The review did not run the journeys itself;
it was read-only.
- **After #635 merged:** `ef457fd → 9559a1f` (trunk merged from GitHub)
→ `012fca8` (trunk at `e94f507` merged; `i18n.spec.js` resolved as
trunk's file plus this PR's two tests, nothing else changed). Checked by
the authoring session, not re-reviewed: `src` and `tests` lint clean,
unit suite 2036 pass / 0 fail, i18n, open-pull-request, review-changes
and pr-checkout journeys 31 pass.

</details>

<details>
<summary>Screenshots or recording</summary>

None. Nothing changes on screen in English, and the pseudo-locale
journeys are the check for the translated state.

</details>

🤖 Generated with [Claude Code](https://claude.com/claude-code)
…e-strings

# Conflicts:
#	tests/e2e/journeys/i18n.spec.js
ryanwelcher added a commit that referenced this pull request Oct 7, 2026
## Why

The setup checklist and the trunk update's cards and banners were still
plain English in a translated app. This is the #626 batch of #622.

## What changes

- **Wrapped:** the setup checklist, the trunk update card, the question
an update asks over edits, the banners for an old trunk and an
incomplete update, the "Up to date with trunk" notice, and the sentences
they take from `setup-steps.cjs`, `update-plan.cjs`, `next-action.cjs`
and `project-type.cjs`'s `setup` copy. Also the dirty-tree dialog's
errors in `use-trunk-update.jsx` and the alert in `use-dev-server.jsx`.
- **Translated when shown, not at load:** `update-plan.cjs`'s messages
and step labels were module-level constants and are now functions
(`SKIP_INSTALL_MESSAGE` became `skipInstallMessage()`, and so on);
`project-type.cjs`'s `setup.*` are getters, like `description` already
was.
- **Whole sentences instead of parts:** the step counters
(`sprintf('step %1$d of %2$d')`), the file count and the day count
(`_n`), "Next step: %s", and the update summary, which was three
fragments and is now one of four sentences (`updateSummarySentence`).
The link-ticket hint took the work item's label as a noun ("Link a %s…")
and now takes its kind and has one sentence each, as #630 asks for.
- Commands and paths (`npm install`, `npm run build`, `package.json`,
`src/`) are passed in as `%s`.
- Deliberately not here: the lines the app writes to the Terminal belong
to #627, apart from the skip-install message, which #626 names because
the update card shows it too.

## How to test this

Covered by journeys in `tests/e2e/journeys/i18n.spec.js`, all red
without this change. From the repository root: `npx playwright test
--project=journeys tests/e2e/journeys/i18n.spec.js -g "checklist|trunk
banners|did not finish"`. In the en-XA pseudo-locale they reach the
checklist with steps done and ready, the checklist after a failed
install, the old-trunk banner, the question an update asks over an edit
(each answer chosen), the update card on its first step, and the banner
after an update that did not finish, and fail on any visible English.

To look at it by hand, on either platform, on the current head: run `npx
electron . --lang=en-XA` from the repository root and open a site whose
trunk is more than 14 days old.

1. **Expected:** the banner about the old trunk and its **Update to
latest trunk** button are accented and in brackets.
2. Edit a file in the site and click that button. **Expected:** the
dialog that asks what to do with the edit is bracketed throughout, apart
from the file's path.

**What must not have happened:** in English, the checklist, the cards
and the banners read exactly as before.

Not reached by a journey, and why:

- The setup chain's "Setting this site up for you — step N of M" and
"Setup stopped." notices: they only appear after a new site's clone,
which a journey would have to fake end to end.
- The "Up to date with trunk" notice: it needs an update to finish. Its
sentences come from `updateSummarySentence`, which a unit test covers.
- The alert about starting the server before the build: no control in
the app can reach it (the checklist's button is disabled until the build
is done, and the header and details only offer the server once the
wizard is skipped). It is wrapped anyway.

## Risks and limitations

- Review: 2 [fix here] · 0 [follow-up], both low, both fixed.
- `tray.spec.js`'s "the tray's edge is moved with the arrow keys…"
failed once in the full run. It fails about one run in five on `trunk`
too, so it is not this branch.
- The other open batches (#643, #644, #645, #646) also append tests to
the end of `tests/e2e/journeys/i18n.spec.js`, so whichever merges later
resolves that by keeping both.

## Related

Fixes #626. Part of #622.

---

<details>
<summary>Design decisions and alternatives considered</summary>

The dirty-tree answers' details carry their own leading dash ("— a .diff
on your machine…"), with the space outside the string: punctuation is
the translator's, and a string that starts with a space is easy to
break. The link-ticket hint takes `workItemNoun` instead of
`workItemLabel`, which removes one of the places #630 lists as splicing
the label into a sentence.

</details>

<details>
<summary>Review outcome (required, see AGENTS.md)</summary>

- **Review:** completed. Reviewer: a separate agent context (Claude
Code, read-only) against
`.github/instructions/code-review.instructions.md`; head `1790e42` /
base `8dbed36`. `npm run lint` clean, `npm test` 1954 pass / 0 fail,
journeys 117 pass. Outcome: 2 [fix here] · 0 [follow-up], both 🔵.
- Translatable-string rules [fix here]: the extractor gave the
dirty-tree details' notes to their labels as well, and gave the
dirty-tree errors' notes to the "Unknown error" fallback on the same
line. **Fixed** in `4b925eb`: each note sits directly above its own
string, and the fallback is on its own line.
- Tests [fix here]: nothing would fail if the new `update-plan.cjs`
message functions went back to English constants. **Fixed** in
`4b925eb`: a unit test loads a catalog after the module is required.
Checked by turning one back into a constant, which fails it.
- Notes on wording, also fixed: a stale comment in `work-item.cjs`, and
the translator example "2m 5s", which the app writes as "2m 05s".
  - It also confirmed the dev-server alert is unreachable from the UI.
- **Since review:** `1790e42 → 4b925eb` checked; the change is the fixes
above. Lint clean, `npm test` 1955 pass, journeys 116 pass and 1 known
flaky (above) on the new head.
- **After #635 merged:** `4b925eb → 7e0e1ad`, rebased onto trunk
`e94f507`. Only `tests/e2e/journeys/i18n.spec.js` conflicted: resolved
as trunk's file plus this PR's four tests, dropping this PR's copies of
the imports and `MY_EDIT`, which trunk now has. #635 brought the
identical `trunkDate` change to `git-site.cjs`, so it is no longer in
this diff. Checked by the authoring session, not re-reviewed: build and
lint clean, `npm test` 2035 pass / 0 fail / 2 skipped, the full journey
suite 134 pass.

</details>

<details>
<summary>Implementation notes</summary>

- The renamed `update-plan.cjs` exports have no consumer left under
their old names in `src`, `tests` or `docs`.
- `main.js` reads `project-type.cjs` but never `setup`, so the getters
change nothing there.

</details>

<details>
<summary>Screenshots or recording</summary>

Nothing changes in English. In the pseudo-locale every screen in this
batch is bracketed, which the journeys assert.

</details>

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Francesco Bigiarini <francesco.bigiarini@gmail.com>
…e-strings

# Conflicts:
#	src/trac-view.js
#	tests/e2e/journeys/i18n.spec.js
#	tests/unit/trac-view.test.cjs
…e-strings

# Conflicts:
#	src/main.js
#	tests/e2e/journeys/i18n.spec.js
@ryanwelcher
ryanwelcher merged commit a2eb226 into trunk Oct 7, 2026
8 checks passed
@ryanwelcher
ryanwelcher deleted the fix/628-apply-failure-strings branch October 7, 2026 14:15
ryanwelcher added a commit that referenced this pull request Oct 7, 2026
…h error (#647)

## Why

#630 is the last batch of #622. This PR is the part of it that doesn't
wait on the five open batches (#636, #643, #644, #645, #646): the mail
dialog, the Playground web server and the copy-path error were still
plain English, and `applyDoneMessage` spliced a verb and a noun into an
English sentence for any pair it had no sentence for.

## What changes

- The mail dialog: "From:", "To:", "CC:", "Date:", the Rendered and Raw
tabs, and the "Email" title of a mail with no subject.
- The Playground web server: its Start and Stop buttons, the loading
announcement, "Starting…", the panel's heading, "Stopped", and "Server
exited with code %d".
- The copy-path failure: the alert and the clipboard-unavailable message
in it.
- `applyDoneMessage`'s fallback is `_x('Done', 'an action that
finished')` instead of `` `${verb} the ${noun}` ``. No caller reaches it
today: the three in `use-apply-patch.jsx` pass only the five pairs it
has sentences for.

Not in this PR, because another open PR already wraps them or because
#630 says to wait for one:

| Item | Where |
|---|---|
| `COPY_BUTTON_LABELS` | #646 |
| "Dismiss" on the deep-link notice, "Choose application…" | #644 |
| "Setting up new site…" | #636 |
| `use-dev-server.jsx` leftovers | after #646 |
| `apply-card.cjs:191` period | after #645 |
| `project-type.cjs` `workItem.label` | after #643, #636, #644 |
| The full `--lang=en-XA` walk | after all five merge |

## How to test this

Two en-XA journeys cover this, and both are red without the change:

```bash
# from the repository root
npm run build:once
npx playwright test --project=journeys tests/e2e/journeys/i18n.spec.js -g "a mail the site sent|Playground web server"
```

- **a mail the site sent is fully translatable**: a mail on its Rendered
tab and its Raw tab, and a mail with no subject, whose dialog is titled
"Email".
- **the Playground web server is fully translatable**: stopped, starting
(including the live-region announcement), running, and after it exits
with code 1.

The English journeys in `mail.spec.js` still pass, so an English speaker
sees the same text as before.

What the journeys can't reach is the copy-path alert. It needs the
clipboard to refuse a write, and I couldn't stage that.

To look by hand (any platform, current head): run `npx electron .
--lang=en-XA` from the repository root, start a site's dev server, and
open a mail from the Email tray. Every label and both tabs show accented
and in brackets. The Playground web server only appears in a build that
ships `local-playground-web`, so a checkout won't show it.

**What must not have happened:** with no `--lang`, the mail dialog and
the web server read exactly as they did before.

## Risks and limitations

- Found while writing the web server journey: `@wordpress/a11y` adds a
screen-reader-only "Notifications" paragraph at DOM-ready, before the
app loads its locale, so it stays English. #630 doesn't list it, so it
isn't fixed here.
- The web server's other errors come from main (`'local-playground-web
directory not found.'`, or a thrown error) and stay English. That
belongs with the main-process strings, not this renderer batch.

## Related

Part of #630. Part of #622.

---

<details>
<summary>Review outcome</summary>

**4 [fix here] · 2 [follow-up]. All 4 fixed in c9c5c0c.** One more
finding was dropped as wrong (below).

- **Review:** completed by a separate agent with a fresh context,
against `.github/instructions/code-review.instructions.md`. Reviewed
head 4f8b808, base e94f507 (trunk). `npm run lint` clean, `npm test`
2033/2033.
- **Fixed:**
- The web server journey could pass with the loading announcement
unwrapped, because `speak()` writes outside the area scanned. It now
asserts `#a11y-speak-polite`.
- `__('Email')` only shows for a mail with no subject. The journey now
opens one.
- The `Done` fallback wasn't in the unit test that runs every sentence
through a catalog. It is now.
- A bare `__('Done')` would share a msgid with the `Done` button in
`@wordpress/components`. It now has a context.

Each new journey assertion was checked to fail with its string
unwrapped.
- **Deferred:**
- The mail journey excuses any text that is part of the mail. No label
is excused today, but matching whole text nodes would be stricter.
  - The web server's main-process error strings (see Risks).
- **Dropped:** the finding that "Dismiss", "Choose application…",
`COPY_BUTTON_LABELS` and "Setting up new site…" were unwrapped. The
reviewer read the issues, not the PR diffs; #644, #646 and #636 wrap
them.
- **Since review:** 4f8b808 → c9c5c0c, which contains only the fixes
above. Lint, unit tests and the i18n and mail journeys were rerun on
c9c5c0c.

</details>

Nothing on screen changes in English, so there are no screenshots.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Francesco Bigiarini <francesco.bigiarini@gmail.com>
ryanwelcher added a commit that referenced this pull request Oct 7, 2026
…eaks left in wrapped code (#653)

## Why

#630 is the last batch of #622. #647 did the part that didn't depend on
the other batches. This does the rest, now that #636, #643, #644, #645
and #646 are merged: the en-XA sweep #630 asks for, and the two rule
breaks it named in already-wrapped code.

## What changes

**Found by the sweep:**
- xterm's own screen-reader strings: the terminal input's label
("Terminal input") and its "too much output" announcement, set through
`Terminal.strings` before each terminal is created.
- Main's refusal to submit while a patch is applied. It was `${label} is
applied. Revert it…`, a sentence spliced around the name, and it's shown
as-is in the patch pane. It's now two whole sentences, one with the name
and one without.
- The Playground web server's three errors from main (no folder to
serve, exits before it's ready, times out). #647's review deferred these
here.

**Rule breaks #630 named:**
- `apply-card.cjs` adds a full stop to a headline that lacks one. #630
said to drop it once main's errors carried their own. They don't, and
can't all: `patch-apply.js` messages double as terminal bullets, the
fallback headline is Git's own stderr line, and four paths pass a thrown
error's text. So the full stop stays but goes through `_x('%s.', …)` for
the translator to choose, and a sentence already ended in any script's
mark (`\p{Sentence_Terminal}`) keeps it.
- `workItem.label` is removed from both project types. Nothing read it
once the sentences that named a work item became one per kind.

**Already done, nothing to change:** `use-dev-server.jsx` had nothing
left after #646.

## How to test this

Any platform, current head. The suites cover it:

```bash
# from the repository root
npm test
npm run build:once
npx playwright test --project=journeys tests/e2e/journeys/i18n.spec.js
```

- `ipc-wiring.test.cjs` has two new pseudo-locale tests, for main's
refusal (with and without a name) and the web server's failures. Both
are red on the old `main.js`.
- `apply-card.test.cjs` checks the translated full stop, and that
sentences ended in `。`, `؟` and `।` keep their mark.
- The i18n Terminal journey now finds the input by its translated name.
The apply card journey now expects the whole refusal, full stop
included, as one translated string.

To look by hand: run `npx electron . --lang=en-XA` from the repository
root, open a site, open the Terminal tray, and inspect the terminal's
textarea. Its `aria-label` is bracketed and accented.

**What must not have happened:** with no `--lang`, nothing reads
differently. The refusal, the web server errors and the terminal's label
are the same English as before, and `mail.spec.js`, `terminal.spec.js`
and the rest of the English journeys pass unchanged.

## Risks and limitations

- **What the sweep covered:** I launched en-XA with a Core site and a
Gutenberg site, opened every tray, log tab and settings tab, the site
menu and the create dialog, and scanned the whole window. After the
xterm fix, the only English left was product names, commands, the `$`
prompt and PHP version numbers. It didn't open every dialog or failure
state; the existing i18n journeys cover many of those.
- **Left on purpose** (#622's do-not-wrap list): main's argument guards
(`'Site is not registered'`, `'No running script'`), the `github-prs.js`
and `trac-view.js` errors that are replaced by a status before display,
and `console.error` messages.
- **Untested:** `'Timed out starting web server'` is wrapped but has no
test. It needs fake timers for its 20-second wait.
- #648 (the a11y "Notifications" text) is separate.

## Related

Closes #630. Part of #622.

---

<details>
<summary>Review outcome</summary>

**1 [fix here] · 1 [follow-up]. The fix is applied in c2eb4ba.**

- **Review:** completed by a separate agent with a fresh context,
against `.github/instructions/code-review.instructions.md`. Reviewed
head c53312a, base 8303849 (trunk). `npm run lint` clean, `npm test`
2081/2081, full journey suite 147/147 on c53312a.
- **Fixed:** the list of marks that end a sentence missed Devanagari
`।`, Armenian, Ethiopic, Khmer and Myanmar, so a Hindi sentence would
have got a second mark. It now uses `\p{Sentence_Terminal}`, with a
Hindi case in the test. The doc comment's over-long line was rewrapped.
- **Deferred:** a test for the web server's 20-second timeout (see
Risks).
- **Confirmed by the review:**
- xterm reads `promptLabel` in `open()` and `tooMuchOutput` on each
announcement, so setting both before `new Terminal` is early enough.
- The refusal's English is unchanged for every label, including a blank
one.
  - Nothing reads `workItem.label`.
  - No English journey asserts a changed string.
- **Since review:** c53312a → c2eb4ba, which contains only the fix
above. Lint, unit tests (2081) and the i18n journeys were rerun on
c2eb4ba. One i18n journey (site details and menu, which this branch
doesn't touch) failed once in that run, then passed three times on its
own and in the full run before. That's a flake.

</details>

Nothing on screen changes in English, so there are no screenshots.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Translate patch and pull request apply failures

1 participant