Fix file history for all sizes

[cruessler]

This Pull Request is a follow-up to #1648 and related to #1560.

It changes the following:

• It uses offset(), now exposed by ratatui, to keep items’ offset in sync with table_state’s offset.

I followed the checklist:

• I added unittests
• I ran make check without errors
• I tested the overall application
• I added an appropriate item to the changelog

[extrawurst]

thanks for tackling this.
I see a different behaviour now where the first view of file history does not show any entry selected and it only shows once you go up or down once

[cruessler]

thanks for tackling this. I see a different behaviour now where the first view of file history does not show any entry selected and it only shows once you go up or down once

I saw that, too. I assumed it was existing behaviour, but since it apparently is not, I’m going to fix it. :-) Thanks for the hint!

[cruessler]

The issue turned out to be more complex than I initially thought. I overhauled how selection as well as data fetching and refetching work in FileRevlogComponent. So far, I’ve found no issues. Behavior I tested:

• Opening the history for a file, scrolling up and down, opening commits and returning to the history view
• Closing the history for one file, opening it for another one, then repeating the tests done for the first file

[extrawurst]

there seems something off:

[extrawurst]

@cruessler any updates? putting this in draft in the meantime

[cruessler]

I haven’t gotten to look at the PR again yet. If I don’t get to it this week, then I’ll find time end of next week, I think.

[cruessler]

@extrawurst I think I found the issue. I rebased and manually tested the branch again, including the test case you provided.

[extrawurst]

now the scrolling works but the diffs still do not update:

Screenflick.Movie.3.mp4

[cruessler]

Thanks a lot for checking so thoroughly, that’s very much appreciated! I pushed another fix.

[extrawurst]

Visually it works now!

I am not sure if this implementation is working as intended though. I added a log::info in fetch_commits (line 221):

log::info!("revlog: fetch commits: {offset}");

if I reduce the batch size to force it to have to fetch commits (200 batch size) this shows that every arrow down leads to a new fetch_commits as soon as the batchsize-elements in view (or something else ^^) is reached:

(1) gitui::app: [src/app.rs:392] event: Input(Key(KeyEvent { code: Down, modifiers: NONE, kind: Press, state: NONE }))
revlog: fetch commits: 174
...
(1) gitui::app: [src/app.rs:392] event: Input(Key(KeyEvent { code: Down, modifiers: NONE, kind: Press, state: NONE }))
revlog: fetch commits: 175
...

this is not as efficient as it could be if we would fetch commit infos in batches and then only again once we run out.

side note: the selection should go up and down inside the view before the scroll position changes to keep the selection in view. right now if you scroll down it will stick to the bottom and the scroll position changes when scrolling up (although I am ok if this is done in a followup PR)

please also rebase on master particularly 9eb8d47 will affect this branch and make reading the log way easier :)

[extrawurst]

ok comparing with current master we should fix the scrolling up/down issue because on master it does it correctly with the scrolling window moving only when the selection reaches the upper/bottom border

putting it in draft again in the meantime, please ping me once you had time to look into it again 👍

[cruessler]

I updated the PR, tested it quite a bit, hoping not to have missed anything, and ran make check. That took slightly longer than expected, so I’ll take a walk now. :-)