diff --git a/TODO.md b/TODO.md index 7ca680f..242b14d 100644 --- a/TODO.md +++ b/TODO.md @@ -1,4 +1,6 @@ -- [ ] improve: pull requests - add a tab for drafts so PRs in this state don't populate the "open" tab. +- [x] improve: pull requests - add a tab for drafts so PRs in this state don't populate the "open" tab. +- [ ] improve: pull requests - add a checkmark left of the PR entry in the PR list if the user in question (me) has already approved it. +- [ ] chore: check to make sure we're mocking everything in tests, not pinging/fetching real data from real websites/APIs. - [ ] 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. - preserve explicit PR identity and reviewed commit IDs; validate anchors against GitHub's PR diff and recheck the PR head before submission. diff --git a/docs/editing.md b/docs/editing.md index 5677688..147f868 100644 --- a/docs/editing.md +++ b/docs/editing.md @@ -404,7 +404,7 @@ Printable input belongs to the focused query, including `:`, `_`, and `|`; Norma The selector initially lists local revisions and known tracking refs **without contacting remotes**. **Load refs from origin** (or another configured remote) in Bookmarks/Branches/Tags lists advertised references, including refs never fetched locally, without advancing the selection stage. Selecting a reference such as `some-bookmark@origin` reuses available content or fetches the missing selected reference into a temporary comparison repository. **Load history from origin** in Revisions/Commits explicitly imports searchable remote history. Network operations use the repository's existing credentials; errors stay in the selector. They do not update active refs, snapshot jj's working copy, or change the checkout. -**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 / Drafts / All** tabs. Open excludes drafts; Drafts contains open draft requests; Closed includes merged requests and closed drafts. 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. @@ -414,9 +414,9 @@ Standard GitHub/GitLab/Gitea hosts and Codeberg are detected from HTTPS or SSH r SSH aliases are resolved with `ssh -G` before provider detection and API authentication, honoring the effective `HostName` and user-specific `Match` settings. SSH ports do not become HTTPS API ports. Git fetches retain the original remote URL, alias, and SSH identity configuration. A failed API load shows its error without also claiming that the search has no matches; the empty-result message appears only after a successful load. -Open and Closed fetch their first pages concurrently, newest-first, and display each page immediately. Subsequent pages load in the background, with the active category dispatched first; loaded rows remain selectable. Pages request 50 entries. GitLab Closed combines separate closed and merged requests (up to 100 entries per batch). All merges the two catalogs without a third fetch stream, deduplicates requests, and sorts by creation time; background updates preserve the selected PR's identity. +Open (including drafts) and Closed fetch their first pages concurrently, newest-first, and display each page immediately. Open and Drafts filter the same catalog, sharing its loading, completion, and retry state without extra requests. Subsequent pages load in the background, with the active category's stream dispatched first; loaded rows remain selectable. Pages request 50 entries. GitLab Closed combines separate closed and merged requests (up to 100 entries per batch). All includes open, draft, closed, and merged requests from the two streams, deduplicates requests, and sorts by creation time; background updates preserve the selected PR's identity. -Search filters loaded entries locally and explicitly indicates partial results while more pages remain. A later-page error retains loaded rows; **Ctrl+r** retries failed categories (the active category, or both from All). Enter selects a loaded row; in an empty failed category it retries loading. Escape cancels outstanding work. Selection stops listing and only then retrieves that PR's historical metadata and preserved head reference, so GitLab detail reads and Gitea ref lookups do not delay the catalog. +Search filters loaded entries locally and explicitly indicates partial results while more pages remain. A later-page error retains loaded rows; **Ctrl+r** retries failed fetch streams (the active category's stream, or both from All). Enter selects a loaded row; in an empty failed category it retries loading. Escape cancels outstanding work. Selection stops listing and only then retrieves that PR's historical metadata and preserved head reference, so GitLab detail reads and Gitea ref lookups do not delay the catalog. Fork heads are fetched through the target repository's PR refs into the existing private comparison store, without changing active refs, jj operations, or checkout files. Closed/merged requests retain historical bases and heads rather than comparing against today's branch tips. If the server no longer retains the required commits, opening the request reports an error instead of displaying a different diff. Review submission and comments are separate, not part of this picker. diff --git a/src/diff.rs b/src/diff.rs index 0120255..04c68f5 100644 --- a/src/diff.rs +++ b/src/diff.rs @@ -24,6 +24,7 @@ pub enum Category { Tags, Open, Closed, + Drafts, All, } impl Category { @@ -36,6 +37,7 @@ impl Category { Self::Tags => "Tags", Self::Open => "Open", Self::Closed => "Closed", + Self::Drafts => "Drafts", Self::All => "All", } } @@ -814,7 +816,12 @@ impl DiffState { pub fn categories(&self) -> &'static [Category] { if self.pull_request_remote.is_some() { - &[Category::Open, Category::Closed, Category::All] + &[ + Category::Open, + Category::Closed, + Category::Drafts, + Category::All, + ] } else { self.catalog .as_ref() @@ -1207,26 +1214,34 @@ impl DiffState { .collect(); } + fn pull_request_streams(&self) -> std::ops::Range { + match self.categories().get(self.category) { + Some(Category::All) => 0..2, + Some(Category::Closed) => 1..2, + _ => 0..1, + } + } + pub(crate) fn pull_request_partial(&self) -> bool { self.pull_request_remote.is_some() && self - .pull_request_categories - .iter() - .enumerate() - .any(|(index, status)| { - (self.category == 2 || self.category == index) && !status.complete - }) + .pull_request_streams() + .any(|index| !self.pull_request_categories[index].complete) } pub(crate) fn pull_request_notice(&self) -> Option { self.pull_request_remote.as_ref()?; + let active = self.categories()[self.category]; let statuses: Vec<_> = self - .pull_request_categories + .categories() .iter() - .enumerate() - .filter(|(index, _)| self.category == 2 || self.category == *index) - .map(|(index, status)| { - let category = [Category::Open, Category::Closed][index]; + .copied() + .filter(|category| { + *category != Category::All && (active == Category::All || *category == active) + }) + .map(|category| { + let status = + &self.pull_request_categories[usize::from(category == Category::Closed)]; let count = self.catalog.as_ref().map_or(0, |catalog| { catalog .entries @@ -1254,8 +1269,9 @@ impl DiffState { } fn retry_pull_requests(&mut self) { - for (index, status) in self.pull_request_categories.iter_mut().enumerate() { - if (self.category == 2 || self.category == index) && status.error.take().is_some() { + for index in self.pull_request_streams() { + let status = &mut self.pull_request_categories[index]; + if status.error.take().is_some() { self.pull_request_retries[index] += 1; self.loading = true; } @@ -1284,7 +1300,10 @@ impl DiffState { if entry.target == RevisionTarget::Working || (entry.category != category && !(category == Category::All - && matches!(entry.category, Category::Open | Category::Closed))) + && matches!( + entry.category, + Category::Open | Category::Closed | Category::Drafts + ))) { continue; } diff --git a/src/forge.rs b/src/forge.rs index 28d3170..a9aaf17 100644 --- a/src/forge.rs +++ b/src/forge.rs @@ -13,6 +13,7 @@ pub(crate) struct PullRequestSummary { pub title: String, pub description: String, pub closed: bool, + pub draft: bool, pub created_at: String, } @@ -694,11 +695,19 @@ fn summary( ) -> Result { let gitlab = provider == Provider::Gitlab; let (closed, merged) = status(provider, value)?; + let draft = match value.get("draft") { + None => false, + Some(value) => value + .as_bool() + .ok_or("Forge API returned invalid draft flag")?, + }; let author = author(provider, value)?; let state = if merged { "merged" } else if closed { "closed" + } else if draft { + "draft" } else { "open" }; @@ -733,6 +742,7 @@ fn summary( _ => format!("{author} · {state} · {size}"), }, closed, + draft, created_at: created_at(value)?, }) } diff --git a/src/main.rs b/src/main.rs index 02da39e..9008423 100644 --- a/src/main.rs +++ b/src/main.rs @@ -105,7 +105,7 @@ Local reviews: :diff.comment anchors exact Visual text, Visual Line rows, or the :diff.review.save PATH exports JSON; :diff.review.open [ID|PATH] selects/imports; :diff.review.new Normal Esc or :pane.close discards unsaved text and dismisses the floating editor Snapshot anchors stay immutable; stale comments lose gutter markers; no publishing -Pull requests: :pull_requests [REMOTE]; GitHub/GitLab/Gitea Open/Closed/All tabs, type to search +Pull requests: :pull_requests [REMOTE]; GitHub/GitLab/Gitea Open/Closed/Drafts/All tabs, type to search Enter opens the immutable PR head against its merge base; API auth uses provider tokens or gh/glab Resolve jj conflicts: :resolve opens the current working-copy conflicts in a dedicated workspace Editable marker-free Draft vs readonly Alternative; :resolve.history shows inputs/base diff --git a/src/repository.rs b/src/repository.rs index c8fcfcd..58fea38 100644 --- a/src/repository.rs +++ b/src/repository.rs @@ -1635,6 +1635,8 @@ impl Repository { description: pull.description, category: if pull.closed { Category::Closed + } else if pull.draft { + Category::Drafts } else { Category::Open }, diff --git a/tests/unit/diff.rs b/tests/unit/diff.rs index af23518..e0c1a17 100644 --- a/tests/unit/diff.rs +++ b/tests/unit/diff.rs @@ -445,7 +445,12 @@ fn pull_request_categories_keep_query_and_select_loaded_rows_before_completion() let mut state = DiffState::pull_requests("origin".into()); assert_eq!( state.categories(), - [Category::Open, Category::Closed, Category::All] + [ + Category::Open, + Category::Closed, + Category::Drafts, + Category::All + ] ); state.take_request(); state.handle(Input::Char('\t')); @@ -464,6 +469,7 @@ fn pull_request_categories_keep_query_and_select_loaded_rows_before_completion() assert!(state.pull_request_partial()); assert_eq!(state.entries().next().unwrap().label, "#2 Fix Ünicode"); state.handle(Input::Char('\t')); + state.handle(Input::Char('\t')); state.apply_result( 1, Ok(pull_page( @@ -498,6 +504,79 @@ fn pull_request_categories_keep_query_and_select_loaded_rows_before_completion() )); } +#[test] +fn pull_request_drafts_filter_independently_and_share_open_loading_and_retry() { + let mut state = DiffState::pull_requests("origin".into()); + state.take_request(); + let mut page = pull_page( + false, + &[ + (1, "Ready", "2024-01-01T00:00:00Z"), + (2, "Draft Ünicode", "2024-01-02T00:00:00Z"), + ], + true, + ); + if let Outcome::PullRequestPage { + result: Ok(page), .. + } = &mut page + { + page.entries[1].0.category = Category::Drafts; + } + state.apply_result(1, Ok(page)); + state.apply_result( + 1, + Ok(pull_page( + true, + &[(3, "Closed", "2024-01-03T00:00:00Z")], + false, + )), + ); + let labels = |state: &DiffState| { + state + .entries() + .map(|entry| entry.label.clone()) + .collect::>() + }; + assert_eq!(labels(&state), ["#1 Ready"]); + state.handle(Input::BackTab); + assert_eq!( + labels(&state), + ["#3 Closed", "#2 Draft Ünicode", "#1 Ready"] + ); + state.handle(Input::BackTab); + assert_eq!(labels(&state), ["#2 Draft Ünicode"]); + assert!(state.pull_request_partial()); + state.apply_result( + 1, + Ok(Outcome::PullRequestPage { + closed: false, + result: Err("open stream unavailable".into()), + }), + ); + assert!( + state + .pull_request_notice() + .unwrap() + .contains("open stream unavailable") + ); + state.handle(Input::Ctrl('r')); + assert_eq!(state.pull_request_retries, [1, 0]); + state.apply_result(1, Ok(pull_page(false, &[], false))); + assert!(!state.pull_request_partial()); + for ch in "Ünicode Alice".chars() { + state.handle(Input::Char(ch)); + } + assert_eq!(labels(&state), ["#2 Draft Ünicode"]); + state.handle(Input::Enter); + assert!(matches!( + state.take_request(), + Some(Operation::ResolvePullRequest(Revision { + target: RevisionTarget::PullRequest { number: 2, .. }, + .. + })) + )); +} + #[test] fn pull_request_all_streams_sort_deduplicate_and_preserve_selection_identity() { let mut state = DiffState::pull_requests("origin".into()); diff --git a/tests/unit/forge.rs b/tests/unit/forge.rs index feb8611..26e73b2 100644 --- a/tests/unit/forge.rs +++ b/tests/unit/forge.rs @@ -7,6 +7,42 @@ fn parse(url: &str, provider: Option<&str>) -> Result { }) } +#[test] +fn draft_summaries_keep_open_closed_and_merged_states_distinct() { + for provider in [Provider::Github, Provider::Gitlab, Provider::Gitea] { + for (closed, merged, expected) in [ + (false, false, "draft"), + (true, false, "closed"), + (true, true, "merged"), + ] { + let mut value = json!({ + "number": 7, "iid": 7, "title": "Unfinished work", "draft": true, + "state": if merged && provider == Provider::Gitlab { "merged" } + else if closed { "closed" } + else if provider == Provider::Gitlab { "opened" } else { "open" }, + "merged": merged, + "merged_at": if merged { Some("2024-01-02T00:00:00Z") } else { None }, + "user": null, "author": null, "created_at": "2024-01-01T00:00:00Z" + }); + let entry = summary(provider, &value, Some((1, 2))).unwrap(); + assert!(entry.draft); + assert_eq!(entry.closed, closed); + assert_eq!( + entry.description, + format!("deleted user · {expected} · +1 -2") + ); + if !closed { + value["draft"] = json!(false); + assert!(!summary(provider, &value, None).unwrap().draft); + value.as_object_mut().unwrap().remove("draft"); + assert!(!summary(provider, &value, None).unwrap().draft); + value["draft"] = json!("true"); + assert!(summary(provider, &value, None).is_err()); + } + } + } +} + #[test] fn pull_request_sizes_include_zero_and_preserve_unknown_statistics() { for (url, provider) in [ diff --git a/tests/unit/tui.rs b/tests/unit/tui.rs index 213245c..5be9279 100644 --- a/tests/unit/tui.rs +++ b/tests/unit/tui.rs @@ -3508,7 +3508,11 @@ fn pull_request_picker_renders_all_rows_without_a_pinned_offset() { }, label: format!("#{index:02} Pull request"), description: "open · main ← topic".into(), - category: Category::Open, + category: if index == 1 { + Category::Drafts + } else { + Category::Open + }, }, format!("2026-01-{:02}T00:00:00Z", 20 - index), ) @@ -3548,6 +3552,11 @@ fn pull_request_picker_renders_all_rows_without_a_pinned_offset() { ); render_diff(&mut terminal, &diff, &mut panels); assert!(screen_text(&terminal).contains("No matching pull requests")); + diff.handle(Input::Char('\t')); + render_diff(&mut terminal, &diff, &mut panels); + let text = screen_text(&terminal); + assert!(text.contains("> #01 Pull request")); + assert!(!text.contains("#00 Pull request") && !text.contains("#19 Pull request")); } }