fix: fixed scroll disabled when log <20 lines

[dimmyjing]

This PR mainly contains two fixes: one is that there exists preexisting checks against number of log lines that prevents scrolling, which is obsolete after the log_scroll rework. The second is that log_scroll can become 0 even though it's clamped to 1, so the first scroll down from 0 to 1 does not actually scroll.

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses critical issues with log scrolling behavior within the Text User Interface (TUI). It removes outdated conditions that previously hindered scrolling for logs containing fewer than 20 lines and rectifies the initialization and boundary clamping of the log scroll position. These changes collectively ensure a consistent and fully functional scrolling experience, allowing users to navigate logs effectively regardless of their length and preventing the scroll position from becoming invalid.

Highlights

• Scrolling Restrictions Removed: Eliminated obsolete checks against log line count that previously prevented scrolling for logs with fewer than 20 lines, ensuring full scrollability regardless of log length.
• Log Scroll Position Correction: Corrected the initialization and boundary clamping of the log_scroll position to a minimum of 1, preventing off-by-one errors and ensuring the first log line is always accessible and the scroll position remains valid.

Changelog

• src/tui/app.rs
  • Initialized log_scroll to 1 instead of 0 upon App creation.
  • Removed conditional checks based on log content length from scroll_logs_down and scroll_logs_page_down functions.
  • Ensured log_scroll cannot go below 1 in scroll_logs_up, scroll_logs_page_up, and log search navigation by applying .max(1).
  • Reset log_scroll to 1 when exiting the log view.
• src/tui/event.rs
  • Updated 'g' key binding to set log_scroll to 1 (top) instead of 0.
  • Removed the log_content.len() > 20 check for 'G' key binding (scroll to bottom).

Activity

• No activity has been recorded for this pull request yet.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[greptile-apps]

Greptile Summary

This PR fixes two related scroll bugs in the TUI log viewer: it removes obsolete > 20 / > visible_lines line-count guards that blocked scrolling on short logs, and changes the log_scroll invariant from 0-based to 1-based to ensure the first scroll-down key-press always produces a visible change.

• Removed line-count guards (scroll_logs_down, scroll_logs_page_down, 'G' key): The old checks (if self.log_content.len() > 20 / > visible_lines) were left over from a previous scroll implementation and correctly removed; the rendering logic in ui.rs already handles short logs via LOG_VIEWPORT_MAX_LINES padding.
• log_scroll initialised to 1 (init, leave_logs, 'g' key): Fixes the no-op first scroll — with log_scroll = 0 the renderer clamped log_take to 1 via .clamp(1, LOG_VIEWPORT_MAX_LINES), so incrementing to 1 looked identical; starting at 1 means the first down-press actually advances to 2.
• Upward scrolls clamped to .max(1) (scroll_logs_up, scroll_logs_page_up, jump_to_log_match): Consistently enforces the new minimum-1 invariant.
• Potential edge case: The 'G' (go-to-bottom) handler in event.rs assigns log_scroll = app.log_content.len() without a .max(1) guard, which could set log_scroll = 0 if content is empty, re-introducing the original bug. In practice load_logs guarantees at least one element, but the defensive guard would make the invariant airtight.

Confidence Score: 4/5

• Safe to merge; fixes are targeted and correct with one minor defensive-coding opportunity.
• Both bug fixes are correct and the changes are small and well-scoped. The only concern is that the 'G' key handler in event.rs doesn't apply the new .max(1) guard, which is inconsistent with every other scroll path but unlikely to trigger given load_logs always produces non-empty content.
• No files require special attention; optionally tighten the 'G' handler in src/tui/event.rs.

Important Files Changed

Filename	Overview
src/tui/app.rs	Removes the obsolete > 20 / > visible_lines guards on scroll functions, initialises log_scroll to 1 instead of 0, and clamps all upward-scroll paths to a minimum of 1. The logic is sound; one edge-case worth noting is toggle_log_follow / the 'G' key where an empty log_content can still produce log_scroll = 0.
src/tui/event.rs	Updates 'g' (go-to-top) to set log_scroll = 1 and removes the > 20 guard from 'G' (go-to-bottom). The removal of the guard is correct; the 'G' handler now sets log_scroll = log_content.len() which can be 0 when content is empty.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[User presses scroll key] --> B{Key type?}
    B -->|Down or Ctrl+D| C[scroll_logs_down / scroll_logs_page_down]
    B -->|Up or Ctrl+U| D[scroll_logs_up / scroll_logs_page_up]
    B -->|g go-to-top| E[log_scroll = 1]
    B -->|G go-to-bottom| F[log_scroll = log_content.len]

    C --> G[log_scroll = log_scroll + step capped at log_content.len]
    D --> H[log_scroll = saturating_sub step then max 1]
    F --> I{log_content empty?}
    I -->|Yes - edge case| J[log_scroll = 0 - violates invariant]
    I -->|No - normal path| K[log_scroll = len which is at least 1]

    G --> L[Render with log_skip and log_take]
    H --> L
    E --> L
    K --> L
    J --> L

Loading

_{Last reviewed commit: 20626d8}

[gemini-code-assist]

Code Review

This pull request fixes issues with log scrolling, particularly when there are few log lines. The changes remove an obsolete check that prevented scrolling for logs with fewer than 20 lines. It also ensures that the scroll position (log_scroll) is handled more consistently by treating it as a 1-based index, preventing it from becoming 0 when scrolling up. The changes are logical and consistent across the affected files. I've added one comment regarding a minor inconsistency in scroll behavior when the logs are empty.