repo-search: don't tell the caller to raise --limit once the forge has already clamped
This commit is contained in:
parent
acb5b676d7
commit
c335bc0962
2 changed files with 45 additions and 6 deletions
|
|
@ -345,9 +345,14 @@ to discover valid label names before triaging or to audit the label set.
|
||||||
with the real remaining count when it's present (`… 50 of 187 shown …
|
with the real remaining count when it's present (`… 50 of 187 shown …
|
||||||
137 more exist`); without it (an older forge, a stripping proxy) it
|
137 more exist`); without it (an older forge, a stripping proxy) it
|
||||||
falls back to the cruder `count == limit` heuristic, same
|
falls back to the cruder `count == limit` heuristic, same
|
||||||
trust-the-trailer-not-the-row-count principle as `list` above. Either
|
trust-the-trailer-not-the-row-count principle as `list` above. Unlike
|
||||||
way it can't tell "doesn't exist" from "you lack permission to see
|
`list`, this verb has no `--page` — so once the forge's own page cap
|
||||||
it" — both render as an absent row with no signal either way — and
|
has clamped the result (`count < limit`), raising `--limit` again asks
|
||||||
it matches repo **name** only, not owner, so there's no way to
|
for the same clamp and gets nothing new; the warning says to narrow
|
||||||
enumerate an org's repos through this verb.
|
`--query` instead in that case, and only suggests raising `--limit`
|
||||||
|
while still under the cap. Either way it can't tell "doesn't exist"
|
||||||
|
from "you lack permission to see it" — both render as an absent row
|
||||||
|
with no signal either way — and it matches repo **name** only, not
|
||||||
|
owner, so there's no way to enumerate an org's repos through this
|
||||||
|
verb.
|
||||||
<!-- vale write-good.Passive = YES -->
|
<!-- vale write-good.Passive = YES -->
|
||||||
|
|
|
||||||
|
|
@ -155,10 +155,23 @@ fn truncation_warning(total: Option<u64>, count: u64, limit: u64) -> Option<Stri
|
||||||
Some(total) => {
|
Some(total) => {
|
||||||
let remaining = total.saturating_sub(count);
|
let remaining = total.saturating_sub(count);
|
||||||
(remaining > 0).then(|| {
|
(remaining > 0).then(|| {
|
||||||
|
// `count < limit` means the forge clamped the page below what
|
||||||
|
// was asked for — raising `--limit` again asks for the same
|
||||||
|
// clamp and gets the same `count` back. Unlike `list`, this
|
||||||
|
// verb has no `--page` to walk past the clamp with, so
|
||||||
|
// narrowing `--query` is the only remedy that can work once
|
||||||
|
// the server has capped the page. "raise --limit" only
|
||||||
|
// helps the other case: still under the cap, where asking
|
||||||
|
// for more actually serves more of it.
|
||||||
|
let remedy = if count < limit {
|
||||||
|
"the forge capped this page at its own limit — narrow --query to see them"
|
||||||
|
} else {
|
||||||
|
"raise --limit or narrow --query to see them"
|
||||||
|
};
|
||||||
format!(
|
format!(
|
||||||
"warning: {count} of {total} results shown — {remaining} more exist (the \
|
"warning: {count} of {total} results shown — {remaining} more exist (the \
|
||||||
forge still silently drops any repo you lack permission to see, with no \
|
forge still silently drops any repo you lack permission to see, with no \
|
||||||
signal either way); raise --limit or narrow --query to see them"
|
signal either way); {remedy}"
|
||||||
)
|
)
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
@ -194,6 +207,27 @@ mod tests {
|
||||||
assert!(msg.contains("137 more"), "{msg}");
|
assert!(msg.contains("137 more"), "{msg}");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn clamp_case_tells_the_caller_to_narrow_not_to_raise_limit() {
|
||||||
|
// `--limit` already exceeded the forge's own page cap here, so
|
||||||
|
// "raise --limit" is a dead end — only `--query` can still narrow
|
||||||
|
// the result set below the cap. (The bug argus caught: both
|
||||||
|
// branches used to say "raise --limit", which can't work once the
|
||||||
|
// server itself has clamped the page.)
|
||||||
|
let msg = truncation_warning(Some(187), 50, 400).expect("137 more exist");
|
||||||
|
assert!(msg.contains("narrow --query"), "{msg}");
|
||||||
|
assert!(!msg.contains("raise --limit"), "{msg}");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn under_the_cap_case_still_offers_raise_limit() {
|
||||||
|
// `count == limit` and both are under the forge's cap: asking for
|
||||||
|
// more can actually serve more, so "raise --limit" is live advice
|
||||||
|
// here, not a dead end.
|
||||||
|
let msg = truncation_warning(Some(187), 30, 30).expect("157 more exist");
|
||||||
|
assert!(msg.contains("raise --limit"), "{msg}");
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn stays_quiet_once_the_total_is_accounted_for() {
|
fn stays_quiet_once_the_total_is_accounted_for() {
|
||||||
assert_eq!(truncation_warning(Some(30), 30, 30), None);
|
assert_eq!(truncation_warning(Some(30), 30, 30), None);
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue