hive-forge list's paging trailer counts the rows actually shown
The trailer computed shown as page * rows-on-this-page, so a short last page (page 27 of 1331 rows at 50/page, 31 rows) was counted as 837 of 1331 and printed a hint to fetch page 28. A page whose unclamped row offset lands exactly on the total is now recognised as the last one, so the trailer stays silent there. Shown is also clamped to the total.
This commit is contained in:
parent
096b45867d
commit
6859eb7f37
1 changed files with 52 additions and 4 deletions
|
|
@ -218,9 +218,12 @@ pub fn run(client: &Client, args: Args) -> Result<()> {
|
|||
///
|
||||
/// `total` is `X-Total-Count`, which Forgejo sends on this endpoint. The
|
||||
/// *effective* page size isn't reported, but it doesn't need to be: a
|
||||
/// short page is by definition the last one, so whenever more pages can
|
||||
/// exist at all, `count` **is** the page size and `page * count` is how
|
||||
/// many rows have been shown up to here.
|
||||
/// short page is by definition the last one, so on a page that is not the
|
||||
/// last, `count` **is** the page size and `page * count` is how many rows
|
||||
/// have been shown up to here. On the last page `count` is only the
|
||||
/// remainder, so `page * count` undercounts; there the unclamped
|
||||
/// `(page - 1) * limit + count` lands exactly on `total`, which identifies
|
||||
/// the page as the last one.
|
||||
///
|
||||
/// Deliberately not clamping `--limit` client-side to 50: that number is
|
||||
/// the *remote's* configuration, and baking a peer's setting into the
|
||||
|
|
@ -233,7 +236,15 @@ fn trailer(total: Option<u64>, count: u64, page: u64, limit: u64) -> Option<Stri
|
|||
}
|
||||
match total {
|
||||
Some(total) => {
|
||||
let shown = page.saturating_mul(count);
|
||||
let by_limit = page
|
||||
.saturating_sub(1)
|
||||
.saturating_mul(limit)
|
||||
.saturating_add(count);
|
||||
let shown = if by_limit == total {
|
||||
total
|
||||
} else {
|
||||
page.saturating_mul(count).min(total)
|
||||
};
|
||||
let remaining = total.saturating_sub(shown);
|
||||
(remaining > 0).then(|| {
|
||||
format!(
|
||||
|
|
@ -514,4 +525,41 @@ mod tests {
|
|||
// Short page with no header: the end, as far as we can tell.
|
||||
assert_eq!(trailer(None, 12, 1, 30), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn trailer_counts_rows_shown_on_first_and_middle_pages() {
|
||||
let msg = trailer(Some(1331), 50, 1, 50).expect("more pages");
|
||||
assert!(msg.contains("50 of 1331 shown"), "{msg}");
|
||||
assert!(msg.contains("1281 more"), "{msg}");
|
||||
let msg = trailer(Some(1331), 50, 26, 50).expect("one page left");
|
||||
assert!(msg.contains("1300 of 1331 shown"), "{msg}");
|
||||
assert!(msg.contains("31 more"), "{msg}");
|
||||
assert!(msg.contains("--page 27"), "{msg}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn trailer_short_last_page_prints_no_hint() {
|
||||
// Page 27 of 1331 rows at 50/page holds 31: `page * count` gave
|
||||
// 837 of 1331 and a hint to fetch page 28.
|
||||
assert_eq!(trailer(Some(1331), 31, 27, 50), None);
|
||||
// Page 2 of 2: `2 * 5 = 10 < 55` also produced a bogus hint.
|
||||
assert_eq!(trailer(Some(55), 5, 2, 50), None);
|
||||
// Control: the page before the last still hints.
|
||||
assert!(trailer(Some(1331), 50, 26, 50).is_some());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn trailer_exact_multiple_last_page_is_silent() {
|
||||
assert_eq!(trailer(Some(100), 50, 2, 50), None);
|
||||
// Control: page 1 of the same list still hints.
|
||||
let msg = trailer(Some(100), 50, 1, 50).expect("page 2 remains");
|
||||
assert!(msg.contains("50 of 100 shown"), "{msg}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn trailer_without_total_is_silent_on_a_short_page() {
|
||||
assert_eq!(trailer(None, 31, 27, 50), None);
|
||||
// Control: a full page still hints.
|
||||
assert!(trailer(None, 50, 27, 50).is_some());
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue