Skip to content

feat(app): support expandable commit list and adjust default commit loading#138

Merged
agavra merged 6 commits intoagavra:mainfrom
saleemdjima:feat/expand-commit-list
Jan 28, 2026
Merged

feat(app): support expandable commit list and adjust default commit loading#138
agavra merged 6 commits intoagavra:mainfrom
saleemdjima:feat/expand-commit-list

Conversation

@saleemdjima
Copy link
Copy Markdown
Contributor

@saleemdjima saleemdjima commented Jan 23, 2026 •
edited by agavra
Loading

This pull request resolves issues #129 and #39.

fixes #129
fixes #39

Changes

  • Added a new field to the app : commit_expanded to track whether the commit list is expanded or not.
  • Increased the number of commits loaded by default from 5 to 20 for recent sessions.
  • Kept the visible commit limit at 5

Notes

  • I'm considering introducing constants for the default commit limit and default loaded value to make these values more maintenable. Would this be a good idea?

agavra
agavra reviewed Jan 23, 2026
View reviewed changes
Copy link
Copy Markdown
Owner

@agavra agavra left a comment

Choose a reason for hiding this comment

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

@saleemdjima the experience with this change is good, but the way it's implemented is a bit rigid. I'd like to be able to have "fetch more" actually fetch more instead of just hiding/showing 15 commits

src/app.rs Outdated
Err(TuicrError::NoChanges) => {
// No unstaged changes - try to get recent commits
let commits = vcs.get_recent_commits(5)?;
let commits = vcs.get_recent_commits(20)?;
Copy link
Copy Markdown
Owner

@agavra agavra Jan 23, 2026

Choose a reason for hiding this comment

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

I think it'd be nice not to have a limit and to allow continuing to hit "show more" to always shore more commits. if that's challenging to implement we can just fetch 100 and every time you hit "show more" we just show 10 more commits

Copy link
Copy Markdown
Contributor Author

@saleemdjima saleemdjima Jan 23, 2026

Choose a reason for hiding this comment

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

Thanks for the feedback! That makes sense. I agree that “fetch more” should actually fetch additional commits. I’ll rework this to support incremental loading.

@saleemdjima
Copy link
Copy Markdown
Contributor Author

saleemdjima commented Jan 25, 2026

This update introduces incremental commit loading along with viewport-based scrolling for the commit list. I added some constants :

  • DEFAULT_COMMIT_COUNT: a default maximum number of commits to fetch,
  • VISIBLE_COMMIT_COUNT: a default number of visible commits,
  • COMMIT_PAGE_SIZE: a fixed page size for incremental display.

Design decision

Fully loading all commits would require changing the get_recent_commits function signature from usize to Option. To keep this PR focused and avoid introducing cross-backend breaking changes, I didn't modify the trait signature here. I think that It will be better to do such modification in a dedicated new pull request.

@agavra
Copy link
Copy Markdown
Owner

agavra commented Jan 26, 2026 •
edited
Loading

Fully loading all commits would require changing the get_recent_commits function signature from usize to Option. To keep this PR focused and avoid introducing cross-backend breaking changes, I didn't modify the trait signature here. I think that It will be better to do such modification in a dedicated new pull request.

@saleemdjima I'm sorry to expand the scope of this but I'd rather we just implemented that in this PR, fetching 1000 commits always seems like overkill and I'd like this functionality to work across all of them (hg/jj/git)

LMK if you don't have bandwidth to take that on and I can add it to my backlog.

@saleemdjima
Copy link
Copy Markdown
Contributor Author

saleemdjima commented Jan 26, 2026

Ah ok, sorry I think I misunderstood earlier ! I'm updating the implementation and make it consistent across git / hg / jj in this PR. Thanks !

@saleemdjima saleemdjima force-pushed the feat/expand-commit-list branch from c75300e to 8da3cb5 Compare January 27, 2026 01:45
agavra
agavra approved these changes Jan 28, 2026
View reviewed changes
Copy link
Copy Markdown
Owner

@agavra agavra left a comment

Choose a reason for hiding this comment

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

This is great, thank you @saleemdjima for the contribution 🔥

@agavra agavra merged commit 06c6f6a into agavra:main Jan 28, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@agavra agavra agavra approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

support "...show more" on commit selection screen when selecting commits to review, allow viewing more than 5

2 participants

@saleemdjima @agavra