diff --git a/README.md b/README.md index f80a243..4a07536 100644 --- a/README.md +++ b/README.md @@ -20,10 +20,11 @@ If the Linear and ADO ticket chips don't link as expected, see `ticketRules` and ## How data is loaded -Two requests per PR, made by Tampermonkey with your GitHub session cookies: +Up to three requests per PR, made by Tampermonkey with your GitHub session cookies: - `///pull/.diff` for the file list, per-file additions and deletions, and blob SHAs (from the diff's `index` lines). - `///raw/HEAD/.github/CODEOWNERS` (falling back to `CODEOWNERS` and `docs/CODEOWNERS`) for ownership. +- `///pull//commits` with `Accept: application/json` for the PR's commit list and head SHA (only while "New commits since your last look" is on). This is roughly 2 KB per commit. If either fails (signed out, SSO expired, very large PR where GitHub refuses to render the diff), the panel says so and the features that depend on it switch off. Everything that only reads the page keeps working. @@ -47,6 +48,8 @@ If either fails (signed out, SSO expired, very large PR where GitHub refuses to | Hide bot comments | Conversation tab timeline items authored by bots. | | Snippet expander | Type `;nit` then space in any comment box. Defaults: `;nit`, `;q`, `;block`, `;lgtm`, `;test`, `;why`. Edit via **Edit config (JSON)**. | | Whitespace hidden by default | Off by default. Redirects to `?w=1` once per tab per PR, and never when the URL already has a `w` parameter. | +| Warn before leaving with an unsubmitted review | If your pending review has comments, closing the tab, reloading or following a link off the PR asks first. Same-site links that leave the PR (another PR, issues, a file view) get a `confirm()`; anything else that unloads the page gets the browser's own "Leave site?" prompt. Moving between the PR's own tabs, clicking Submit in the review dialog, and F5 / Ctrl+R don't prompt (the browser's reload button does). A pending review with no comments, or your submitted comments, never count. Browser back/forward inside GitHub's app is not caught, and nor is closing the tab within 10s of clicking a link inside the PR. | +| New commits since your last look | Your last look is the PR's head commit when you last marked a file Viewed. When the head has moved since, the panel shows "N new commits since your last look" with **View changes**, which opens GitHub's own range view (`/pull//files/..`). After a force-push the old commit isn't in the PR and that view 404s, so it links to `/compare/..` instead and says so. **Dismiss** marks the current head as seen. | Every feature has a toggle in the settings modal, opened with the ⚙ in the panel bar or Alt+, (grouped, with a Reset to defaults button). **Edit config (JSON)** and **Reset config** are in the Tampermonkey menu; only settings you change are stored. @@ -58,4 +61,8 @@ The filter row is injected into the React `ActionMenu` that the tree's filter bu Viewed state is read from each file's Viewed control (`VIEWED_CONTROL_SEL`): the classic checkbox, or the newer diff's `button[aria-pressed][aria-label*="viewed"]`. Viewed-state across pushes and Alt+N both depend on it. Alt+C reads line numbers from `[data-line-number]` cells inside the selection's `` rows (`rowLine`, `selectedRows`), which is verified against the classic diff markup; the newer diff view may fall back to a link without line numbers. +The pending-review warning reads the count from GitHub's own review button: in the React Files view, the `[data-component="ButtonCounter"]` inside `button[class*="ReviewMenuButton__"]` (Primer's `count` prop, set to the number of pending comments); in the classic view, `.js-pending-review-comment-count`. On a fresh load it also reads `viewerPendingReview` from the React payload (`react-app[initial-path]` → `script[data-target="react-app.embeddedData"]`), used only when that payload was rendered for the current PR. The last count seen sticks while you're on a tab without the button. Submitting from the React dialog sets `location.href` before the count re-renders, so beforeunload stays quiet while `#anchored-review-title` (the review overlay's heading) is on the page. See `livePendingCount`, `payloadPendingCount` and `reviewDialogOpen`. + +"New commits since your last look" reads `payload.pullRequestsCommitsRoute.commitGroups[].commits[].oid` from the commits route (with a fallback that parses the same payload out of the HTML page) and cross-checks the last commit against the SHA embedded in `metadata.commitHeadShaChannel`. If the route is truncated or the two disagree, the feature shows nothing. `.patch` isn't used because it leaves out merge commits, so its last commit isn't the head after a "Merge main into …" push. See `parseCommitOids`. + Not built: whitespace-only change detection, ticket hover cards, per-file private notes. diff --git a/gh-pr-review-toolkit.user.js b/gh-pr-review-toolkit.user.js index 5c7d1d8..484a1e6 100644 --- a/gh-pr-review-toolkit.user.js +++ b/gh-pr-review-toolkit.user.js @@ -42,6 +42,8 @@ copyPermalink: true, checksTab: false, reviewButton: true, + pendingReviewGuard: true, + sinceLastLook: true, }, snippets: { nit: "nit: ", @@ -94,6 +96,7 @@ { key: "whitespace", label: "Open files tab with whitespace hidden" }, { key: "viewedAcrossPushes", label: "Viewed-state across pushes" }, { key: "copyPermalink", label: "Copy selected lines as a quote with link", hint: "Alt+C" }, + { key: "sinceLastLook", label: "New commits since your last look", note: "Your last look is when you last marked a file Viewed" }, ], }, { @@ -102,6 +105,7 @@ { key: "hideNoise", label: "Hide bot comments" }, { key: "snippets", label: "Snippet expander", hint: ";trigger + space" }, { key: "tickets", label: "Ticket chips" }, + { key: "pendingReviewGuard", label: "Warn before leaving with an unsubmitted review", note: "Only when your pending review has comments" }, ], }, { @@ -163,6 +167,7 @@ if (now - seenAt < PR_RETENTION_MS) continue; GM_deleteValue(`v3:viewed:${id}`); GM_deleteValue(`v3:changedFilter:${id}`); + GM_deleteValue(`v3:lastLook:${id}`); delete lastSeen[id]; } store.set("prLastSeen", lastSeen); @@ -233,11 +238,12 @@ return { owner, repo, number: Number(number), key: `${owner}/${repo}#${number}` }; }; - const fetchText = (url) => + const fetchText = (url, headers = {}) => new Promise((resolve) => { GM_xmlhttpRequest({ method: "GET", url, + headers, onload: (res) => resolve(res.status < 300 ? res.responseText : null), onerror: () => resolve(null), ontimeout: () => resolve(null), @@ -269,6 +275,49 @@ return files; }; + // .patch skips merge commits, so the head comes from the commits route instead: JSON when asked for it, else the page's embedded copy. + const commitsRoute = (text) => { + try { + return JSON.parse(text).payload?.pullRequestsCommitsRoute; + } catch {} + for (const [, json] of text.matchAll(/