Vi mode: half-page scroll, visible cursor, gg fix (#851)

* Vi mode P0 improvements: half-page scroll, visible cursor, gg fix

Three changes to keyboard copy mode (vi mode):

1. Ctrl+U/D now scroll half-page (was full-page). Ctrl+B/F remain
   full-page. Uses Ghostty's scroll_page_fractional binding.

2. Entering copy mode now creates a 1-cell selection at the terminal
   cursor via select_cursor_cell, giving the user a visible cursor
   indicator. Visual mode (v) is tracked separately from Ghostty's
   has_selection so the cursor selection doesn't make every motion
   behave as visual. Exiting visual mode (v again) collapses back
   to the cursor cell.

3. Single 'g' is now a prefix key requiring 'gg' to scroll to top,
   matching standard vim behavior. Uses the same pendingG state
   machine pattern as pendingYankLine for 'yy'.

Part of https://github.com/manaflow-ai/cmux/issues/846

* Fix viewport row refresh with persistent cursor selection

The 1-cell cursor selection made refreshKeyboardCopyModeViewportRowFromVisibleAnchor
always bail (has_selection was always true). Guard on keyboardCopyModeVisualActive
instead so viewport row updates work after scrolling in non-visual mode. Also
re-creates the cursor cell after refresh to preserve visibility.

Additionally: use performBindingAction(_:repeatCount:) for scrollHalfPage,
add state-reset assertion to testGGWithSelectionAdjustsToHome.

Addresses review feedback from CodeRabbit, Cubic, Codex, and Greptile.
This commit is contained in:
Lawrence Chen 2026-03-03 23:56:01 -08:00 committed by GitHub
parent c7bdd92df9
commit 044a3dbb64
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 127 additions and 11 deletions

View file

@ -292,6 +292,7 @@ enum TerminalKeyboardCopyModeAction: Equatable {
case copyLineAndExit
case scrollLines(Int)
case scrollPage(Int)
case scrollHalfPage(Int)
case scrollToTop
case scrollToBottom
case jumpToPrompt(Int)
@ -304,10 +305,12 @@ enum TerminalKeyboardCopyModeAction: Equatable {
struct TerminalKeyboardCopyModeInputState: Equatable {
var countPrefix: Int?
var pendingYankLine = false
var pendingG = false
mutating func reset() {
countPrefix = nil
pendingYankLine = false
pendingG = false
}
}
@ -395,10 +398,10 @@ func terminalKeyboardCopyModeAction(
if normalized == [.control] {
if chars == "u" || chars == "\u{15}" {
return hasSelection ? .adjustSelection(.pageUp) : .scrollPage(-1)
return hasSelection ? .adjustSelection(.pageUp) : .scrollHalfPage(-1)
}
if chars == "d" || chars == "\u{04}" {
return hasSelection ? .adjustSelection(.pageDown) : .scrollPage(1)
return hasSelection ? .adjustSelection(.pageDown) : .scrollHalfPage(1)
}
if chars == "b" || chars == "\u{02}" {
return hasSelection ? .adjustSelection(.pageUp) : .scrollPage(-1)
@ -439,7 +442,8 @@ func terminalKeyboardCopyModeAction(
if normalized == [.shift] {
return hasSelection ? .adjustSelection(.end) : .scrollToBottom
}
return hasSelection ? .adjustSelection(.home) : .scrollToTop
// Bare "g" is a prefix key (e.g. gg); handled in resolve.
return nil
case "0", "^":
return hasSelection ? .adjustSelection(.beginningOfLine) : nil
case "$", "4":
@ -486,6 +490,17 @@ func terminalKeyboardCopyModeResolve(
state.pendingYankLine = false
}
if state.pendingG {
if chars == "g", normalized.isEmpty {
let count = terminalKeyboardCopyModeClampCount(state.countPrefix ?? 1)
let action: TerminalKeyboardCopyModeAction = hasSelection ? .adjustSelection(.home) : .scrollToTop
state.reset()
return .perform(action, count: count)
}
// Not `gg`, cancel and treat as fresh command.
state.pendingG = false
}
if normalized.isEmpty,
let scalar = chars.unicodeScalars.first,
scalar.isASCII,
@ -509,6 +524,11 @@ func terminalKeyboardCopyModeResolve(
return .consume
}
if chars == "g", normalized.isEmpty {
state.pendingG = true
return .consume
}
guard let action = terminalKeyboardCopyModeAction(
keyCode: keyCode,
charactersIgnoringModifiers: charactersIgnoringModifiers,
@ -2785,6 +2805,11 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations {
private var keyboardCopyModeConsumedKeyUps: Set<UInt16> = []
private var keyboardCopyModeInputState = TerminalKeyboardCopyModeInputState()
private var keyboardCopyModeViewportRow: Int?
/// Tracks whether the user has explicitly entered visual selection mode (v).
/// Separate from Ghostty's `has_selection` because copy mode always maintains
/// a 1-cell selection as a visible cursor. This flag determines whether
/// movements should extend the selection (visual) or scroll the viewport.
private var keyboardCopyModeVisualActive = false
fileprivate var isKeyboardCopyModeActive: Bool { keyboardCopyModeActive }
#if DEBUG
private static let keyLatencyProbeEnabled: Bool = {
@ -3229,6 +3254,7 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations {
private func setKeyboardCopyModeActive(_ active: Bool) {
keyboardCopyModeInputState.reset()
keyboardCopyModeVisualActive = false
keyboardCopyModeActive = active
if active, let surface {
keyboardCopyModeViewportRow = keyboardCopyModeSelectionAnchor(surface: surface)?.row
@ -3236,6 +3262,9 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations {
if keyboardCopyModeViewportRow == nil {
keyboardCopyModeViewportRow = keyboardCopyModeImeViewportRow(surface: surface)
}
// Create a 1-cell selection at the terminal cursor to serve as a
// visible cursor indicator in copy mode.
_ = ghostty_surface_select_cursor_cell(surface)
} else {
keyboardCopyModeViewportRow = nil
}
@ -3286,10 +3315,14 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations {
}
private func refreshKeyboardCopyModeViewportRowFromVisibleAnchor(surface: ghostty_surface_t) {
guard !ghostty_surface_has_selection(surface) else { return }
// In visual mode the user owns the selection range; don't disturb it.
// Outside visual mode we keep a 1-cell cursor selection for visibility,
// so we still need to refresh the viewport row after scrolling.
guard !keyboardCopyModeVisualActive else { return }
guard let anchor = keyboardCopyModeSelectionAnchor(surface: surface) else { return }
keyboardCopyModeViewportRow = anchor.row
_ = ghostty_surface_clear_selection(surface)
// Preserve the visible cursor indicator.
_ = ghostty_surface_select_cursor_cell(surface)
}
private func copyCurrentViewportLinesToClipboard(
@ -3344,7 +3377,9 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations {
return false
}
let hasSelection = ghostty_surface_has_selection(surface)
// Use the visual-mode flag instead of raw has_selection so that the
// 1-cell cursor selection doesn't make every motion behave as visual.
let hasSelection = keyboardCopyModeVisualActive
let resolution = terminalKeyboardCopyModeResolve(
keyCode: event.keyCode,
charactersIgnoringModifiers: event.charactersIgnoringModifiers,
@ -3361,9 +3396,12 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations {
_ = ghostty_surface_clear_selection(surface)
setKeyboardCopyModeActive(false)
case .startSelection:
_ = ghostty_surface_select_cursor_cell(surface)
keyboardCopyModeVisualActive = true
case .clearSelection:
keyboardCopyModeVisualActive = false
_ = ghostty_surface_clear_selection(surface)
// Re-create 1-cell cursor at terminal cursor position.
_ = ghostty_surface_select_cursor_cell(surface)
case .copyAndExit:
_ = performBindingAction("copy_to_clipboard")
_ = ghostty_surface_clear_selection(surface)
@ -3383,6 +3421,10 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations {
case let .scrollPage(delta):
performBindingAction(delta > 0 ? "scroll_page_down" : "scroll_page_up", repeatCount: count)
refreshKeyboardCopyModeViewportRowFromVisibleAnchor(surface: surface)
case let .scrollHalfPage(delta):
let fraction = delta > 0 ? 0.5 : -0.5
performBindingAction("scroll_page_fractional:\(fraction)", repeatCount: count)
refreshKeyboardCopyModeViewportRowFromVisibleAnchor(surface: surface)
case .scrollToTop:
keyboardCopyModeViewportRow = 0
_ = performBindingAction("scroll_to_top")