feat(ui): add gg / G navigation in hunk diff#289
Conversation
Greptile SummaryThis PR adds Vim-style
Confidence Score: 4/5Safe to merge; the shortcut logic is sound and the state machine correctly resets across all handled modes and overlay states. The implementation is correct end-to-end for the full-app path and the modifier-guard inconsistency in isUppercaseGKey is unlikely to cause a real issue on standard terminals. The pager-mode path is untested but mirrors the app-mode path exactly, so a silent regression there is low-probability rather than guaranteed. src/ui/hooks/useAppKeyboardShortcuts.ts — the isUppercaseGKey modifier guard inconsistency is worth a second look before shipping. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
K[KeyEvent received] --> MT{handleMenuToggleShortcut?}
MT -->|yes| RESET_MT[reset pendingTopJumpRef\nreturn]
MT -->|no| PM{pagerMode?}
PM -->|yes| PH[handlePagerShortcut]
PM -->|no| HS{handleHelpShortcut?}
HS -->|yes| RESET_H[reset pendingTopJumpRef\nreturn]
HS -->|no| MS{handleMenuShortcut?}
MS -->|yes| RESET_M[reset pendingTopJumpRef\nreturn]
MS -->|no| FS{handleFilterShortcut?}
FS -->|yes| RESET_F[reset pendingTopJumpRef\nreturn]
FS -->|no| AH[handleAppShortcut]
PH --> RJ1[resolveJumpShortcut]
AH --> RJ2[resolveJumpShortcut]
RJ1 --> GU1{isUppercaseGKey?}
RJ2 --> GU1
GU1 -->|yes| BOT[return bottom\nscrollDiff +1 content]
GU1 -->|no| GL1{isLowercaseGKey?}
GL1 -->|no| CLR[clear pending\nreturn null → other shortcut]
GL1 -->|yes| PEND{pending already?}
PEND -->|yes| TOP[return top\nscrollDiff -1 content]
PEND -->|no| SET[set pending = true\nreturn pending → no-op]
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
src/ui/hooks/useAppKeyboardShortcuts.ts:34-38
**Modifier guards missing on `sequence === "G"` branch**
`isLowercaseGKey` explicitly checks `!key.option && !key.ctrl && !key.meta`, and the second branch of `isUppercaseGKey` does the same. The first branch (`key.sequence === "G"`) skips all four modifier guards entirely — so a combo that happens to produce the raw sequence `"G"` (e.g. on an unusual terminal mapping) would still fire jump-to-bottom. The inconsistency also makes the intent harder to follow at a glance.
```suggestion
function isUppercaseGKey(key: KeyEvent) {
return (
(key.sequence === "G" && !key.option && !key.ctrl && !key.meta) ||
(key.name === "g" && key.shift && !key.option && !key.ctrl && !key.meta)
);
}
```
### Issue 2 of 2
src/ui/AppHost.interactions.test.tsx:1693-1763
**Pager-mode path not exercised**
The PR description explicitly states "support the aliases in both full app mode and pager mode", but the new test only exercises `handleAppShortcut` (full app mode). `handlePagerShortcut` has its own independent call to `resolveJumpShortcut`, and there is no test that launches with `pagerMode: true` and verifies `G` → bottom / `gg` → top. A regression in the pager branch would not be caught by the current test suite.
Reviews (1): Last reviewed commit: "feat(ui): add gg and G navigation shortc..." | Re-trigger Greptile |
Summary
Gjumps to bottom (alias for End)ggjumps to top (alias for Home)gstate when entering menus/help/filter soggstays predictableG-> bottom andgg-> top behaviorTesting
bun run format:checkbun run typecheckbun run lintbun test src/ui/AppHost.interactions.test.tsx src/ui/components/ui-components.test.tsxCloses #286