fix(log): avoid panic when showing log of a branch with no commits#535
Merged
Merged
Conversation
30b5386 to
c7356cc
Compare
Opening the log of a branch with no commits panicked with "index out of bounds: the len is 0 but the index is 0". The log screen had no items, so `line_index` was empty, but `update_cursor` still tried to position the cursor and indexed `line_index[0]` via `at_line`. Guard `update_cursor` to reset the cursor when nothing is selectable, matching the existing empty-checks in the other cursor/scroll helpers. Closes altsem#262
c7356cc to
9dacc27
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Opening the log of a branch with no commits panics (fixes #262):
Root cause
The log screen for a branch with no commits has no items, so
line_indexis empty. ButScreen::new→update()→update_cursor()still tries to position the cursor:move_from_unselectablecallsnav_filter(self.cursor)→at_line(0), which does&self.items[self.line_index[0]]and panics on the emptyline_index.Fix
Guard
update_cursorto reset the cursor when nothing is selectable, instead of positioning it into an emptyline_index. This matches the existing empty-checks the other cursor/scroll helpers already use (scroll_fit_start,scroll_fit_end,move_cursor_to_screen_line).I kept the fix to the single chokepoint on the panic path rather than changing
get_selected_item()/at_line()to returnOption, sinceget_selected_item()has ~12 call sites across the crate — that would be a large ripple for this bug.Test
Added
log_empty_branchinsrc/tests/log.rs(same empty-repo setup as the existingfresh_inittest, thenl l). Before the fix it panics; after, it renders the empty log. The snapshot is blank because a branch with no commits has nothing to show — the test's purpose is that a regression would panic before reaching the assertion.make testpasses locally (cargo insta test --unreferenced reject,cargo clippy -- -Dwarnings,cargo fmt --check,cargo bench --no-run); full suite is green.