Skip to content

Conversation

@stevenvo
Copy link
Contributor

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 <[email protected]>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 15, 2025

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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Comment on lines 492 to 505
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;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ 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
Copy link
Member

sawka commented Dec 15, 2025

@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.

@sawka sawka merged commit 8b37bc6 into wavetermdev:main Dec 15, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants