feat: show line counts in pull request picker

This commit is contained in:
Valdemar Lorenzen
2026-10-21 10:06:48 +02:00
parent 31203889bb
commit cce3b1e469
4 changed files with 234 additions and 22 deletions
+1
View File
@@ -1,3 +1,4 @@
- [ ] improve: pull requests - add a tab for drafts so PRs in this state don't populate the "open" tab.
- [ ] feature: diff view - GitHub code review integration after the local review UX is usable. - [ ] feature: diff view - GitHub code review integration after the local review UX is usable.
- publish local reviews explicitly: general PR comments, inline line/range comments, review summaries, approval, and requests for changes. - publish local reviews explicitly: general PR comments, inline line/range comments, review summaries, approval, and requests for changes.
- preserve explicit PR identity and reviewed commit IDs; validate anchors against GitHub's PR diff and recheck the PR head before submission. - preserve explicit PR identity and reviewed commit IDs; validate anchors against GitHub's PR diff and recheck the PR head before submission.
+2
View File
@@ -406,6 +406,8 @@ The selector initially lists local revisions and known tracking refs **without c
**Pull requests:** run **`:pull_requests [remote]`**, use the palette's `pull_requests` action, or bind **`config.core.pull_requests`**. The floating picker has **Open / Closed / All** tabs; Closed includes merged requests. Tab/Shift+Tab changes category, typing fuzzy-searches its number, title, author, state, and branches, and Enter immediately opens the selected request in **Branch changes** mode using its immutable head and merge base. Escape restores the source buffer. A failed list can be retried with Enter. The default remote is `origin`, or the sole configured remote; specify a name when there are multiple remotes without `origin`. **Pull requests:** run **`:pull_requests [remote]`**, use the palette's `pull_requests` action, or bind **`config.core.pull_requests`**. The floating picker has **Open / Closed / All** tabs; Closed includes merged requests. Tab/Shift+Tab changes category, typing fuzzy-searches its number, title, author, state, and branches, and Enter immediately opens the selected request in **Branch changes** mode using its immutable head and merge base. Escape restores the source buffer. A failed list can be retried with Enter. The default remote is `origin`, or the sole configured remote; specify a name when there are multiple remotes without `origin`.
Each pull request's metadata shows added/deleted line counts after its state, before the branch names: `author · open · +123 -45 · topic → main`. Zero counts are shown explicitly; missing, denied, or failed statistics show `size unavailable` without preventing selection. Counts come from one batched read-only statistics query per nonempty GitHub/GitLab page ([GitHub](https://docs.github.com/en/graphql/reference/pulls), [GitLab](https://docs.gitlab.com/api/graphql/reference/#diffstatssummary)); [Gitea](https://docs.gitea.com/api/operations/repo-list-pull-requests/) supplies them directly in its list response. GitHub statistics require API authentication even for public repositories. Listing does not fetch source diffs.
GitHub, GitLab merge requests, and Gitea are supported. API reads require `curl`; public repositories work anonymously. Authentication uses `GH_TOKEN`/`GITHUB_TOKEN` for github.com, `GH_ENTERPRISE_TOKEN`/`GITHUB_ENTERPRISE_TOKEN` for GitHub Enterprise, `GITLAB_TOKEN` for GitLab, and `GITEA_TOKEN` for Gitea. With no matching environment token, GitHub/GitLab reuse `gh auth token --hostname HOST` / `glab auth token --hostname HOST` when available. Tokens are supplied to curl on stdin, not in subprocess arguments, and sent only to the HTTPS API; redirects are not followed. Git object fetching uses the configured remote's existing Git/SSH credentials separately. GitHub, GitLab merge requests, and Gitea are supported. API reads require `curl`; public repositories work anonymously. Authentication uses `GH_TOKEN`/`GITHUB_TOKEN` for github.com, `GH_ENTERPRISE_TOKEN`/`GITHUB_ENTERPRISE_TOKEN` for GitHub Enterprise, `GITLAB_TOKEN` for GitLab, and `GITEA_TOKEN` for Gitea. With no matching environment token, GitHub/GitLab reuse `gh auth token --hostname HOST` / `glab auth token --hostname HOST` when available. Tokens are supplied to curl on stdin, not in subprocess arguments, and sent only to the HTTPS API; redirects are not followed. Git object fetching uses the configured remote's existing Git/SSH credentials separately.
Standard GitHub/GitLab/Gitea hosts and Codeberg are detected from HTTPS or SSH remote URLs. For another host, set `remote.<remote>.vamp-forge` to `github`, `gitlab`, or `gitea` in the repository's Git configuration, for example `git config remote.origin.vamp-forge gitea`. For a non-colocated jj repository, use `git --git-dir .jj/repo/store/git config remote.origin.vamp-forge gitea`. Standard GitHub/GitLab/Gitea hosts and Codeberg are detected from HTTPS or SSH remote URLs. For another host, set `remote.<remote>.vamp-forge` to `github`, `gitlab`, or `gitea` in the repository's Git configuration, for example `git config remote.origin.vamp-forge gitea`. For a non-colocated jj repository, use `git --git-dir .jj/repo/store/git config remote.origin.vamp-forge gitea`.
+141 -13
View File
@@ -42,6 +42,7 @@ struct Forge {
provider: Provider, provider: Provider,
host: String, host: String,
endpoint: String, endpoint: String,
repository: String,
} }
fn encode(text: &str) -> String { fn encode(text: &str) -> String {
@@ -237,6 +238,7 @@ impl Forge {
provider, provider,
host: authority, host: authority,
endpoint, endpoint,
repository: path.to_owned(),
}) })
} }
@@ -308,14 +310,18 @@ impl Forge {
.transpose() .transpose()
} }
fn get( fn request(
&self, &self,
url: &str, url: &str,
body: Option<&str>,
token: Option<&str>, token: Option<&str>,
root: &Path, root: &Path,
cancelled: &AtomicBool, cancelled: &AtomicBool,
) -> Result<Value, String> { ) -> Result<Value, String> {
let mut headers = String::from("Accept: application/json\n"); let mut headers = String::from("Accept: application/json\n");
if body.is_some() {
headers.push_str("Content-Type: application/json\n");
}
if let Some(token) = token { if let Some(token) = token {
let prefix = match self.provider { let prefix = match self.provider {
Provider::Github => "Authorization: Bearer", Provider::Github => "Authorization: Bearer",
@@ -347,6 +353,9 @@ impl Forge {
"--url", "--url",
url, url,
]); ]);
if let Some(body) = body {
command.args(["--data-binary", body]);
}
let bytes = checked(run(command, headers.into_bytes(), cancelled)?).map_err( let bytes = checked(run(command, headers.into_bytes(), cancelled)?).map_err(
|_| "Forge API request failed; check curl, network access, and authentication", |_| "Forge API request failed; check curl, network access, and authentication",
)?; )?;
@@ -374,7 +383,7 @@ impl Forge {
&self, &self,
closed: bool, closed: bool,
page: u32, page: u32,
mut get: impl FnMut(&str) -> Result<Value, String>, mut request: impl FnMut(&str, Option<&str>) -> Result<Value, String>,
) -> Result<PullRequestPage, String> { ) -> Result<PullRequestPage, String> {
if page == 0 { if page == 0 {
return Err("Forge page numbers start at 1".into()); return Err("Forge page numbers start at 1".into());
@@ -394,17 +403,26 @@ impl Forge {
// Gitea's default sort is descending creation time; "oldest" reverses it. // Gitea's default sort is descending creation time; "oldest" reverses it.
Provider::Gitea => "limit=50", Provider::Gitea => "limit=50",
}; };
let values = get(&format!( let values = request(
"{}?state={state}&{query}&page={page}", &format!("{}?state={state}&{query}&page={page}", self.endpoint),
self.endpoint None,
))?; )?;
let values = values let values = values
.as_array() .as_array()
.ok_or("Forge API returned a non-array pull request list")?; .ok_or("Forge API returned a non-array pull request list")?;
// Servers may cap page sizes. Only an empty page proves exhaustion. // Servers may cap page sizes. Only an empty page proves exhaustion.
has_more |= !values.is_empty(); has_more |= !values.is_empty();
let sizes = self.sizes_with(values, &mut request);
for value in values { for value in values {
let entry = summary(self.provider, value)?; let id = number(
value,
if self.provider == Provider::Gitlab {
"iid"
} else {
"number"
},
)?;
let entry = summary(self.provider, value, sizes.get(&id).copied())?;
if entry.closed != closed { if entry.closed != closed {
return Err("Forge API returned a pull request in the wrong category".into()); return Err("Forge API returned a pull request in the wrong category".into());
} }
@@ -422,6 +440,108 @@ impl Forge {
Ok(PullRequestPage { entries, has_more }) Ok(PullRequestPage { entries, has_more })
} }
/// One statistics query per page, never one detail request per pull request.
fn sizes_with(
&self,
values: &[Value],
request: &mut impl FnMut(&str, Option<&str>) -> Result<Value, String>,
) -> BTreeMap<u64, (u64, u64)> {
if values.is_empty() {
return BTreeMap::new();
}
if self.provider == Provider::Gitea {
return values
.iter()
.filter_map(|value| {
Some((
value.get("number")?.as_u64()?,
(
value.get("additions")?.as_u64()?,
value.get("deletions")?.as_u64()?,
),
))
})
.collect();
}
let mut fields = String::new();
for value in values {
let Some(id) = value
.get(if self.provider == Provider::Gitlab {
"iid"
} else {
"number"
})
.and_then(Value::as_u64)
else {
continue;
};
use std::fmt::Write;
match self.provider {
Provider::Github => write!(
fields,
"p{id}:pullRequest(number:{id}){{additions deletions}}"
)
.unwrap(),
Provider::Gitlab => write!(
fields,
"p{id}:mergeRequest(iid:\"{id}\"){{diffStatsSummary{{additions deletions}}}}"
)
.unwrap(),
Provider::Gitea => unreachable!(),
}
}
let (url, query) = if self.provider == Provider::Github {
let (owner, name) = self.repository.split_once('/').unwrap();
let api = self.endpoint.split_once("/repos/").unwrap().0;
(
format!("{}/graphql", api.strip_suffix("/v3").unwrap_or(api)),
format!(
"query{{repository(owner:{},name:{}){{{fields}}}}}",
serde_json::to_string(owner).unwrap(),
serde_json::to_string(name).unwrap()
),
)
} else {
(
format!("https://{}/api/graphql", self.host),
format!(
"query{{project(fullPath:{}){{{fields}}}}}",
serde_json::to_string(&self.repository).unwrap()
),
)
};
let body = serde_json::json!({ "query": query }).to_string();
// Missing/denied statistics must not prevent browsing or imply a zero-size PR.
let Ok(response) = request(&url, Some(&body)) else {
return BTreeMap::new();
};
let container = if self.provider == Provider::Github {
"/data/repository"
} else {
"/data/project"
};
let Some(entries) = response.pointer(container).and_then(Value::as_object) else {
return BTreeMap::new();
};
entries
.iter()
.filter_map(|(alias, value)| {
let value = if self.provider == Provider::Gitlab {
value.get("diffStatsSummary")?
} else {
value
};
Some((
alias.strip_prefix('p')?.parse().ok()?,
(
value.get("additions")?.as_u64()?,
value.get("deletions")?.as_u64()?,
),
))
})
.collect()
}
fn resolve_with( fn resolve_with(
&self, &self,
id: u64, id: u64,
@@ -567,7 +687,11 @@ fn created_at(value: &Value) -> Result<String, String> {
)) ))
} }
fn summary(provider: Provider, value: &Value) -> Result<PullRequestSummary, String> { fn summary(
provider: Provider,
value: &Value,
size: Option<(u64, u64)>,
) -> Result<PullRequestSummary, String> {
let gitlab = provider == Provider::Gitlab; let gitlab = provider == Provider::Gitlab;
let (closed, merged) = status(provider, value)?; let (closed, merged) = status(provider, value)?;
let author = author(provider, value)?; let author = author(provider, value)?;
@@ -578,6 +702,10 @@ fn summary(provider: Provider, value: &Value) -> Result<PullRequestSummary, Stri
} else { } else {
"open" "open"
}; };
let size = size.map_or_else(
|| "size unavailable".into(),
|(added, deleted)| format!("+{added} -{deleted}"),
);
let (base, head) = if gitlab { let (base, head) = if gitlab {
( (
value.get("target_branch").and_then(Value::as_str), value.get("target_branch").and_then(Value::as_str),
@@ -601,8 +729,8 @@ fn summary(provider: Provider, value: &Value) -> Result<PullRequestSummary, Stri
number: number(value, if gitlab { "iid" } else { "number" })?, number: number(value, if gitlab { "iid" } else { "number" })?,
title: field(value, "title")?.to_owned(), title: field(value, "title")?.to_owned(),
description: match (base, head) { description: match (base, head) {
(Some(base), Some(head)) => format!("{author} · {state} · {head} → {base}"), (Some(base), Some(head)) => format!("{author} · {state} · {size} · {head} → {base}"),
_ => format!("{author} · {state}"), _ => format!("{author} · {state} · {size}"),
}, },
closed, closed,
created_at: created_at(value)?, created_at: created_at(value)?,
@@ -788,8 +916,8 @@ pub(crate) fn page(
ssh_hostname(command, destination, port, cancelled) ssh_hostname(command, destination, port, cancelled)
})?; })?;
let token = forge.token(root, cancelled)?; let token = forge.token(root, cancelled)?;
forge.page_with(closed, page, |url| { forge.page_with(closed, page, |url, body| {
forge.get(url, token.as_deref(), root, cancelled) forge.request(url, body, token.as_deref(), root, cancelled)
}) })
} }
@@ -807,7 +935,7 @@ pub(crate) fn resolve(
})?; })?;
let token = forge.token(root, cancelled)?; let token = forge.token(root, cancelled)?;
forge.resolve_with(number, |url| { forge.resolve_with(number, |url| {
forge.get(url, token.as_deref(), root, cancelled) forge.request(url, None, token.as_deref(), root, cancelled)
}) })
} }
+90 -9
View File
@@ -7,6 +7,82 @@ fn parse(url: &str, provider: Option<&str>) -> Result<Forge, String> {
}) })
} }
#[test]
fn pull_request_sizes_include_zero_and_preserve_unknown_statistics() {
for (url, provider) in [
("https://github.com/owner/repo", Provider::Github),
("https://github.enterprise/owner/repo", Provider::Github),
("https://gitlab.com/group/repo", Provider::Gitlab),
("https://codeberg.org/owner/repo", Provider::Gitea),
] {
let forge = parse(
url,
Some(match provider {
Provider::Github => "github",
Provider::Gitlab => "gitlab",
Provider::Gitea => "gitea",
}),
)
.unwrap();
let page = forge
.page_with(false, 1, |url, body| {
if let Some(body) = body {
let query: Value = serde_json::from_str(body).unwrap();
let query = query["query"].as_str().unwrap();
assert!(query.starts_with("query{"));
assert!(!query.contains("mutation"));
if provider == Provider::Github {
assert_eq!(
url,
if forge.host == "github.com" {
"https://api.github.com/graphql"
} else {
"https://github.enterprise/api/graphql"
}
);
assert!(query.contains("p7:pullRequest(number:7)"));
return Ok(json!({"data":{"repository":{
"p7":{"additions":123,"deletions":45},
"p8":{"additions":0,"deletions":0},
"p9":null
}}}));
}
assert_eq!(url, "https://gitlab.com/api/graphql");
assert!(query.contains("p7:mergeRequest(iid:\"7\")"));
return Ok(json!({"data":{"project":{
"p7":{"diffStatsSummary":{"additions":123,"deletions":45}},
"p8":{"diffStatsSummary":{"additions":0,"deletions":0}},
"p9":{"diffStatsSummary":null}
}}}));
}
Ok(Value::Array((7..=9).map(|id| json!({
"merged_at":null, "merged":false,
"number":id, "iid":id, "title":"Sized PR",
"state":if provider == Provider::Gitlab { "opened" } else { "open" },
"user":{"login":"author"}, "author":{"username":"author"},
"created_at":"2024-01-01T00:00:00Z",
"base":{"ref":"main"}, "head":{"ref":"topic"},
"target_branch":"main", "source_branch":"topic",
"additions":match id { 7 => Some(123), 8 => Some(0), _ => None },
"deletions":match id { 7 => Some(45), 8 => Some(0), _ => None }
})).collect()))
})
.unwrap();
let description = |id| {
&page
.entries
.iter()
.find(|entry| entry.number == id)
.unwrap()
.description
};
assert!(description(7).contains("open · +123 -45 · topic → main"));
assert!(description(8).contains("open · +0 -0 · topic → main"));
assert!(description(9).contains("size unavailable"));
assert!(!description(9).contains("+0 -0"));
}
}
#[test] #[test]
fn pages_are_bounded_newest_first_summaries_without_historical_metadata() { fn pages_are_bounded_newest_first_summaries_without_historical_metadata() {
for (provider, url, query) in [ for (provider, url, query) in [
@@ -27,9 +103,10 @@ fn pages_are_bounded_newest_first_summaries_without_historical_metadata() {
), ),
] { ] {
let forge = parse(url, None).unwrap(); let forge = parse(url, None).unwrap();
let mut calls = 0; let page = forge.page_with(false, 3, |url, body| {
let page = forge.page_with(false, 3, |url| { if body.is_some() {
calls += 1; return Err("statistics unavailable".into());
}
assert_eq!(url, format!("{}?{query}", forge.endpoint)); assert_eq!(url, format!("{}?{query}", forge.endpoint));
Ok(json!([ Ok(json!([
{"number": 1, "iid": 1, "title": "Older", "state": if provider == Provider::Gitlab { "opened" } else { "open" }, {"number": 1, "iid": 1, "title": "Older", "state": if provider == Provider::Gitlab { "opened" } else { "open" },
@@ -39,22 +116,20 @@ fn pages_are_bounded_newest_first_summaries_without_historical_metadata() {
"created_at": "2024-01-01T00:00:00.001Z", "diff_refs": null, "head": {"sha": "not-a-commit"}} "created_at": "2024-01-01T00:00:00.001Z", "diff_refs": null, "head": {"sha": "not-a-commit"}}
])) ]))
}).unwrap(); }).unwrap();
assert_eq!(calls, 1);
assert_eq!( assert_eq!(
page.entries.iter().map(|p| p.number).collect::<Vec<_>>(), page.entries.iter().map(|p| p.number).collect::<Vec<_>>(),
[2, 1] [2, 1]
); );
assert_eq!(page.entries[0].description, "author · open");
assert!(page.entries[1].description.contains("deleted user")); assert!(page.entries[1].description.contains("deleted user"));
assert!( assert!(
page.has_more, page.has_more,
"A short, server-capped page is not exhaustion" "A short, server-capped page is not exhaustion"
); );
let empty = forge.page_with(false, 4, |_| Ok(json!([]))).unwrap(); let empty = forge.page_with(false, 4, |_, _| Ok(json!([]))).unwrap();
assert!(empty.entries.is_empty() && !empty.has_more); assert!(empty.entries.is_empty() && !empty.has_more);
assert!( assert!(
forge forge
.page_with(false, 0, |_| panic!("Page zero must not request")) .page_with(false, 0, |_, _| panic!("Page zero must not request"))
.is_err() .is_err()
); );
} }
@@ -67,7 +142,10 @@ fn gitlab_closed_combines_closed_and_merged_pages_without_open_requests() {
for page in 1..=3 { for page in 1..=3 {
let mut states = Vec::new(); let mut states = Vec::new();
let result = forge let result = forge
.page_with(true, page, |url| { .page_with(true, page, |url, body| {
if body.is_some() {
return Err("statistics unavailable".into());
}
assert!(url.ends_with(&format!("&page={page}"))); assert!(url.ends_with(&format!("&page={page}")));
let state = if url.contains("?state=closed&") { let state = if url.contains("?state=closed&") {
"closed" "closed"
@@ -100,7 +178,10 @@ fn gitlab_closed_combines_closed_and_merged_pages_without_open_requests() {
} }
all.sort_unstable_by(|a, b| b.created_at.cmp(&a.created_at)); all.sort_unstable_by(|a, b| b.created_at.cmp(&a.created_at));
assert_eq!(all.iter().map(|p| p.number).collect::<Vec<_>>(), [3, 2, 1]); assert_eq!(all.iter().map(|p| p.number).collect::<Vec<_>>(), [3, 2, 1]);
let raced = forge.page_with(true, 1, |url| { let raced = forge.page_with(true, 1, |url, body| {
if body.is_some() {
return Err("statistics unavailable".into());
}
Ok(json!([{"iid": 7, "title": "Raced merge", "state": if url.contains("?state=merged&") { "merged" } else { "closed" }, Ok(json!([{"iid": 7, "title": "Raced merge", "state": if url.contains("?state=merged&") { "merged" } else { "closed" },
"author": null, "created_at": "2024-01-01T00:00:00Z"}])) "author": null, "created_at": "2024-01-01T00:00:00Z"}]))
}).unwrap(); }).unwrap();