Title: --all flag caps at 1000 entries but doesn't document this limit
Labels: documentation, enhancement
Body:
The --all flag promises to show "all reflog entries" but silently caps at 1000:
let limit = if show_all { "1000" } else { "50" };Options:
- Update help text to mention the limit:
--all # Show all reflog entries (max 1000) - Remove the cap entirely and truly show all entries
Priority: Medium - affects user expectations but not functionality
Title: Refactor: Extract duplicate diff refresh logic
Labels: refactoring, code-quality
Body: The "update diff if showing" logic is duplicated in 6 places:
next()previous()Homekey handlerEndkey handlerPageDownkey handlerPageUpkey handler
Suggestion: Extract to a helper method:
fn update_diff_if_visible(&mut self) -> Result<()> {
if self.show_diff {
let idx = self.selected_index();
if let Some(entry) = self.entries.get(idx) {
self.diff_content = self.git_manager.get_diff_stat(&entry.hash)?;
self.diff_scroll_offset = 0;
}
}
Ok(())
}Then call it after any selection change.
Priority: Low - code quality improvement, no user-facing impact
Title: Optimize: Pull selected_index() out of map closure
Labels: performance, code-quality
Body:
In ui(), app.selected_index() is called once per list item inside the .map() closure:
.map(|(i, entry)| {
let selected_idx = app.selected_index(); // Called N times
let is_selected = i == selected_idx;
// ...
})Suggestion: Pull it out before the iterator:
let selected_idx = app.selected_index();
let items: Vec<ListItem> = app.entries.iter()
.enumerate()
.map(|(i, entry)| {
let is_selected = i == selected_idx;
// ...
})Priority: Low - micro-optimization, the method is cheap (just unwrap_or)
Credit: Issues identified by code reviewer feedback