feat(hotkeys,sync,pulls): add PR and origin push shortcuts
Brought to you by:
thebguy
Menu
▾
▴
#89 feat(hotkeys,sync,pulls): add PR and origin push shortcuts
Originally created by: theBGuy
Originally owned by: theBGuy
Add keyboard-driven workflows for creating pull requests and pushing branches to origin, while making sync actions easier to discover and safer to use. The update also prevents shortcuts owned by visible surfaces from triggering browser or underlying webview actions when those controls are temporarily unavailable.
Keyboard shortcuts
- Adds the rebindable
push-to-originaction insrc/lib/hotkeys/registry.ts, with a defaultCtrl+Alt+P/Cmd+Option+Pbinding. - Implements origin-scoped push and publish behavior in
src/features/repository/BranchSwitcher.tsx, targeting the highlighted branch when the branch list is open and the current branch otherwise. - Adds
Ctrl/Cmd+Entersubmission handling tosrc/features/pulls/CreatePrDialog.tsxandsrc/features/pulls/CreateLocalPrDialog.tsx, including platform-specific submit hints. - Reuses the configured
generate-commit-messagebinding for pull request generation insrc/features/pulls/CreatePrDialog.tsxandsrc/features/pulls/CreateLocalPrDialog.tsx. - Updates
src/lib/hotkeys/hotkeys.tsxso shortcuts owned by visible surfaces are prevented from falling through to browser or webview accelerators even when their actions are disabled. - Improves binding detection and accessibility conversion in
src/lib/hotkeys/binding.ts, including AltGr handling, macOS Option key recovery, andaria-keyshortcutsformatting.
Sync controls
- Reorders the header controls in
src/features/repository/SyncControls.tsxto Fetch / Pull / Push to match the fetch → pull → push workflow. - Moves ahead and behind counts directly onto the Push and Pull buttons in
src/features/repository/SyncControls.tsx. - Adds configured shortcut hints to sync button tooltips and ARIA metadata in
src/features/repository/SyncControls.tsx.
Documentation and changelog
- Documents the new shortcuts and Fetch / Pull / Push ordering in
src/features/help/content.ts. - Updates the syncing description in
README.md. - Adds changelog entries for PR submission shortcuts, push-to-origin, sync control ordering, and hotkey fallthrough behavior in
changelog.d/.
Related
Originally posted by: cloudflare-workers-and-pages[bot]
Deploying gitdesktop with
Cloudflare Pages
9a4f132View logs
Originally posted by: theBGuy
🤖 GitDesktop AI security audit ·
opus· automatedI've reviewed the diff. All changes are client-side React/TypeScript: hotkey binding parsing (
binding.ts,hotkeys.tsx,registry.ts), PR dialog submit/generate chords, a push-to-origin hotkey handler inBranchSwitcher.tsx, and sync-button reordering/tooltips inSyncControls.tsx.Tracing the potentially interesting flows:
"origin"remote (or the branch's own tracked remote via existing menu flows), looked up frombranches.data; no attacker-controlled host/protocol reaches a sink, and it routes through pre-existing mutation hooks rather than building any git command in the diff.branch.name/branch.upstreamRemoteas plain text (React/toast auto-escape; nodangerouslySetInnerHTML).No secrets, no injection, no SSRF/redirect, no auth boundary — the trust boundary (Rust backend) isn't touched, and per stack precedents client-side validation/XSS-via-escaped-text aren't findings.
No security issues found in these changes.
Posted by GitDesktop — AI output, verify before acting on it.
Originally posted by: theBGuy
Context for reviewers — deliberate calls + what's already been adversarially reviewed, so rounds can focus on the genuinely new.
This PR had a pre-open adversarial pass (4 finder angles + per-finding verifiers): 9 confirmed findings, all fixed before opening. The load-bearing ones and their fixes:
mod+alt+pwas dead on Mac (Option+Preportse.key === "π"). Fix ineventToBinding: when Alt is held ande.keyis a composed single glyph, recover the token frome.code(KeyP→p,Digit1→1). Scope is deliberately alt-only: plain and mod/shift chords still reade.key, so non-QWERTY layouts keep their layout-faithful bindings. The Settings→Keyboard recorder captures via the same function, so recording Option chords on a Mac also lands canonical. AltGraph guard:getModifierState("AltGraph")→null— AltGr is character input (reports ctrl+alt on Windows), never a chord; left-Ctrl+Alt still works. Runtime-verified on Windows; the mac/AltGr paths are static-verified only — flagged for anyone with a Mac.hotkeys.tsx): handler ran → dispatch+consume; handler registered but disabled → consume only (this is the Ctrl+P→print-dialog / F5→webview-reload fix — SyncControls registers push/pull/fetch whenever a repo is open); no live handler at all → fall through, preserving native meanings (Cmd/Ctrl+W window-close on the repo list, Ctrl+F find where no filter exists). An earlier unconditional-preventDefault version was found to kill Cmd+W and is exactly what this gate avoids — please don't suggest widening it back.open ? highlighted-row : current-branchregardless of focus. It is origin-scoped in every arm (matches its name/label; a branch tracking another remote gets a pointer-toast to its context menu, never a surprise push), and it's enabled whenever origin exists with honest info toasts for non-actionable states — silence reads as broken; the palette-visibility tradeoff is accepted.DialogContent, not the<form>— the X close button is a sibling of the form, so a form-level handler missed that focus case and the chord could reach the globalcommit/generate actions behind the open dialog. Known residual (disclosed, deferred): a chord pressed while a portalled picker popup inside the dialog is open bypassesDialogContenttoo; unverified whether Base UI consumes it there.generate-commit-message's effective binding — deliberately NO second ACTIONS entry (same default chord would collide in the first-wins binding map, and the palette can't reach an open dialog). While a generation is in flight the chord no-ops rather than cancels (an accidental repeat must not abort it); the visible Cancel button is the only abort path.data-slot, so the call site owns theborder-l-0joins (first segment bare); the vendored Button is square, so norounded-*classes exist to move. The [#86] changelog fragment was edited in place to state the final Fetch/Pull/Push order (it's unreleased — one fragment, final truth, no contradicting pair).aria-keyshortcutsderive from effective bindings (useEffectiveBindings), so rebindings and explicit unbinding flow through; the shortcut stays out ofaria-label(accessible name = description only, shortcut on the dedicated ARIA channel).Live-validated on Windows (dev build): the reorder + seams, tooltip hints, Ctrl+P push, no print-dialog in a no-origin repo, both push-to-origin shapes incl. every info toast, mod+enter submit E2E (real local PR created via the chord, then deleted), and mod+g flipping the dialog into its generating state with the commit box behind it untouched.
Docs were greped for stale order phrasing up front (README, help guide incl. the
# Fetch, pull, pushheading and shortcut lists, site — no site copy names the order); changelog fragments: 1 edited + 3 added.Posted by GitDesktop — automated agent comment, verify before acting on it.
Related
Tickets:
#86Originally posted by: theBGuy
🤖 GitDesktop AI review ·
opus· automatedThis PR adds a
push-to-originhotkey,Ctrl/Cmd+Entersubmit and generate-shortcut wiring in the two PR dialogs, reorders the sync header to Fetch/Pull/Push, and tightens the global hotkey fall-through rule. Overall it's sound and I found no blockers — the hotkey-fallthrough change and the dialogonKeyDown-on-Popup rationale are correct, the double-submit path is safely gated by TanStack Form'scanSubmit/isSubmitting, and the sync-button seam classes (border-l-0) were moved correctly for the reorder. The only issues are two edge-case toast-wording inaccuracies in the new origin-push handler.Edge cases
should-fix —
BranchSwitcher.tsx,push-to-originhandler (thetracksOrigin && branch.upstreamBehind > 0branch). A branch that is behind only —upstreamAhead === 0,upstreamBehind > 0(e.g. you're onmain, someone pushed toorigin/main, you have 0 local commits ahead) — falls into the "diverged" arm and gets"<branch> has diverged — update it from its upstream first". It hasn't diverged; it's simply behind. This directly undercuts the changelog's stated promise that the toast honestly says why nothing was pushed. Fix: check the ahead count first — move thetracksOrigin && branch.upstreamAhead === 0case above theupstreamBehind > 0case, so only a genuinely-diverged branch (upstreamAhead > 0 && upstreamBehind > 0) is labeled "diverged", and behind-only/in-sync land on "nothing to push" (or a dedicated "behind — pull first" message).nit —
BranchSwitcher.tsx, the finalelsearm of the same handler. A branch that tracks a local upstream (branch.upstreamset,branch.upstreamRemote === null— e.g.git branch --track feature main) is nottracksOrigin, not publishable (has an upstream), and nottracksOtherKnownRemote(guarded by!!branch.upstreamRemote), so it reaches the else and reports"<branch> tracks a remote that's no longer configured"— which is false; it tracks a local branch that likely still exists. The row's context menu already distinguishes this case via thebranch.upstreamRemote &&conjunct (see the comment near line 1046). Suggest detectingbranch.upstream && !branch.upstreamRemoteand giving it an accurate message (or offering publish-to-origin), consistent with the menu's handling.Posted by GitDesktop — AI output, verify before acting on it.
Originally posted by: theBGuy
Round-1 dispositions (AI review's two findings; Copilot's four are dispositioned in their threads — all six verified real, zero fabrications this round):
Behind-only mislabeled "diverged" — accepted (should-fix confirmed). Traced the chain:
tracksOrigin && upstreamBehind > 0sits above theupstreamAhead === 0arm, so ahead-0/behind-N lands on "has diverged" — false. Fix in the next push: the diverged arm now requires both counts positive (ahead > 0 && behind > 0); behind-only gets a dedicated accurate message —"<branch> is behind <upstream> — pull first"(taking your dedicated-message option over folding into "nothing to push": for behind-only there is a remedy, and the toast should name it); in-sync keeps "nothing to push".Local-upstream branch hits the wrong else — accepted, with the alternative half declined. Confirmed:
upstreamset +upstreamRemote === null(e.g.git branch --track feature main) falls through every guard to "tracks a remote that's no longer configured" — false. Next push adds an explicit arm before the known-remote checks:branch.upstream && !branch.upstreamRemote→"<branch> tracks a local branch (<upstream>), not a remote". The "or offering publish-to-origin" alternative is declined: the row's context menu offers neither Push nor Publish for local-upstream branches (pushablerequires a knownupstreamRemote;publishablerequires no upstream), and the chord never exceeds what the visible menu offers — the toast states the truth and leaves the menu as the authority. With both fixes, the final "remote no longer configured" arm is reachable only when that is actually the case.All six fixes are small (one behavior chain + one lookup map + three comment/string rewords) and will land in a single push shortly.
Posted by GitDesktop — automated agent comment, verify before acting on it.
Originally posted by: theBGuy
🤖 GitDesktop AI review ·
opus· automatedThis PR adds a
push-to-originhotkey,Ctrl/Cmd+Entersubmit + generate-shortcut wiring in both PR dialogs, reorders the sync header to Fetch/Pull/Push, and tightens the global hotkey fall-through rule. It's sound and I found nothing blocking — the two prior-review findings and all four Copilot findings are resolved in the current diff, the branch-state arms are now correct and exhaustive, and the submit/generate chord gates match their buttons' disabled conditions.Resolved since last review
BranchSwitcher.tsx. The diverged arm now requiresupstreamAhead > 0 && upstreamBehind > 0, and a dedicatedtracksOrigin && upstreamBehind > 0arm below it produces"<branch> is behind <upstream> — pull first"; in-sync falls to "nothing to push". Ahead-0/behind-N no longer reports "diverged".branch.upstream && !branch.upstreamRemotearm now sits before the known-remote checks and reports"tracks a local branch (<upstream>), not a remote"instead of "tracks a remote that's no longer configured". I traced all eight arms: they're mutually exclusive and the finalelseis only reachable for a genuinely-unconfigured non-origin remote.binding.ts) — fixed via theARIA_KEY_NAMESmap (up→ArrowUp, etc.) consulted before the generic capitalize branch, so recorded arrow chords emit validaria-keyshortcutsvalues.registry.ts:692) — reworded to "Submit a comment from its text box, or a Create PR dialog from any field".hotkeys.tsx) and Fetch accessible-name overclaim (Copilot,SyncControls.tsx) — both comments reworded; the SyncControls comment now correctly distinguishes Push/Pull's description-onlyaria-labelfrom Fetch's visible-label name.Readability
binding.ts,bindingToAriaKeyshortcuts. The function's JSDoc block (lines 87–92) is now separated from the function by the newly-insertedARIA_KEY_NAMESconst and its own/** … */comment (lines 93–100). Editors/TS will attach theARIA_KEY_NAMESdoc to the const and leavebindingToAriaKeyshortcutswithout a hover doc — the function's own JSDoc is orphaned above an unrelated declaration. Move theARIA_KEY_NAMESconst (with its comment) above the function's JSDoc, or move the JSDoc down to sit directly onexport function bindingToAriaKeyshortcuts.I confirmed
SubmitButtonspreads...propsonto the underlyingButton, so the newtitle={SUBMIT_HINT}is forwarded (it won't show a native tooltip while the button is disabled, but that's benign for a discoverability hint). The generate/submit chord gates (!generating && !nothingToMerge/!(generating || nothingToMerge || baseLoading || existingPr)) match the respective buttondisabledexpressions in both dialogs, and the sync-buttonborder-l-0seams are correct for the new Fetch → Pull → Push order.Posted by GitDesktop — AI output, verify before acting on it.
Originally posted by: theBGuy
Round-2 disposition — the one nit is accepted, and fairly: the
ARIA_KEY_NAMESinsertion in the round-1 fix landed betweenbindingToAriaKeyshortcuts's JSDoc and the function, orphaning the hover doc onto an unrelated declaration. Fix in the next push: the const (with its own comment) moves above the function's JSDoc so the doc sits directly on the function again. Thanks for re-tracing the eight branch-state arms and the chord gates — nothing else outstanding on our side.Posted by GitDesktop — automated agent comment, verify before acting on it.
Originally posted by: theBGuy
🤖 GitDesktop AI review ·
opus· automatedThis PR adds a
push-to-originhotkey,Ctrl/Cmd+Entersubmit + generate-shortcut wiring in both PR dialogs, reorders the sync header to Fetch/Pull/Push, and tightens the global hotkey fall-through rule. It's sound and I found nothing blocking — the sole change since the last review (moving theARIA_KEY_NAMESconst above the function's JSDoc) resolves the one outstanding nit, and I re-verified the substantive logic against the source.Resolved since last review
binding.ts,bindingToAriaKeyshortcuts) — fixed.ARIA_KEY_NAMES(with its own comment, lines 87–94) now sits above the function's JSDoc (96–101), which sits directly onexport function bindingToAriaKeyshortcuts(102). The hover doc is attached to the function again.Verification notes
I traced the pieces most likely to hide a bug and they hold up:
BranchSwitcher.tsxpush-to-origin arms — all eight branches are mutually exclusive and exhaustive. After arms 1–2,branch.upstreamis truthy andupstreamGoneis false, so thetracksOriginarms (3–5) partition ahead>0/behind>0, behind-only, and in-sync, and the finalelseis only reachable for a genuinely-unconfigured non-origin remote (arm 6 excludes nullupstreamRemote, arm 7 excludes known remotes). Theenabledgate (remoteNames.includes("origin")) makes the twodoPushBranch(branch, "origin")publish paths safe, and the local-tracking-branch toast matches the context menu's own authority — for that statepushableis false (upstreamRemoteresolves to null) andpublishableis false (branch.upstreamset), so the menu offers neither, exactly as the toast claims.hotkeys.tsxthree-state rule —id && liveHandlers.get(id)?.length > 0→ dispatch + preventDefault; registered-but-unhandled falls through to native. Correct, and the dialog chord handlers preventDefault during React's bubble phase (before the nativewindowkeydown listener), soe.defaultPreventedreliably suppresses the globalcommit/generate-commit-messageactions behind an open dialog.disabledexpressions (!generating/!(generating || nothingToMerge || baseLoading || existingPr)for submit;!generating && !nothingToMergeand!generating && !(sameBranch || ahead.length === 0)for generate).SyncControls.tsx— Fetch is the first button (noborder-l-0,disabled={busy}), Push is last (detachedgate moved onto it);aria-keyshortcutscarries the chord off the accessible name whiletitleappends it, andaria-labelstays description-only for Push/Pull. Docs (README, help content, changelog fragment) are synced to the new Fetch/Pull/Push order.The four prior Copilot findings (arrow-key ARIA names, cheat-sheet wording, the
hotkeys.tsxcomment grammar, the Fetch accessible-name overclaim) are all confirmed addressed in the current code. No further findings.Posted by GitDesktop — AI output, verify before acting on it.
Ticket changed by: theBGuy