From 5cff08e810b6dc393987b316ddd65c1c842d37b6 Mon Sep 17 00:00:00 2001 From: JuanMa Date: Thu, 6 Aug 2026 17:16:36 +0200 Subject: [PATCH 1/3] Fix the apply panel's error/copy states, and apply a PR by URL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three fixes from real testing plus one requested addition, all in the "Try someone else's patch" panel: - The apply error stayed on screen after Cancel. Cancel now clears it, so dismissing a failed attempt leaves no stale warning behind. - The panel's sub-copy ("Apply a .diff/.patch file…") contradicted itself while a pull request preview was open. It is now hidden whenever a preview or the apply chain is showing — the preview card speaks for itself — and the idle copy names pull requests as well as files. - The failure message ran two sentences together ("…no longer applies The checkout was not changed."). A period is added when the reason does not end in punctuation. - New: paste a pull request URL (or number) and apply it directly, without it having to be linked to the ticket. parsePrRef (pure, in patch-sources.cjs) accepts a wordpress-develop PR URL or a bare number and rejects other repos, other hosts and non-PR URLs; the number then rides the existing previewPr → fetchPrDiff → preview → apply flow, so it shares the same trust boundary as the linked-PR list. Refs #109, #11, part of #110. Co-Authored-By: Claude Opus 4.8 --- src/patch-sources.cjs | 36 ++++++++++++++++++++++++++- src/renderer/index.jsx | 49 +++++++++++++++++++++++++++++++------ test/patch-sources.test.cjs | 29 ++++++++++++++++++++++ 3 files changed, 105 insertions(+), 9 deletions(-) diff --git a/src/patch-sources.cjs b/src/patch-sources.cjs index 06a5818..72c482f 100644 --- a/src/patch-sources.cjs +++ b/src/patch-sources.cjs @@ -20,6 +20,39 @@ */ const TICKET_HOST = 'core.trac.wordpress.org'; +const PR_REPO_PATH = 'WordPress/wordpress-develop'; + +/** + * Resolves what a contributor pastes into "apply a PR" to a pull request + * number. Accepts a bare number or a wordpress-develop PR URL (with any + * trailing `/files`, `#…`, `?…`). A PR from another repo is rejected by name — + * its diff would not fit this checkout. + * + * @param {string} input + * @return {{ok: true, number: number}|{ok: false, error: string}} + */ +function parsePrRef(input) { + const raw = typeof input === 'string' ? input.trim() : ''; + if (!raw) return { ok: false, error: 'Enter a pull request URL or number.' }; + + if (/^#?\d+$/.test(raw)) return { ok: true, number: Number(raw.replace('#', '')) }; + + let parsed; + try { + parsed = new URL(/^[a-z][a-z0-9+.-]*:\/\//i.test(raw) ? raw : `https://${raw}`); + } catch { + return { ok: false, error: 'That is not a pull request URL or number.' }; + } + if (parsed.hostname.toLowerCase() !== 'github.com') { + return { ok: false, error: 'Only github.com pull requests are supported.' }; + } + const match = /^\/([^/]+\/[^/]+)\/pull\/(\d+)(?:[/?#]|$)/.exec(parsed.pathname + (parsed.pathname.endsWith('/') ? '' : '/')); + if (!match) return { ok: false, error: 'That does not look like a pull request URL.' }; + if (match[1].toLowerCase() !== PR_REPO_PATH.toLowerCase()) { + return { ok: false, error: `Only ${PR_REPO_PATH} pull requests can be applied here.` }; + } + return { ok: true, number: Number(match[2]) }; +} /** * True when a PR body cites this exact ticket. The negative lookahead stops @@ -95,5 +128,6 @@ module.exports = { TICKET_HOST, bodyCitesTicket, parseLinkedPrs, - classifyHttpFailure + classifyHttpFailure, + parsePrRef }; diff --git a/src/renderer/index.jsx b/src/renderer/index.jsx index 21dba9b..e021f08 100644 --- a/src/renderer/index.jsx +++ b/src/renderer/index.jsx @@ -21,6 +21,7 @@ import { planDevServerStart, formatElapsed } from './dev-server-command.cjs'; import { pathBasename } from './path-basename.cjs'; import { trunkAgeInfo, planUpdateSteps, updateStepStatuses, SKIP_INSTALL_MESSAGE, planApplySteps, APPLY_STATE_TO_STEP } from './update-plan.cjs'; import { pickLatest } from '../latest-patch.cjs'; +import { parsePrRef } from '../patch-sources.cjs'; import { parseTicketRef, ticketUrl } from './trac-ticket.cjs'; const TERMINAL_ALLOWED_SCRIPTS = ['build', 'build:dev', 'dev', 'test', 'watch', 'grunt']; @@ -836,6 +837,7 @@ function SiteRow({ sitePath, initialized, createdAt, label, onInitialized, onSit const [applyNeedsInstall, setApplyNeedsInstall] = useState(false); const [applyError, setApplyError] = useState(''); const [appliedPatch, setAppliedPatch] = useState(null); + const [prUrlInput, setPrUrlInput] = useState(''); const [dirtyModalOpen, setDirtyModalOpen] = useState(false); const [dirtySaving, setDirtySaving] = useState(false); const [dirtyFiles, setDirtyFiles] = useState([]); @@ -1766,6 +1768,15 @@ function SiteRow({ sitePath, initialized, createdAt, label, onInitialized, onSit } }; + // Apply a PR straight from a pasted URL or number, without needing it to be + // linked to the ticket — same fetch → preview flow as the linked-PR list. + const previewPrFromInput = () => { + const parsed = parsePrRef(prUrlInput); + if (!parsed.ok) { setApplyError(parsed.error); return; } + setPrUrlInput(''); + previewPr({ number: parsed.number, url: `https://github.com/WordPress/wordpress-develop/pull/${parsed.number}` }); + }; + const runApply = ({ reverse = false } = {}) => { const state = terminalStateRef.current; if (state.running) { @@ -2527,9 +2538,11 @@ function SiteRow({ sitePath, initialized, createdAt, label, onInitialized, onSit {skipInit ? (
Try someone else's patch
-
- Apply a .diff or .patch file to this checkout and rebuild, so you can test the work before adding your own. Your own changes are left alone. -
+ {!applyPreview && !isApplying ? ( +
+ Apply a pull request or a .diff/.patch file to this checkout and rebuild, so you can test the work before adding your own. Your own changes are left alone. +
+ ) : null} {appliedPatch && !isApplying ? (
@@ -2570,7 +2583,7 @@ function SiteRow({ sitePath, initialized, createdAt, label, onInitialized, onSit ) : null}
- +
) : null} @@ -2593,15 +2606,35 @@ function SiteRow({ sitePath, initialized, createdAt, label, onInitialized, onSit {applyError ? (
- {applyError} The checkout was not changed. + {/[.!?]$/.test(applyError.trim()) ? applyError : `${applyError.trim()}.`} The checkout was not changed.
) : null} {!applyPreview && !isApplying ? (
- +
+
+ { setPrUrlInput(value); setApplyError(''); }} + onKeyDown={(event) => { if (event.key === 'Enter') { event.preventDefault(); previewPrFromInput(); } }} + disabled={isUpdating || installing || building} + placeholder="Paste a pull request URL or number" + aria-label="Pull request URL or number" + /> +
+ +
+
+ +
) : null}
diff --git a/test/patch-sources.test.cjs b/test/patch-sources.test.cjs index 2426b07..927963c 100644 --- a/test/patch-sources.test.cjs +++ b/test/patch-sources.test.cjs @@ -79,3 +79,32 @@ test('classifyHttpFailure: a spent rate limit is told apart from a plain error ( assert.strictEqual(classifyHttpFailure(500, {}), 'error'); assert.strictEqual(classifyHttpFailure(404, {}), 'error'); }); + +const { parsePrRef } = require('../src/patch-sources.cjs'); + +test('parsePrRef: a bare number or #number resolves (issue #11)', () => { + assert.deepStrictEqual(parsePrRef('4496'), { ok: true, number: 4496 }); + assert.deepStrictEqual(parsePrRef(' #4496 '), { ok: true, number: 4496 }); +}); + +test('parsePrRef: a wordpress-develop PR URL resolves, with trailing bits (issue #11)', () => { + assert.strictEqual(parsePrRef('https://github.com/WordPress/wordpress-develop/pull/4496').number, 4496); + assert.strictEqual(parsePrRef('https://github.com/WordPress/wordpress-develop/pull/4496/files').number, 4496); + assert.strictEqual(parsePrRef('https://github.com/WordPress/wordpress-develop/pull/4496#pullrequestreview-1').number, 4496); + // Missing scheme, copied from the address bar. + assert.strictEqual(parsePrRef('github.com/WordPress/wordpress-develop/pull/4496').number, 4496); +}); + +test('parsePrRef: a PR from another repo is rejected by name (issue #11)', () => { + const res = parsePrRef('https://github.com/WordPress/gutenberg/pull/4496'); + assert.strictEqual(res.ok, false); + assert.match(res.error, /wordpress-develop/); +}); + +test('parsePrRef: non-PR and empty input are rejected with a reason (issue #11)', () => { + assert.strictEqual(parsePrRef('').ok, false); + assert.strictEqual(parsePrRef(' ').ok, false); + assert.strictEqual(parsePrRef('https://github.com/WordPress/wordpress-develop/issues/4496').ok, false); + assert.strictEqual(parsePrRef('https://example.com/pull/1').ok, false); + assert.strictEqual(parsePrRef('not a url').ok, false); +}); From 1babb842350d7fd2f113a441736edc19f6084b58 Mon Sep 17 00:00:00 2001 From: JuanMa Date: Fri, 7 Aug 2026 06:28:59 +0200 Subject: [PATCH 2/3] Rename the apply panel to 'Apply a patch or PR' 'Try someone else's patch' undersold it once the panel also applied pull requests and the contributor's own downloaded files. The heading now names what the panel does. Co-Authored-By: Claude Opus 4.8 --- src/renderer/index.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/renderer/index.jsx b/src/renderer/index.jsx index e021f08..5492db6 100644 --- a/src/renderer/index.jsx +++ b/src/renderer/index.jsx @@ -2537,7 +2537,7 @@ function SiteRow({ sitePath, initialized, createdAt, label, onInitialized, onSit {skipInit ? (
-
Try someone else's patch
+
Apply a patch or PR
{!applyPreview && !isApplying ? (
Apply a pull request or a .diff/.patch file to this checkout and rebuild, so you can test the work before adding your own. Your own changes are left alone. From 82ee3b40bd5f844927f65bdf20e579581c1b2c33 Mon Sep 17 00:00:00 2001 From: JuanMa Date: Fri, 7 Aug 2026 06:31:17 +0200 Subject: [PATCH 3/3] Show only patch files in the Trac attachments list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Screenshots and other non-patch attachments were listed with a 'not a patch' label, which is noise in a panel whose only action is to apply one. The list now shows just .diff/.patch files; when a ticket has attachments but none are patches, it says so. The parser still returns every attachment — the latest indicator and its tests rely on the full set — so this is purely what the panel displays. Co-Authored-By: Claude Opus 4.8 --- src/renderer/index.jsx | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/src/renderer/index.jsx b/src/renderer/index.jsx index 5492db6..c62bff1 100644 --- a/src/renderer/index.jsx +++ b/src/renderer/index.jsx @@ -1579,6 +1579,12 @@ function SiteRow({ sitePath, initialized, createdAt, label, onInitialized, onSit // pill and the "latest is a patch file" note. const latestPatch = pickLatest({ prs: ticketPatches?.items, attachments: tracAttachments?.items }); const latestIsAttachment = latestPatch?.kind === 'attachment'; + // The panel lists only what can be applied — screenshots and other non-patch + // attachments are noise here. The parser still returns them (pickLatest and + // tests rely on the full list); the filtering is purely what's shown. + const patchAttachments = (tracAttachments?.items || []).filter((a) => a.applyable); + const tracAttachmentsRead = tracAttachments + && (tracAttachments.status === 'ok' || tracAttachments.status === 'no-attachments'); const latestPill = (isLatest) => (isLatest ? ( Latest @@ -2452,8 +2458,8 @@ function SiteRow({ sitePath, initialized, createdAt, label, onInitialized, onSit
Opening the ticket on Trac…
) : null} - {tracAttachments && tracAttachments.status === 'no-attachments' ? ( -
This ticket has no attachments.
+ {tracAttachmentsRead && patchAttachments.length === 0 ? ( +
No patch files attached to this ticket.
) : null} {tracAttachments && (tracAttachments.status === 'challenge-timeout' || tracAttachments.status === 'error' || tracAttachments.status === 'closed') ? ( @@ -2466,9 +2472,9 @@ function SiteRow({ sitePath, initialized, createdAt, label, onInitialized, onSit
) : null} - {tracAttachments && tracAttachments.items && tracAttachments.items.length ? ( + {patchAttachments.length ? (
- {tracAttachments.items.map((att) => ( + {patchAttachments.map((att) => (
@@ -2481,17 +2487,13 @@ function SiteRow({ sitePath, initialized, createdAt, label, onInitialized, onSit {[att.author && `by ${att.author}`, att.dateText, att.sizeText].filter(Boolean).join(' · ')}
- {att.applyable ? ( - - ) : ( - not a patch - )} +
))}