Skip to content

Commit 46c2cd4

Browse files
committed
Write the patch's write failure as whole sentences, and wrap the rest 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.
1 parent 84e71a3 commit 46c2cd4

6 files changed

Lines changed: 21 additions & 12 deletions

File tree

‎src/patch-apply.js‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -588,8 +588,19 @@ async function applyPatchToDir({ dir, patchText, reverse = false, onLog = () =>
588588
return { ok: false, error, applied: [], skipped };
589589
}
590590
const written = await applyPatch(dir, applyText, { reverse, platform, prefix });
591-
// translators: %s: the exit code of the command git apply.
592-
let writeError = written.ok ? null : written.stderr.split(/\r?\n/).filter((line) => line.trim()).pop() || sprintf(__('git apply exited %s'), written.status);
591+
// Why the write failed, as one whole sentence: Git's own last line when it
592+
// printed one, the app's own words otherwise.
593+
let message = null;
594+
if (!written.ok) {
595+
const stderr = written.stderr.split(/\r?\n/).filter((line) => line.trim()).pop();
596+
if (stderr) {
597+
// translators: %s: why writing the patch failed, Git's own message, in English.
598+
message = sprintf(__('writing %s'), stderr);
599+
} else {
600+
// translators: %s: the exit code of the command git apply.
601+
message = sprintf(__('writing git apply exited %s'), written.status);
602+
}
603+
}
593604
// Windows Git can exit 0 without creating a file beneath a regular-file
594605
// parent (#413). Check the actual destinations before claiming success.
595606
// Git still decides the contents; this only detects an omitted write.
@@ -601,15 +612,13 @@ async function applyPatchToDir({ dir, patchText, reverse = false, onLog = () =>
601612
await fs.promises.lstat(path.join(dir, relPath));
602613
} catch (e) {
603614
// translators: 1: the path of a file the patch writes. 2: why it could not be read back, as the system said it.
604-
writeError = sprintf(__('could not verify %1$s after git apply: %2$s'), relPath, e.message);
615+
message = sprintf(__('writing could not verify %1$s after git apply: %2$s'), relPath, e.message);
605616
break;
606617
}
607618
}
608619
}
609-
if (writeError) {
620+
if (message) {
610621
const recovery = rollback(dir, snapshot);
611-
// translators: %s: why writing the patch failed, often Git's own message in English.
612-
const message = sprintf(__('writing %s'), writeError);
613622
if (recovery.length) {
614623
// translators: %s: the files that could not be put back, and why, separated by semicolons.
615624
onLog(`\n${sprintf(__('The patch could not be written, and the checkout could not be fully put back — it is in an unknown state. Could not undo: %s'), recovery.join('; '))}\n`);

‎src/patch-plan.cjs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -503,7 +503,7 @@ function planApply({ files, dirtyPaths = [] } = {}) {
503503
// applied from a text diff; one that carries its bytes is applied like
504504
// any other file (#385). Naming the first kind is the difference
505505
// between "this patch is partly unapplied" and a silent gap.
506-
unsupported: list.filter((f) => f.kind === 'binary' && !f.hasBinaryData).map((f) => f.path || '(unnamed binary file)'),
506+
unsupported: list.filter((f) => f.kind === 'binary' && !f.hasBinaryData).map((f) => f.path || __('(unnamed binary file)')),
507507
// Same rule the trunk update uses (#94): the lockfile moving is what
508508
// makes an install necessary rather than merely possible.
509509
needsInstall: touched.has('package-lock.json')

‎src/renderer/apply-card.cjs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@
77
// as it is: what a pull request's preview and banner say (pr-checkout.cjs,
88
// watch-activity.cjs), what an applied patch says (applied-layer.cjs), why a
99
// patch did not fit (apply-conflict.cjs) and the steps of an apply
10-
// (update-plan.cjs). Each is translated where it is written, not here.
10+
// (update-plan.cjs). Each is translated where it is written, not here; the
11+
// banner's and the steps' words are still English until their own batches.
1112
'use strict';
1213

1314
const { __, _n, sprintf } = require('@wordpress/i18n');

‎src/renderer/pr-checkout.cjs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,7 @@ function describePrPreview({ number, files = [], needsInstall = false, exists =
9696
headline,
9797
actionLabel: exists && moved && hasEdits ? __('Return to saved copy') : __('Apply and rebuild'),
9898
closedNote: state === 'closed' ? __('This pull request is closed. You can still check out its last head to investigate it.') : '',
99-
// translators: %s: the name of a file, package-lock.json.
100-
installNote: needsInstall ? sprintf(__('It changes %s, so dependencies will be installed before the rebuild.'), 'package-lock.json') : ''
99+
installNote: needsInstall ? __('It changes package-lock.json, so dependencies will be installed before the rebuild.') : ''
101100
};
102101
}
103102

‎src/trac-view.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ async function fetchAttachment(url) {
176176
// not the caller's string: the request rides the session cookie, so the URL
177177
// fetched has to be the one that passed the check.
178178
const safe = secureTracUrl(url);
179-
if (!safe) return { ok: false, error: 'Only https core.trac.wordpress.org attachments are allowed' };
179+
if (!safe) return { ok: false, error: __('Only https core.trac.wordpress.org attachments are allowed') };
180180

181181
let res;
182182
try {

‎tests/unit/pr-checkout-renderer.test.cjs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ test('a checkout\'s sentences are said in the locale: one per kind of work item,
125125

126126
const preview = describePrPreview({ number: 7, files: [{ path: 'a.php' }, { path: 'b.php' }], needsInstall: true, state: 'closed' });
127127
assert.equal(preview.headline, pseudo('PR #%1$d changes %2$d files.', 7, 2));
128-
assert.equal(preview.installNote, pseudo('It changes %s, so dependencies will be installed before the rebuild.', 'package-lock.json'));
128+
assert.equal(preview.installNote, pseudoLocalize('It changes package-lock.json, so dependencies will be installed before the rebuild.'));
129129
assert.equal(describePrPreview({ number: 7, files: [{ path: 'a.php' }] }).headline, pseudo('PR #%1$d changes %2$d file.', 7, 1));
130130
assert.equal(describePrPreview({ number: 7, exists: true, moved: true, hasEdits: true }).actionLabel, pseudoLocalize('Return to saved copy'));
131131
});

0 commit comments

Comments
 (0)