Add Cmd+Up/Down keyboard shortcuts for terminal scrolling by stevenvo · Pull Request #2679 · wavetermdev/waveterm

stevenvo · 2025-12-15T04:05:06Z

Summary

Adds keyboard shortcuts for quick navigation in terminal output.

Shortcuts

Cmd+Up: Scroll to top of terminal
Cmd+Down: Scroll to bottom of terminal

Use Cases

Quickly jump to start of long command output
Return to bottom after scrolling up to read
Navigate large terminal buffers efficiently
Useful when terminal has scrolled unexpectedly

Implementation

Added to terminal keyDownHandler
Works in any terminal block
Instant navigation (no animation)
Doesn't interfere with other shortcuts

Test Plan

Cmd+Up scrolls to top
Cmd+Down scrolls to bottom
Works with long terminal output
Doesn't break existing shortcuts

🤖 Generated with Claude Code

- Cmd+Down: Scroll to bottom of terminal - Cmd+Up: Scroll to top of terminal - Quick navigation for long terminal output 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>

coderabbitai · 2025-12-15T04:05:17Z

Walkthrough

Adds multiple keyboard shortcut handlers to TermViewModel.keyDownHandler in frontend/app/view/term/term-model.ts. New branches handle Shift+End, Shift+Home, Cmd+End (macOS), Cmd+Home (macOS), Shift+PageDown, and Shift+PageUp to scroll the terminal view to top, bottom, or by pages; each handled branch performs scrolling and returns true. Minor formatting-only adjustments were made around shouldHandleCtrlVPaste with no behavioral change and no public API signature changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Single-file change with a consistent pattern of adding keyboard-case branches.
Low logic density and small, localized edits.
No API changes.

Areas to double-check:

Correct target element/container for scrolling operations.
That returning true in each branch has the intended effect on event propagation and integration with existing key handling.

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title mentions Cmd+Up/Down shortcuts but the actual implementation includes Shift+End, Shift+Home, Cmd+End, Cmd+Home, Shift+PageDown, and Shift+PageUp shortcuts - not Cmd+Up/Down.	Update the title to accurately reflect all implemented shortcuts, such as 'Add keyboard shortcuts for terminal scrolling (Shift+End/Home, Cmd+End/Home, Shift+PageUp/Down)' or clarify which shortcuts are the primary focus.
Description check	⚠️ Warning	The description only mentions Cmd+Up and Cmd+Down shortcuts, but the actual changeset implements Shift+End, Shift+Home, Cmd+End, Cmd+Home, Shift+PageDown, and Shift+PageUp - creating a significant mismatch.	Update the description to accurately document all implemented keyboard shortcuts and their functions, or remove shortcuts that were not actually implemented.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1ed6a9b and 11b73a6.

📒 Files selected for processing (1)

frontend/app/view/term/term-model.ts (3 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

frontend/app/view/term/term-model.ts (1)

frontend/app/store/global.ts (3)

getSettingsKeyAtom (861-861)

globalStore (865-865)

getApi (852-852)

🔇 Additional comments (2)

frontend/app/view/term/term-model.ts (2)

541-553: LGTM on formatting adjustments.

The whitespace changes in shouldHandleCtrlVPaste() are minor formatting improvements with no behavioral impact.

492-527: PR description does not match implemented shortcuts.

The PR title and objectives reference "Cmd+Up/Down" shortcuts, but the implementation uses different key combinations: Shift+End/Home, Cmd+End/Home (macOS), and Shift+PageDown/PageUp. Please update the PR description to accurately reflect the implemented shortcuts.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

…ll-shortcuts

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f622c65 and 1ed6a9b.

📒 Files selected for processing (1)

frontend/app/view/term/term-model.ts (1 hunks)

coderabbitai · 2025-12-15T19:11:51Z

frontend/app/view/term/term-model.ts

+        if (keyutil.checkKeyPressed(waveEvent, "Cmd:ArrowDown")) {
+            // Scroll to bottom
+            if (this.termRef?.current?.terminal) {
+                this.termRef.current.terminal.scrollToBottom();
+            }
+            return true;
+        }
+        if (keyutil.checkKeyPressed(waveEvent, "Cmd:ArrowUp")) {
+            // Scroll to top
+            if (this.termRef?.current?.terminal) {
+                this.termRef.current.terminal.scrollToLine(0);
+            }
+            return true;
+        }


⚠️ Potential issue | 🟡 Minor

Only consume Cmd+ArrowUp/Down when a terminal instance actually exists

Right now these branches return true even if this.termRef?.current?.terminal is falsy. In vdom mode or before the terminal mounts, that means Cmd+ArrowUp/Down are swallowed and never reach the vdom key handler or app-level handlers, which goes against the goal of “not interfering with other shortcuts.”

Recommend only returning true when a terminal is present and we actually scroll; otherwise let the event fall through:

- if (keyutil.checkKeyPressed(waveEvent, "Cmd:ArrowDown")) { - // Scroll to bottom - if (this.termRef?.current?.terminal) { - this.termRef.current.terminal.scrollToBottom(); - } - return true; - } - if (keyutil.checkKeyPressed(waveEvent, "Cmd:ArrowUp")) { - // Scroll to top - if (this.termRef?.current?.terminal) { - this.termRef.current.terminal.scrollToLine(0); - } - return true; - } + if (keyutil.checkKeyPressed(waveEvent, "Cmd:ArrowDown")) { + const terminal = this.termRef?.current?.terminal; + if (terminal) { + // Scroll to bottom + terminal.scrollToBottom(); + return true; + } + return false; + } + if (keyutil.checkKeyPressed(waveEvent, "Cmd:ArrowUp")) { + const terminal = this.termRef?.current?.terminal; + if (terminal) { + // Scroll to top + terminal.scrollToLine(0); + return true; + } + return false; + }

This keeps the new shortcuts effective on terminal blocks while avoiding unintended interception when there is no terminal to scroll.

🤖 Prompt for AI Agents

In frontend/app/view/term/term-model.ts around lines 492 to 505, the Cmd+ArrowUp/Down handlers currently return true even when this.termRef?.current?.terminal is falsy, causing the key event to be swallowed when no terminal instance exists; change the logic so you only return true after confirming a terminal exists and you actually call scrollToBottom()/scrollToLine(0) — i.e., check for this.termRef?.current?.terminal, perform the scroll, then return true; otherwise do not return true so the event falls through to vdom or app-level handlers.

sawka · 2025-12-15T19:32:42Z

@stevenvo I did some research here, and it isn't safe to bind Cmd+Up/Down (which ends up mapping to Alt+Up/Down) in the terminal. This can conflict with terminal apps like vim or less.

So, I looked into what native MacOS and Linux/Windows terminals use. And it turns out to be:

Shift:PageUp/Down, and Shift:Home/End which controls scrollback. On MacOS you can get these by using Fn:Shift:Arrows (Fn:Shift:ArrowLeft is Shift:Home, Fn:Shift:ArrowUp is Shift:PageUp, etc).

These now mirror iterm2 and terminals like gnome terminal etc and so are safe to add.

stevenvo mentioned this pull request Dec 15, 2025

UX Improvements: Folder-based tab titles, scroll enhancements, and file preview features #2674

Closed

9 tasks

stevenvo force-pushed the feature/keyboard-scroll-shortcuts branch from 2247aa6 to ff44fb3 Compare December 15, 2025 04:22

Merge remote-tracking branch 'origin/main' into feature/keyboard-scro…

1ed6a9b

…ll-shortcuts

coderabbitai bot reviewed Dec 15, 2025

View reviewed changes

terminal scrolling keys in line with native macos/linux terminals

11b73a6

sawka merged commit 8b37bc6 into wavetermdev:main Dec 15, 2025
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Cmd+Up/Down keyboard shortcuts for terminal scrolling#2679

Add Cmd+Up/Down keyboard shortcuts for terminal scrolling#2679
sawka merged 3 commits intowavetermdev:mainfrom
stevenvo:feature/keyboard-scroll-shortcuts

stevenvo commented Dec 15, 2025

Uh oh!

coderabbitai bot commented Dec 15, 2025 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

coderabbitai bot Dec 15, 2025

Uh oh!

sawka commented Dec 15, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

stevenvo commented Dec 15, 2025

Summary

Shortcuts

Use Cases

Implementation

Test Plan

Uh oh!

coderabbitai bot commented Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Estimated code review effort

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

sawka commented Dec 15, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai bot commented Dec 15, 2025 •

edited

Loading