feat: add persistent pull request sort and filter menus

This commit is contained in:
Valdemar Lorenzen
2026-10-21 13:30:38 +02:00
parent 37b47ebc8a
commit fbcb519e13
22 changed files with 1557 additions and 181 deletions
+3 -2
View File
@@ -5,8 +5,9 @@
- make the symbols displayed for these states configurable.
- [x] chore: check to make sure we're mocking everything in tests, not pinging/fetching real data from real websites/APIs.
- [x] improve: pull requests - sort and filter based on review state:
- pr approved
- review requested
- personal review buckets with multi-select checkboxes
- ranked sorting by review request, update time, approval, and creation time
- persisted settings with explicit config.rb section overrides
- [ ] 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.
+14
View File
@@ -109,6 +109,20 @@ Indicators compare the current editor text (including unsaved edits) against Git
`config.pull_request_review_requested_symbol` and `config.pull_request_approved_symbol` control the markers before PR numbers in the pull-request picker. Defaults are `"🙏"` for an outstanding personal review request and `"👍"` for your active approval, using the API-authenticated account. Requests take precedence over approvals when both apply. Each option accepts a string without control characters; multi-character and wide Unicode symbols are supported. Set either to `""` to hide that state, or both to `""` to remove the marker gutter. PR numbers and metadata stay aligned by terminal-cell width, including rows without a marker. Non-strings and strings containing tabs, newlines, or other control characters reject the configuration.
`config.pull_request_sort` and `config.pull_request_filters` optionally override saved pull-request picker settings, independently. `nil` or an omitted option uses saved preferences, then built-in defaults. A configured section replaces that entire saved section; omitted sort criteria use built-in defaults, not previously saved rules.
```ruby
config.pull_request_sort = {
updated_at: { precedence: 1, direction: :desc },
created_at: { direction: :asc }
}
config.pull_request_filters = [:requested, :approved]
```
Sort fields are `requested`, `updated_at`, `approved`, and `created_at`, in that default precedence order. Default direction is `desc`; timestamp criteria support `asc` for oldest first. `precedence` is `nil` (unnumbered default position) or a unique integer in `1..4`. Filters are any subset of `requested` (Requested from me), `approved` (Approved by me without an outstanding personal request), and `other` (Other / unavailable). `[]` excludes all requests; `{}` resets sorting to built-in defaults. Unknown fields/options, duplicate ranks/filters, and invalid types or directions reject configuration.
Enter Apply in the picker saves global preferences to `$XDG_STATE_HOME/vamp/pull_requests.json`, or `~/.local/state/vamp/pull_requests.json` if XDG state is unset or relative. This also works with `--no-config`. Configuration remains a startup override, not a lock: UI edits work and persist, but explicitly configured sections win again next startup. Save failures report session-only changes. Corrupt, incompatible, or non-regular storage is reported and preserved rather than reset or overwritten.
`config.resolve_history_layout` controls the read-only conflict inputs opened by `:resolve.history`: `"columns"` (default) places left/base/right side by side; `"base_below"` places left/right above a full-width base. Both layouts share aligned rows and synchronized vertical scrolling. Tab/Shift+Tab follows layout order (left/base/right or left/right/base); directional pane actions follow the displayed geometry. Narrow terminals show the focused source only; the base-below layout does the same when there is insufficient height. This setting does not change the editable Resolution view or ordinary diffs. Invalid values reject the configuration.
```ruby
+1 -1
View File
@@ -88,7 +88,7 @@ Clipboard checks use isolated providers rather than reading or overwriting the d
Diff checks cover staged category/filter navigation, stale request cancellation, row alignment, read-only buffer isolation, and rendered panes. Repository integration fixtures exercise Git and Git-backed jj, advertised but unfetched remote branches/bookmarks/tags, imported historical commit/change IDs, conflicts, unusual paths, binary/mode/link/submodule changes, and unchanged active metadata. Remote fixtures are local bare copies; they never push. Git fixture identities and commit dates are fixed.
Pull-request checks cover personal review filters and priority sorting alongside fuzzy queries, category changes, streamed metadata updates, identity-preserving selection, and immediate opening. Live PTY smoke at 120×30 and 44×16 cycles both controls, combines filters with search, restores defaults, and exits cleanly. A disposable curl fixture supplies all forge responses, and the editor and descendants run with networking denied.
Pull-request checks cover ranked sort menus, rank collisions/default slots, timestamp directions, independent personal review flags, checkbox unions, Apply/Cancel, category/query composition, streamed updates, and selection identity. Storage and session checks cover restart persistence, config section overrides, corrupt/incompatible-file preservation, and failed writes with isolated paths. Live PTY smoke at 100×24 and 44×16 exercises checkbox Apply/Cancel, restart restoration, creation-rank movement and ascending direction, and a config filter override. All forge responses come from a disposable curl fixture; the editor and descendants run with networking denied.
Diff-stat checks cover unopened frozen files, additions/deletions/renames, binary/mode changes, CRLF/EOF, refresh, oversized-file laziness, filtering, live edits/undo, and inactive tabs. Live PTY smoke opens a two-file Git Working comparison, observes selected-file `+2 -1` alongside whole-diff `+2 -2` before opening the second file, edits and undoes text, switches to an ordinary tab, and exits with source bytes and HEAD unchanged.
+5 -1
View File
@@ -412,7 +412,11 @@ Immediately before `#number`, **🙏 marks an outstanding personal review reques
A later request for changes or dismissal removes 👍; comment-only and pending reviews do not replace an existing approval. The mark is not the PR's overall approval decision or a guarantee that you reviewed its current head; the forge controls approval resets after new commits. Customize either symbol with [`config.pull_request_review_requested_symbol` and `config.pull_request_approved_symbol`](configuration.md); an empty string hides that symbol. Multi-cell symbols remain aligned, and decorations are not part of searchable PR labels.
**Ctrl+f** cycles the review filter through **All → Requested → Approved**. **Ctrl+s** cycles sorting through **Newest → Requested first → Approved first**. Both use the personal review states above, even when their symbols are hidden; a re-review request belongs to Requested, not Approved. Filters combine with the selected category and fuzzy query. Review priority sorts ahead of fuzzy relevance; within each group, fuzzy ranking is unchanged, or newest creation time first when the query is empty (higher PR number breaks date ties). Sorting and filtering preserve the selected PR when it remains visible, including as pages arrive or metadata changes. Category counts still report all loaded requests, and partial-search notices still apply. These controls only use already-loaded metadata and make no additional API requests.
**Ctrl+s** opens sorting settings in the floating picker. The default comparison order is **Requested from me → Updated at → Approved by me → Created at**, with true statuses and newest timestamps first. Up/Down or the configured selection bindings navigate. Press **1–4** to assign the highlighted criterion an absolute precedence; it moves to that position and stays highlighted. Occupied explicit ranks shift forward (wrapping to a free slot); unnumbered criteria fill the remaining slots in their default relative order. Only explicitly assigned ranks display numbers. **Backspace** clears an explicit rank; **Left/Right** toggles oldest/newest first for timestamp criteria. Comparisons are lexicographic, with higher PR number as the final tie-breaker. Fuzzy text still determines which requests match; these settings determine their order.
**Ctrl+f** opens review filter checkboxes. **Space** toggles the highlighted bucket: **Requested from me**, **Approved by me**, or **Other / unavailable**. Checked buckets combine with OR, then intersect with the category and text query. All checked includes every review state; none checked produces no matches. These are personal states, not the PR's team approval threshold: teammates' outstanding requests do not cancel your approval. A new request for your own review takes precedence over your earlier approval. Unavailable metadata remains Other, not a claim that the PR has never been reviewed.
**Enter** applies either settings view; **Escape** discards its draft and returns to the PR list. Applied settings preserve the selected request where possible and carry across tabs and sessions. Category counts remain loaded-category totals, and partial-search notices still apply. Settings use already-loaded metadata without additional API requests. Explicit [configuration overrides](configuration.md#preferences) win again at startup; the settings view indicates when this applies.
Review-state reads reuse the same API credentials as listing. GitHub matches the current list's [requested reviewers](https://docs.github.com/en/rest/pulls/review-requests#list-requested-reviewers-for-a-pull-request) and filters published [reviews by author and decision state](https://docs.github.com/en/graphql/reference/pulls#pullrequest). GitLab filters approvals by the authenticated approver and reads individual reviewer [interaction states](https://docs.gitlab.com/api/graphql/reference/) in separate bounded, paginated batches; unsupported or denied reviewer-state fields do not discard known approvals or size metadata. Gitea reuses [paginated review reads per PR](https://docs.gitea.com/api/operations/repo-list-pull-reviews/) for both states, so its pages can take longer to load.
+186 -94
View File
@@ -1,5 +1,6 @@
use crate::editor::{BackspaceKind, Input};
use crate::files::{FilePicker, FilePresentation, FileView, fuzzy_match};
use crate::pull_requests::{Direction, Preferences, ReviewFilter, SortField};
use std::{
borrow::Cow,
collections::{BTreeMap, BTreeSet},
@@ -109,37 +110,43 @@ pub enum PullRequestReviewState {
Requested,
}
#[derive(Clone, Copy, Debug, Default, Eq, Hash, PartialEq)]
pub(crate) enum PullRequestReviewScope {
#[default]
All,
Requested,
Approved,
#[derive(Clone, Debug, Default)]
pub struct PullRequestMetadata {
pub created_at: String,
pub updated_at: String,
pub requested: bool,
pub approved: bool,
}
impl PullRequestReviewScope {
pub(crate) fn label(self) -> &'static str {
match self {
Self::All => "All",
Self::Requested => "Requested",
Self::Approved => "Approved",
impl PullRequestMetadata {
fn filter(&self) -> ReviewFilter {
if self.requested {
ReviewFilter::Requested
} else if self.approved {
ReviewFilter::Approved
} else {
ReviewFilter::Other
}
}
}
fn next(self) -> Self {
match self {
Self::All => Self::Requested,
Self::Requested => Self::Approved,
Self::Approved => Self::All,
}
#[derive(Clone, Debug)]
pub(crate) struct PullRequestMenu {
pub sort: bool,
pub selected: usize,
pub draft: Preferences,
}
fn matches(self, state: PullRequestReviewState) -> bool {
match self {
Self::All => true,
Self::Requested => state == PullRequestReviewState::Requested,
Self::Approved => state == PullRequestReviewState::Approved,
}
impl PullRequestMenu {
fn set_rank(&mut self, rank: Option<u8>) {
let field = self.draft.ordered_fields()[self.selected];
self.draft.set_rank(field, rank);
self.selected = self
.draft
.ordered_fields()
.iter()
.position(|&candidate| candidate == field)
.expect("four sort fields");
}
}
@@ -147,7 +154,7 @@ impl PullRequestReviewScope {
pub struct PullRequestPage {
pub kind: RepoKind,
pub remote: String,
pub entries: Vec<(Revision, String, PullRequestReviewState)>,
pub entries: Vec<(Revision, PullRequestMetadata)>,
pub has_more: bool,
}
@@ -614,10 +621,11 @@ pub struct DiffState {
pub catalog: Option<Catalog>,
pub pull_request_remote: Option<String>,
pull_request_categories: [PullRequestCategory; 2],
pull_request_order: BTreeMap<u64, (String, usize, PullRequestReviewState)>,
pull_request_order: BTreeMap<u64, (PullRequestMetadata, usize)>,
pub(crate) pull_request_retries: [u64; 2],
pub(crate) pull_request_filter: PullRequestReviewScope,
pub(crate) pull_request_priority: PullRequestReviewScope,
pub(crate) pull_request_preferences: Preferences,
pub(crate) pull_request_menu: Option<PullRequestMenu>,
pub(crate) pull_request_preferences_changed: bool,
pub phase: Phase,
pub word_wrap: bool,
pub view_mode: ViewMode,
@@ -682,8 +690,9 @@ impl DiffState {
pull_request_categories: Default::default(),
pull_request_order: BTreeMap::new(),
pull_request_retries: [0; 2],
pull_request_filter: PullRequestReviewScope::All,
pull_request_priority: PullRequestReviewScope::All,
pull_request_preferences: Preferences::default(),
pull_request_menu: None,
pull_request_preferences_changed: false,
phase: Phase::Before,
word_wrap: false,
view_mode: ViewMode::Split,
@@ -980,37 +989,19 @@ impl DiffState {
entries: Vec::new(),
notice: Some(format!("Pull requests from {}", page.remote)),
});
for (revision, created_at, review_state) in page.entries {
for (revision, metadata) in page.entries {
let RevisionTarget::PullRequest { number, .. } = &revision.target else {
continue;
};
if let Some((date, index, state)) = self.pull_request_order.get_mut(number)
{
*date = created_at;
*state = review_state;
if let Some((details, index)) = self.pull_request_order.get_mut(number) {
*details = metadata;
catalog.entries[*index] = revision;
} else {
self.pull_request_order
.insert(*number, (created_at, catalog.entries.len(), review_state));
.insert(*number, (metadata, catalog.entries.len()));
catalog.entries.push(revision);
}
}
let number = |revision: &Revision| match revision.target {
RevisionTarget::PullRequest { number, .. } => number,
_ => unreachable!("pull request catalog"),
};
catalog.entries.sort_unstable_by(|a, b| {
self.pull_request_order[&number(b)]
.0
.cmp(&self.pull_request_order[&number(a)].0)
.then_with(|| number(b).cmp(&number(a)))
});
for (index, revision) in catalog.entries.iter().enumerate() {
self.pull_request_order
.get_mut(&number(revision))
.expect("known pull request")
.1 = index;
}
self.prepare_search();
self.refilter();
if let Some(target) = selected {
@@ -1271,12 +1262,35 @@ impl DiffState {
}
pub(crate) fn pull_request_review_state(&self, revision: &Revision) -> PullRequestReviewState {
if let RevisionTarget::PullRequest { number, .. } = revision.target {
self.pull_request_order
.get(&number)
.map_or(PullRequestReviewState::None, |entry| entry.2)
} else {
PullRequestReviewState::None
match self
.pull_request_metadata(revision)
.map(PullRequestMetadata::filter)
{
Some(ReviewFilter::Requested) => PullRequestReviewState::Requested,
Some(ReviewFilter::Approved) => PullRequestReviewState::Approved,
_ => PullRequestReviewState::None,
}
}
fn pull_request_metadata(&self, revision: &Revision) -> Option<&PullRequestMetadata> {
let RevisionTarget::PullRequest { number, .. } = revision.target else {
return None;
};
self.pull_request_order.get(&number).map(|entry| &entry.0)
}
pub(crate) fn set_pull_request_preferences(&mut self, preferences: Preferences) {
if self.pull_request_preferences == preferences {
return;
}
let selected = self
.entries()
.nth(self.selected)
.map(|entry| entry.target.clone());
self.pull_request_preferences = preferences;
self.refilter();
if let Some(target) = selected {
self.select_target(&target);
}
}
@@ -1363,9 +1377,11 @@ impl DiffState {
Category::Open | Category::Closed | Category::Drafts
)))
|| (self.pull_request_remote.is_some()
&& !self
.pull_request_filter
.matches(self.pull_request_review_state(entry)))
&& !self.pull_request_preferences.filters.contains(
&self
.pull_request_metadata(entry)
.map_or(ReviewFilter::Other, PullRequestMetadata::filter),
))
{
continue;
}
@@ -1401,26 +1417,50 @@ impl DiffState {
self.search_matches.push((score, index));
}
}
let priority = |index: usize| {
if self.pull_request_priority == PullRequestReviewScope::All {
return false;
}
let RevisionTarget::PullRequest { number, .. } = catalog.entries[index].target
else {
return false;
};
!self
.pull_request_priority
.matches(self.pull_request_order[&number].2)
};
if self.pull_request_priority != PullRequestReviewScope::All {
self.visible
.sort_unstable_by_key(|&index| (priority(index), index));
}
if self.pull_request_remote.is_none() {
self.search_matches
.sort_unstable_by_key(|&(score, index)| (priority(index), score, index));
.sort_unstable_by_key(|&(score, index)| (score, index));
}
self.visible
.extend(self.search_matches.iter().map(|(_, index)| *index));
if self.pull_request_remote.is_some() {
let fields = self.pull_request_preferences.ordered_fields().map(|field| {
(
field,
self.pull_request_preferences
.sort
.get(&field)
.map_or(Direction::Desc, |rule| rule.direction),
)
});
self.visible.sort_unstable_by(|&a, &b| {
let number = |index: usize| match catalog.entries[index].target {
RevisionTarget::PullRequest { number, .. } => number,
_ => unreachable!("pull request catalog"),
};
let a_number = number(a);
let b_number = number(b);
let a = &self.pull_request_order[&a_number].0;
let b = &self.pull_request_order[&b_number].0;
for (field, direction) in fields {
let order = match field {
SortField::Requested => b.requested.cmp(&a.requested),
SortField::UpdatedAt => b.updated_at.cmp(&a.updated_at),
SortField::Approved => b.approved.cmp(&a.approved),
SortField::CreatedAt => b.created_at.cmp(&a.created_at),
};
let order = if direction == Direction::Asc {
order.reverse()
} else {
order
};
if !order.is_eq() {
return order;
}
}
b_number.cmp(&a_number)
});
}
}
self.selected = 0;
}
@@ -1433,6 +1473,15 @@ impl DiffState {
}
pub(crate) fn navigate(&mut self, forward: bool) {
if let Some(menu) = &mut self.pull_request_menu {
let count = if menu.sort { 4 } else { 3 };
menu.selected = if forward {
(menu.selected + 1) % count
} else {
(menu.selected + count - 1) % count
};
return;
}
if self.phase == Phase::View {
if self.focus == Focus::Files {
if let Some(files) = &mut self.files {
@@ -1463,6 +1512,20 @@ impl DiffState {
/// Handles selector filtering and comparison navigation; the editor owns selector closing.
pub(crate) fn handle(&mut self, input: Input) {
if self.pull_request_remote.is_some() && self.phase != Phase::View {
if matches!(input, Input::Ctrl('s' | 'f')) {
self.pull_request_menu = Some(PullRequestMenu {
sort: input == Input::Ctrl('s'),
selected: 0,
draft: self.pull_request_preferences.clone(),
});
return;
}
if self.pull_request_menu.is_some() {
self.handle_pull_request_menu(input);
return;
}
}
if input == Input::Escape {
if self.phase == Phase::View
&& self.focus == Focus::Files
@@ -1506,22 +1569,6 @@ impl DiffState {
self.choose()
}
Input::Ctrl('r') if self.pull_request_remote.is_some() => self.retry_pull_requests(),
Input::Ctrl('f' | 's') if self.pull_request_remote.is_some() => {
let selected = self
.entries()
.nth(self.selected)
.map(|entry| entry.target.clone());
let scope = if input == Input::Ctrl('f') {
&mut self.pull_request_filter
} else {
&mut self.pull_request_priority
};
*scope = scope.next();
self.refilter();
if let Some(target) = selected {
self.select_target(&target);
}
}
input if BackspaceKind::from_input(&input).is_some() => {
if BackspaceKind::from_input(&input)
.expect("guard checked backspace input")
@@ -1542,6 +1589,51 @@ impl DiffState {
}
}
fn handle_pull_request_menu(&mut self, input: Input) {
let menu = self.pull_request_menu.as_mut().expect("active menu");
match input {
Input::Escape => self.pull_request_menu = None,
Input::Enter => {
let preferences = self.pull_request_menu.take().expect("active menu").draft;
self.set_pull_request_preferences(preferences);
self.pull_request_preferences_changed = true;
}
Input::Up | Input::Ctrl('k') => self.navigate(false),
Input::Down | Input::Ctrl('j') => self.navigate(true),
Input::Home => menu.selected = 0,
Input::End => menu.selected = if menu.sort { 3 } else { 2 },
Input::Char('1'..='4') if menu.sort => {
let Input::Char(digit) = input else {
unreachable!()
};
menu.set_rank(Some(digit as u8 - b'0'));
}
input if menu.sort && BackspaceKind::from_input(&input).is_some() => {
menu.set_rank(None);
}
Input::Left | Input::Right if menu.sort => {
let field = menu.draft.ordered_fields()[menu.selected];
if field.is_date() {
let direction = &mut menu.draft.sort.entry(field).or_default().direction;
*direction = if *direction == Direction::Desc {
Direction::Asc
} else {
Direction::Desc
};
}
}
Input::Char(' ') if !menu.sort => {
let filter = ReviewFilter::ALL[menu.selected];
if menu.draft.filters.contains(&filter) {
menu.draft.filters.retain(|item| *item != filter);
} else {
menu.draft.filters.push(filter);
}
}
_ => {}
}
}
fn choose(&mut self) {
let revision = self.entries().nth(self.selected).cloned();
let Some(revision) = revision else {
+49 -1
View File
@@ -2286,6 +2286,9 @@ pub struct Editor {
diff_display_options: (ViewMode, SplitAxis),
pub(crate) pull_request_review_requested_symbol: String,
pub(crate) pull_request_approved_symbol: String,
pub(crate) pull_request_preferences: crate::pull_requests::Preferences,
pub(crate) pull_request_preferences_changed: bool,
pub(crate) pull_request_configured: [bool; 2],
next_diff_request: u64,
outstanding_diff_request: Option<u64>,
pending_lsp_navigation: Option<PendingLspNavigation>,
@@ -2428,6 +2431,9 @@ impl Editor {
diff_display_options: (ViewMode::Split, SplitAxis::Vertical),
pull_request_review_requested_symbol: "🙏".into(),
pull_request_approved_symbol: "👍".into(),
pull_request_preferences: Default::default(),
pull_request_preferences_changed: false,
pull_request_configured: [false; 2],
next_diff_request: 0,
outstanding_diff_request: None,
pending_lsp_navigation: None,
@@ -5318,6 +5324,46 @@ impl Editor {
let phase = self.picker_diff().expect("active diff").phase;
let files_focused =
phase == Phase::View && self.picker_diff().unwrap().focus == Focus::Files;
if phase != Phase::View
&& self.picker_diff().unwrap().pull_request_remote.is_some()
&& (self.picker_diff().unwrap().pull_request_menu.is_some()
|| matches!(input, Input::Ctrl('s' | 'f')))
{
if !matches!(
input,
Input::Char('0'..='9' | ' ')
| Input::Ctrl('s' | 'f')
| Input::Escape
| Input::Enter
| Input::Left
| Input::Right
) && BackspaceKind::from_input(&input).is_none()
&& self.handle_selection_binding(&input)
{
return None;
}
let diff = self.picker_diff_mut().unwrap();
diff.handle(input);
if std::mem::take(&mut diff.pull_request_preferences_changed) {
let preferences = diff.pull_request_preferences.clone();
self.pull_request_preferences = preferences.clone();
self.pull_request_preferences_changed = true;
for diff in self
.diff
.iter_mut()
.chain(self.parked_diff.iter_mut())
.chain(
self.tabs
.values_mut()
.flat_map(|tab| tab.diff.iter_mut().chain(tab.parked_diff.iter_mut())),
)
.filter(|diff| diff.pull_request_remote.is_some())
{
diff.set_pull_request_preferences(preferences.clone());
}
}
return None;
}
if phase != Phase::View && input == Input::Escape {
return self.execute_action(Action::DiffClose.entry(), "");
}
@@ -10172,7 +10218,9 @@ impl Editor {
if args.split_whitespace().count() > 1 {
self.set_message("Usage: pull_requests [remote]");
} else {
self.open_diff(DiffState::pull_requests(args.into()));
let mut diff = DiffState::pull_requests(args.into());
diff.set_pull_request_preferences(self.pull_request_preferences.clone());
self.open_diff(diff);
}
} else if args.is_empty() {
self.open_diff(DiffState::new());
+6 -4
View File
@@ -17,6 +17,7 @@ pub(crate) struct PullRequestSummary {
pub approved: bool,
pub review_requested: bool,
pub created_at: String,
pub updated_at: String,
}
struct PullRequestMetadata {
@@ -908,9 +909,9 @@ fn gitea_head(value: &Value, id: u64) -> Result<String, String> {
sha(object, "sha")
}
fn created_at(value: &Value) -> Result<String, String> {
let invalid = || "Forge API returned invalid created_at".to_owned();
let text = field(value, "created_at")?;
fn timestamp(value: &Value, key: &str) -> Result<String, String> {
let invalid = || format!("Forge API returned invalid {key}");
let text = field(value, key)?;
let digits = |text: &str| -> Option<i32> {
(!text.is_empty() && text.bytes().all(|b| b.is_ascii_digit()))
.then(|| text.parse().ok())
@@ -1055,7 +1056,8 @@ fn summary(
draft,
approved: false,
review_requested: false,
created_at: created_at(value)?,
created_at: timestamp(value, "created_at")?,
updated_at: timestamp(value, "updated_at")?,
})
}
+2
View File
@@ -12,6 +12,8 @@ mod lsp_config;
mod markdown;
pub mod panes;
pub mod plugin;
mod pull_request_preferences;
mod pull_requests;
pub mod repository;
pub mod resolve;
pub mod review;
+3 -1
View File
@@ -108,7 +108,9 @@ Local reviews: :diff.comment anchors exact Visual text, Visual Line rows, or the
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
🙏 marks a personal review request; 👍 marks your active approval (API account)
Ctrl+f filters All/Requested/Approved; Ctrl+s sorts Newest/Requested first/Approved first
Ctrl+s sort menu: 1-4 precedence, Backspace clear, Left/Right date direction
Ctrl+f filter menu: Space includes/excludes personal review states; Enter applies, Esc cancels
Applied choices persist; config.pull_request_sort / config.pull_request_filters override at startup
Symbols: config.pull_request_review_requested_symbol / config.pull_request_approved_symbol
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
+151
View File
@@ -0,0 +1,151 @@
use crate::pull_requests::Preferences;
use serde::{Deserialize, Serialize};
use std::{
ffi::OsStr,
fs::{self, OpenOptions},
io::{self, Write},
path::{Path, PathBuf},
sync::atomic::{AtomicU64, Ordering},
};
static NEXT_TEMP: AtomicU64 = AtomicU64::new(1);
#[derive(Serialize, Deserialize)]
#[serde(deny_unknown_fields)]
struct StoredPreferences<T> {
version: u8,
preferences: T,
}
pub(crate) fn default_path() -> Result<PathBuf, String> {
path_from(
std::env::var_os("XDG_STATE_HOME").as_deref(),
std::env::var_os("HOME").as_deref(),
)
}
fn path_from(xdg: Option<&OsStr>, home: Option<&OsStr>) -> Result<PathBuf, String> {
if let Some(path) = xdg.map(Path::new).filter(|path| path.is_absolute()) {
return Ok(path.join("vamp/pull_requests.json"));
}
home.map(Path::new)
.filter(|path| path.is_absolute())
.map(|path| path.join(".local/state/vamp/pull_requests.json"))
.ok_or_else(|| {
"Cannot locate pull request preferences: HOME must be an absolute directory".into()
})
}
fn read_preferences(path: &Path) -> Result<Option<Preferences>, String> {
match fs::symlink_metadata(path) {
Err(error) if error.kind() == io::ErrorKind::NotFound => return Ok(None),
Err(error) => return Err(error.to_string()),
Ok(metadata) if !metadata.is_file() => {
return Err("preferences must be a regular file (not a symlink)".into());
}
Ok(_) => {}
}
let bytes = fs::read(path).map_err(|error| error.to_string())?;
let stored: StoredPreferences<Preferences> =
serde_json::from_slice(&bytes).map_err(|error| error.to_string())?;
if stored.version != 1 {
return Err(format!(
"unsupported preferences version {}",
stored.version
));
}
stored.preferences.validate()?;
Ok(Some(stored.preferences))
}
pub(crate) struct Store {
path: Option<PathBuf>,
blocked: Option<String>,
}
impl Store {
pub(crate) fn load(path: Result<PathBuf, String>) -> (Self, Preferences, Option<String>) {
let result = match &path {
Ok(path) => read_preferences(path),
Err(error) => Err(error.clone()),
};
let (preferences, blocked) = match result {
Ok(preferences) => (preferences.unwrap_or_default(), None),
Err(error) => (Preferences::default(), Some(error)),
};
let store = Self {
path: path.ok(),
blocked,
};
let notice = store.blocked.as_ref().map(|error| store.notice(error));
(store, preferences, notice)
}
pub(crate) fn disabled() -> Self {
Self {
path: None,
blocked: None,
}
}
fn notice(&self, error: &str) -> String {
let location = self.path.as_ref().map_or_else(
|| "Pull request preferences".to_owned(),
|path| format!("Pull request preferences ({})", path.display()),
);
format!("{location}: {error}. Changes are session-only; existing data was preserved.")
}
pub(crate) fn save(&self, preferences: &Preferences) -> Result<(), String> {
self.save_inner(preferences)
.map_err(|error| self.notice(&error))
}
fn save_inner(&self, preferences: &Preferences) -> Result<(), String> {
if let Some(error) = &self.blocked {
return Err(error.clone());
}
preferences.validate()?;
let Some(path) = &self.path else {
return Ok(());
};
// Refuse to overwrite data that became corrupt or incompatible after startup.
read_preferences(path)?;
let bytes = serde_json::to_vec_pretty(&StoredPreferences {
version: 1,
preferences,
})
.map_err(|error| error.to_string())?;
let parent = path.parent().ok_or("preferences path has no parent")?;
fs::create_dir_all(parent).map_err(|error| error.to_string())?;
let (temporary, mut file) = loop {
let id = NEXT_TEMP.fetch_add(1, Ordering::Relaxed);
let temporary = parent.join(format!(".pull-requests-{}-{id}.tmp", std::process::id()));
let mut options = OpenOptions::new();
options.write(true).create_new(true);
#[cfg(unix)]
{
use std::os::unix::fs::OpenOptionsExt;
options.mode(0o600);
}
match options.open(&temporary) {
Ok(file) => break (temporary, file),
Err(error) if error.kind() == io::ErrorKind::AlreadyExists => continue,
Err(error) => return Err(error.to_string()),
}
};
let result = (|| {
file.write_all(&bytes).and_then(|_| file.sync_all())?;
drop(file);
read_preferences(path).map_err(io::Error::other)?;
// ponytail: optimistic validation; OS compare-and-swap if concurrent writers matter.
fs::rename(&temporary, path)
})();
let _ = fs::remove_file(temporary);
result.map_err(|error| error.to_string())
}
}
#[cfg(test)]
#[path = "../tests/unit/pull_request_preferences.rs"]
mod tests;
+144
View File
@@ -0,0 +1,144 @@
use serde::{Deserialize, Serialize};
use std::collections::BTreeMap;
#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
#[serde(default, deny_unknown_fields)]
pub(crate) struct Preferences {
pub sort: SortSettings,
pub filters: Vec<ReviewFilter>,
}
impl Default for Preferences {
fn default() -> Self {
Self {
sort: BTreeMap::new(),
filters: ReviewFilter::ALL.to_vec(),
}
}
}
pub(crate) type SortSettings = BTreeMap<SortField, SortRule>;
#[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize, Deserialize)]
#[serde(rename_all = "snake_case")]
pub(crate) enum SortField {
Requested,
UpdatedAt,
Approved,
CreatedAt,
}
impl SortField {
pub const ALL: [Self; 4] = [
Self::Requested,
Self::UpdatedAt,
Self::Approved,
Self::CreatedAt,
];
pub fn label(self) -> &'static str {
match self {
Self::Requested => "Requested from me",
Self::UpdatedAt => "Updated at",
Self::Approved => "Approved by me",
Self::CreatedAt => "Created at",
}
}
pub fn is_date(self) -> bool {
matches!(self, Self::UpdatedAt | Self::CreatedAt)
}
}
#[derive(Clone, Copy, Debug, Default, Eq, Hash, PartialEq, Serialize, Deserialize)]
#[serde(default, deny_unknown_fields)]
pub(crate) struct SortRule {
pub precedence: Option<u8>,
pub direction: Direction,
}
#[derive(Clone, Copy, Debug, Default, Eq, Hash, PartialEq, Serialize, Deserialize)]
#[serde(rename_all = "snake_case")]
pub(crate) enum Direction {
Asc,
#[default]
Desc,
}
#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq, Serialize, Deserialize)]
#[serde(rename_all = "snake_case")]
pub(crate) enum ReviewFilter {
Requested,
Approved,
Other,
}
impl ReviewFilter {
pub const ALL: [Self; 3] = [Self::Requested, Self::Approved, Self::Other];
pub fn label(self) -> &'static str {
match self {
Self::Requested => "Requested from me",
Self::Approved => "Approved by me",
Self::Other => "Other / unavailable",
}
}
}
impl Preferences {
pub fn validate(&self) -> Result<(), String> {
let mut ranks = [false; 4];
for rule in self.sort.values() {
if let Some(rank) = rule.precedence {
if !(1..=4).contains(&rank) {
return Err("Sort precedence must be between 1 and 4".into());
}
if std::mem::replace(&mut ranks[usize::from(rank - 1)], true) {
return Err("Sort precedence must be unique".into());
}
}
}
for (index, filter) in self.filters.iter().enumerate() {
if self.filters[..index].contains(filter) {
return Err("Review filters must be unique".into());
}
}
Ok(())
}
pub fn ordered_fields(&self) -> [SortField; 4] {
let mut slots = [None; 4];
for (&field, rule) in &self.sort {
if let Some(rank) = rule.precedence {
slots[usize::from(rank - 1)] = Some(field);
}
}
let mut unranked = SortField::ALL.into_iter().filter(|field| {
self.sort
.get(field)
.is_none_or(|rule| rule.precedence.is_none())
});
slots.map(|field| field.unwrap_or_else(|| unranked.next().expect("four sort fields")))
}
pub fn set_rank(&mut self, field: SortField, rank: Option<u8>) {
self.sort.entry(field).or_default().precedence = None;
let Some(mut rank) = rank else {
return;
};
let mut field = field;
loop {
let displaced = self
.sort
.iter()
.find_map(|(&other, rule)| (rule.precedence == Some(rank)).then_some(other));
self.sort.entry(field).or_default().precedence = Some(rank);
let Some(other) = displaced else {
break;
};
self.sort.get_mut(&other).expect("ranked field").precedence = None;
field = other;
rank = rank % 4 + 1;
}
}
}
+6 -8
View File
@@ -1,7 +1,7 @@
//! Read-only active-repository discovery and isolated revision comparison.
use crate::diff::{
BlameRevision, Catalog, Category, ChangedFile, Comparison, ComparisonMode, FileStatus,
PullRequestPage, PullRequestReviewState, RepoKind, Revision, RevisionTarget, line_hunks,
PullRequestMetadata, PullRequestPage, RepoKind, Revision, RevisionTarget, line_hunks,
};
use std::{
collections::{BTreeMap, BTreeSet},
@@ -1641,13 +1641,11 @@ impl Repository {
Category::Open
},
},
pull.created_at,
if pull.review_requested {
PullRequestReviewState::Requested
} else if pull.approved {
PullRequestReviewState::Approved
} else {
PullRequestReviewState::None
PullRequestMetadata {
created_at: pull.created_at,
updated_at: pull.updated_at,
requested: pull.review_requested,
approved: pull.approved,
},
)
})
+22
View File
@@ -1,6 +1,7 @@
use crate::chrome::{ChromeLine, PopupConfig};
use crate::editor::{Context, Effect, Feature};
use crate::plugin::{DocumentEvent, PluginEvent, PluginUpdate, UpdateEffect};
use crate::pull_requests::{Preferences, ReviewFilter, SortSettings};
use serde::{Deserialize, Serialize};
use std::io::{self, BufRead, BufReader, Write};
use std::num::{NonZeroU8, NonZeroUsize};
@@ -60,6 +61,8 @@ pub struct Config {
pub diff_follow_working_copy: bool,
pub pull_request_review_requested_symbol: String,
pub pull_request_approved_symbol: String,
pub(crate) pull_request_sort: Option<SortSettings>,
pub(crate) pull_request_filters: Option<Vec<ReviewFilter>>,
pub resolve_history_layout: crate::editor::ResolverLayout,
}
@@ -116,6 +119,8 @@ impl Default for Config {
diff_follow_working_copy: true,
pull_request_review_requested_symbol: "🙏".into(),
pull_request_approved_symbol: "👍".into(),
pull_request_sort: None,
pull_request_filters: None,
resolve_history_layout: crate::editor::ResolverLayout::Columns,
}
}
@@ -159,6 +164,8 @@ pub fn load_config(path: &Path) -> Result<Config, String> {
diff_follow_working_copy,
pull_request_review_requested_symbol,
pull_request_approved_symbol,
pull_request_sort,
pull_request_filters,
resolve_history_layout,
..
} = *response;
@@ -203,6 +210,17 @@ pub fn load_config(path: &Path) -> Result<Config, String> {
config.diff_follow_working_copy = diff_follow_working_copy;
config.pull_request_review_requested_symbol = pull_request_review_requested_symbol;
config.pull_request_approved_symbol = pull_request_approved_symbol;
let sort_configured = pull_request_sort.is_some();
let filters_configured = pull_request_filters.is_some();
let preferences = Preferences {
sort: pull_request_sort.unwrap_or_default(),
filters: pull_request_filters.unwrap_or_default(),
};
preferences
.validate()
.map_err(|error| format!("Ruby config: {error}"))?;
config.pull_request_sort = sort_configured.then_some(preferences.sort);
config.pull_request_filters = filters_configured.then_some(preferences.filters);
config.resolve_history_layout = resolve_history_layout;
Ok(config)
}
@@ -568,6 +586,10 @@ struct ConfigFrame {
pull_request_review_requested_symbol: String,
#[serde(deserialize_with = "pull_request_symbol")]
pull_request_approved_symbol: String,
#[serde(default)]
pull_request_sort: Option<SortSettings>,
#[serde(default)]
pull_request_filters: Option<Vec<ReviewFilter>>,
resolve_history_layout: crate::editor::ResolverLayout,
}
+57
View File
@@ -159,6 +159,8 @@ module Vamp
@diff_follow_working_copy = true
@pull_request_review_requested_symbol = "🙏"
@pull_request_approved_symbol = "👍"
@pull_request_sort = nil
@pull_request_filters = nil
@resolve_history_layout = "columns"
end
@@ -341,6 +343,60 @@ module Vamp
@pull_request_approved_symbol = pull_request_symbol!(symbol, "pull_request_approved_symbol")
end
# @param fields [Hash{String, Symbol => Hash}, nil] explicit ranks and sort directions
# @return [Hash, nil]
def pull_request_sort=(fields)
return @pull_request_sort = nil if fields.nil?
raise TypeError, "pull_request_sort must be a Hash or nil" unless fields.is_a?(Hash)
ranks = []
normalized = Vamp.chrome_hash!(fields).to_h do |field, options|
unless %w[requested updated_at approved created_at].include?(field)
raise ArgumentError, "unknown pull_request_sort field: #{field}"
end
raise TypeError, "pull_request_sort #{field} must be a Hash" unless options.is_a?(Hash)
options = Vamp.chrome_hash!(options)
unknown = options.keys - %w[precedence direction]
raise ArgumentError, "unknown pull_request_sort option: #{unknown.first}" unless unknown.empty?
rank = options["precedence"]
unless rank.nil? || (rank.is_a?(Integer) && rank.between?(1, 4))
raise ArgumentError, "pull_request_sort precedence must be between 1 and 4"
end
raise ArgumentError, "duplicate pull_request_sort precedence: #{rank}" if rank && ranks.include?(rank)
ranks << rank if rank
if options.key?("direction")
direction = options["direction"]
direction = direction.to_s if direction.is_a?(Symbol)
unless %w[asc desc].include?(direction)
raise ArgumentError, "pull_request_sort direction must be asc or desc"
end
options["direction"] = direction
end
[field, options]
end
@pull_request_sort = normalized
end
# @param filters [Array<String, Symbol>, nil] selected personal review buckets
# @return [Array<String>, nil]
def pull_request_filters=(filters)
return @pull_request_filters = nil if filters.nil?
raise TypeError, "pull_request_filters must be an Array or nil" unless filters.is_a?(Array)
normalized = filters.map do |filter|
filter = filter.to_s if filter.is_a?(Symbol)
unless %w[requested approved other].include?(filter)
raise ArgumentError, "unknown pull_request_filters value: #{filter.inspect}"
end
filter
end
unless normalized.uniq.length == normalized.length
raise ArgumentError, "duplicate pull_request_filters value"
end
@pull_request_filters = normalized
end
# @param layout [String] "columns" or "base_below"
# @return [String]
def resolve_history_layout=(layout)
@@ -398,6 +454,7 @@ module Vamp
"diff_follow_working_copy" => @diff_follow_working_copy,
"pull_request_review_requested_symbol" => @pull_request_review_requested_symbol,
"pull_request_approved_symbol" => @pull_request_approved_symbol,
"pull_request_sort" => @pull_request_sort, "pull_request_filters" => @pull_request_filters,
"resolve_history_layout" => @resolve_history_layout }
end
+54
View File
@@ -450,6 +450,7 @@ struct ShellCommand {
/// the editor, but cannot independently mutate its plugin registry.
pub struct EditorSession {
editor: Editor,
pull_request_preferences_store: crate::pull_request_preferences::Store,
plugins: BTreeMap<String, RubyPlugin>,
documents: BTreeMap<usize, TrackedDocument>,
requests: BTreeMap<String, AsyncRequests>,
@@ -488,6 +489,20 @@ impl EditorSession {
file: Option<PathBuf>,
capabilities: Vec<String>,
config: Option<&Path>,
) -> io::Result<Self> {
let path = if cfg!(test) {
None
} else {
Some(crate::pull_request_preferences::default_path())
};
Self::open_with_preferences_path(file, capabilities, config, path)
}
pub(crate) fn open_with_preferences_path(
file: Option<PathBuf>,
capabilities: Vec<String>,
config: Option<&Path>,
preferences_path: Option<Result<PathBuf, String>>,
) -> io::Result<Self> {
let mut editor = Editor::open(file, capabilities)?;
let mut notices = Vec::new();
@@ -501,6 +516,28 @@ impl EditorSession {
}),
None => Config::default(),
};
let (pull_request_preferences_store, mut preferences, notice) = match preferences_path {
Some(path) => crate::pull_request_preferences::Store::load(path),
None => (
crate::pull_request_preferences::Store::disabled(),
crate::pull_requests::Preferences::default(),
None,
),
};
if let Some(notice) = notice {
notices.push(notice);
}
editor.pull_request_configured = [
config.pull_request_sort.is_some(),
config.pull_request_filters.is_some(),
];
if let Some(sort) = config.pull_request_sort {
preferences.sort = sort;
}
if let Some(filters) = config.pull_request_filters {
preferences.filters = filters;
}
editor.pull_request_preferences = preferences;
editor.set_review_directory(match config.review_directory {
Some(directory) => directory,
None => crate::review::default_directory().map_err(io::Error::other)?,
@@ -580,6 +617,7 @@ impl EditorSession {
}
let mut session = Self {
editor,
pull_request_preferences_store,
plugins,
documents: BTreeMap::new(),
requests: BTreeMap::new(),
@@ -665,6 +703,7 @@ impl EditorSession {
}
self.start_diff_request();
self.start_resolver_requests();
self.save_pull_request_preferences();
}
pub fn execute(&mut self, command: &str) {
@@ -677,6 +716,20 @@ impl EditorSession {
}
self.start_diff_request();
self.start_resolver_requests();
self.save_pull_request_preferences();
}
fn save_pull_request_preferences(&mut self) -> bool {
if !std::mem::take(&mut self.editor.pull_request_preferences_changed) {
return false;
}
if let Err(error) = self
.pull_request_preferences_store
.save(&self.editor.pull_request_preferences)
{
self.editor.set_message(error);
}
true
}
fn start_resolver_requests(&mut self) {
@@ -1850,6 +1903,7 @@ impl EditorSession {
fn poll_at(&mut self, now: Instant) -> bool {
let mut changed = self.refresh_files_at(now);
changed |= self.save_pull_request_preferences();
changed |= self.poll_gutter_at(now);
changed |= self.synchronize_documents();
changed |= self.poll_battery();
+95 -8
View File
@@ -3367,12 +3367,79 @@ fn draw_diff(
} else {
"After"
};
let title = if pull_requests {
let title = if let Some(menu) = &diff.pull_request_menu {
if menu.sort {
" Sort pull requests ".into()
} else {
" Filter pull requests ".into()
}
} else if pull_requests {
" Pull requests ".into()
} else {
format!(" Diff · Choose {stage} · {} ", diff.comparison_mode.label())
};
let content = diff_panel(frame, panel, &title, "", "", true);
if let Some(menu) = &diff.pull_request_menu {
panels.picker_scroll.revisions = None;
let configured = editor
.is_some_and(|editor| editor.pull_request_configured[usize::from(!menu.sort)]);
let mut lines = vec![Line::from("↑/↓ or Ctrl+k/j to select")];
if configured {
lines.push(Line::styled(
"Configured override on next startup",
Style::default().fg(Color::Yellow),
));
}
let count = if menu.sort { 4 } else { 3 };
let fields = menu.draft.ordered_fields();
for index in 0..count {
let text = if menu.sort {
let field = fields[index];
let rule = menu.draft.sort.get(&field).copied().unwrap_or_default();
let rank = rule.precedence.map_or("–".into(), |rank| rank.to_string());
let direction = if !field.is_date() {
""
} else if rule.direction == crate::pull_requests::Direction::Desc {
" · newest first"
} else {
" · oldest first"
};
format!("[{rank}] {}{direction}", field.label())
} else {
let filter = crate::pull_requests::ReviewFilter::ALL[index];
format!(
"[{}] {}",
if menu.draft.filters.contains(&filter) {
"x"
} else {
" "
},
filter.label()
)
};
lines.push(Line::styled(
format!("{} {text}", if index == menu.selected { ">" } else { " " }),
if index == menu.selected {
Style::default()
.bg(Color::Blue)
.add_modifier(Modifier::BOLD)
} else {
Style::default()
},
));
}
lines.push(Line::from(if menu.sort {
"1–4 rank · Backspace clear rank"
} else {
"Space toggle · checked buckets OR"
}));
if menu.sort {
lines.push(Line::from("←/→ date direction · unranked default"));
}
lines.push(Line::from("Enter Apply · Esc Cancel · Ctrl+s/f switch"));
frame.render_widget(Paragraph::new(lines), content);
return;
}
let pull_request_notice = (pull_requests && diff.phase == Phase::Before)
.then(|| diff.pull_request_notice())
.flatten();
@@ -3425,14 +3492,34 @@ fn draw_diff(
.as_ref()
.map_or("not selected", |revision| revision.label.as_str());
let stage_text = if pull_requests {
let sort = match diff.pull_request_priority {
crate::diff::PullRequestReviewScope::All => "Newest".into(),
scope => format!("{} first", scope.label()),
let filters = &diff.pull_request_preferences.filters;
let review = if filters.len() == 3 {
"All".into()
} else if filters.is_empty() {
"None".into()
} else {
filters
.iter()
.map(|filter| match filter {
crate::pull_requests::ReviewFilter::Requested => "Requested",
crate::pull_requests::ReviewFilter::Approved => "Approved",
crate::pull_requests::ReviewFilter::Other => "Other",
})
.collect::<Vec<_>>()
.join("+")
};
format!(
"Review: {} · Ctrl+f{}Sort: {sort} · Ctrl+s",
diff.pull_request_filter.label(),
"Review: {review} · Ctrl+f{}Sort settings · Ctrl+s{}",
if stacked_review_controls { "\n" } else { " " },
if editor.is_some_and(|editor| editor
.pull_request_configured
.iter()
.any(|&configured| configured))
{
" · configured"
} else {
""
},
)
} else {
format!(
@@ -3531,8 +3618,8 @@ fn draw_diff(
diff.category,
&diff.query,
diff.comparison_mode as u8,
diff.pull_request_filter,
diff.pull_request_priority,
&diff.pull_request_preferences.filters,
&diff.pull_request_preferences.sort,
diff.catalog
.as_ref()
.map(|catalog| (catalog.entries.as_ptr(), catalog.entries.len())),
+303 -46
View File
@@ -432,8 +432,11 @@ fn pull_page(closed: bool, rows: &[(u64, &str, &str)], has_more: bool) -> Outcom
Category::Open
},
),
date.into(),
PullRequestReviewState::None,
PullRequestMetadata {
created_at: date.into(),
updated_at: date.into(),
..Default::default()
},
)
})
.collect(),
@@ -464,7 +467,8 @@ fn pull_request_review_refresh_preserves_identity_sorting_and_raw_search_labels(
result: Ok(page), ..
} = &mut page
{
page.entries[0].2 = review_state;
page.entries[0].1.requested = review_state == PullRequestReviewState::Requested;
page.entries[0].1.approved = review_state == PullRequestReviewState::Approved;
}
state.apply_result(1, Ok(page));
let selected = state.entries().nth(state.selected).unwrap();
@@ -487,8 +491,8 @@ fn pull_request_review_refresh_preserves_identity_sorting_and_raw_search_labels(
}
#[test]
fn pull_request_review_controls_compose_with_search_categories_and_streamed_updates() {
use PullRequestReviewState::{Approved, None, Requested};
fn pull_request_menus_apply_cancel_and_compose_with_query_categories_and_updates() {
use crate::pull_requests::{Direction, Preferences, ReviewFilter, SortField};
let mut editor = editor();
editor.execute("pull_requests origin");
let (id, _) = editor.take_diff_request().unwrap();
@@ -509,10 +513,9 @@ fn pull_request_review_controls_compose_with_search_categories_and_streamed_upda
result: Ok(page), ..
} = &mut page
{
for (entry, state) in page.entries.iter_mut().zip([
Requested, Approved, None, Requested, Approved, None, Requested,
]) {
entry.2 = state;
for (index, entry) in page.entries.iter_mut().enumerate() {
entry.1.requested = matches!(index, 0 | 3 | 6);
entry.1.approved = matches!(index, 0 | 1 | 4);
}
page.entries[6].0.category = Category::Drafts;
}
@@ -524,47 +527,110 @@ fn pull_request_review_controls_compose_with_search_categories_and_streamed_upda
.entries()
.map(|entry| {
let RevisionTarget::PullRequest { number, .. } = entry.target else {
panic!("expected pull request");
panic!("expected PR")
};
number
})
.collect::<Vec<_>>()
};
let selected = |editor: &Editor| {
let diff = editor.diff().unwrap();
diff.entries().nth(diff.selected).unwrap().target.clone()
};
assert_eq!(numbers(&editor), [6, 5, 4, 3, 2, 1]);
editor.handle(Input::End);
let target = selected(&editor);
editor.handle(Input::Ctrl('s'));
assert_eq!(numbers(&editor), [4, 1, 6, 5, 3, 2]);
assert_eq!(selected(&editor), target);
editor.handle(Input::Ctrl('s'));
assert_eq!(numbers(&editor), [5, 2, 6, 4, 3, 1]);
assert_eq!(selected(&editor), target);
editor.handle(Input::Ctrl('s'));
assert_eq!(numbers(&editor), [6, 5, 4, 3, 2, 1]);
for ch in "Fix".chars() {
editor.handle(Input::Char(ch));
}
assert_eq!(numbers(&editor), [5, 4, 3, 2, 1, 6]);
assert_eq!(
numbers(&editor),
[4, 1, 6, 5, 3, 2],
"query matches without outranking configured sort"
);
editor.handle(Input::Ctrl('u'));
editor.handle(Input::End);
let target = editor
.diff()
.unwrap()
.entries()
.last()
.unwrap()
.target
.clone();
editor.handle(Input::Ctrl('s'));
assert_eq!(numbers(&editor), [4, 1, 5, 3, 2, 6]);
editor.handle(Input::Down);
editor.handle(Input::Char('1'));
editor.handle(Input::Right);
assert_eq!(editor.diff().unwrap().query, "");
assert_eq!(editor.pull_request_preferences, Preferences::default());
editor.handle(Input::Escape);
assert!(editor.diff().is_some());
assert!(editor.diff().unwrap().pull_request_menu.is_none());
assert!(!editor.pull_request_preferences_changed);
editor.handle(Input::Ctrl('s'));
editor.handle(Input::Ctrl('j'));
editor.handle(Input::Char('1'));
editor.handle(Input::Right);
editor.handle(Input::Enter);
assert_eq!(numbers(&editor), [1, 2, 3, 4, 5, 6]);
let diff = editor.diff().unwrap();
assert_eq!(diff.entries().nth(diff.selected).unwrap().target, target);
assert!(editor.pull_request_preferences_changed);
assert_eq!(
editor.pull_request_preferences.sort[&SortField::UpdatedAt].direction,
Direction::Asc
);
for ch in "Fix".chars() {
editor.handle(Input::Char(ch));
}
assert_eq!(
numbers(&editor),
[1, 2, 3, 4, 5, 6],
"settings sort wins over fuzzy scores"
);
editor.pull_request_preferences_changed = false;
editor.handle(Input::Ctrl('s'));
editor.handle(Input::Char('4'));
editor.handle(Input::Ctrl('f'));
assert_eq!(numbers(&editor), [4, 1]);
editor.handle(Input::Ctrl('f'));
assert_eq!(numbers(&editor), [5, 2]);
editor.handle(Input::Ctrl('f'));
assert_eq!(numbers(&editor), [4, 1, 5, 3, 2, 6]);
editor.handle(Input::Ctrl('f'));
editor.handle(Input::Home);
let mut update = pull_page(false, &[(4, "Fix", "4")], true);
assert!(
!editor
.diff()
.unwrap()
.pull_request_menu
.as_ref()
.unwrap()
.sort
);
assert_eq!(
editor
.diff()
.unwrap()
.pull_request_menu
.as_ref()
.unwrap()
.draft,
editor.pull_request_preferences
);
editor.handle(Input::Down);
editor.handle(Input::Char(' '));
editor.handle(Input::Down);
editor.handle(Input::Char(' '));
assert_eq!(
numbers(&editor),
[1, 2, 3, 4, 5, 6],
"draft filtering is not applied"
);
editor.handle(Input::Enter);
assert_eq!(
editor.pull_request_preferences.filters,
[ReviewFilter::Requested]
);
assert_eq!(
numbers(&editor),
[1, 4],
"outstanding request wins even with prior approval"
);
let mut update = pull_page(false, &[(4, "Fix updated", "0")], true);
if let Outcome::PullRequestPage {
result: Ok(page), ..
} = &mut update
{
page.entries[0].2 = Approved;
page.entries[0].1.approved = true;
}
editor.apply_diff_result(id, Ok(update));
assert_eq!(numbers(&editor), [1]);
@@ -576,19 +642,210 @@ fn pull_request_review_controls_compose_with_search_categories_and_streamed_upda
assert_eq!(numbers(&editor), [1, 7]);
assert_eq!(editor.diff().unwrap().query, "Fix");
assert!(editor.diff().unwrap().pull_request_partial());
editor.handle(Input::Ctrl('u'));
assert_eq!(numbers(&editor), [7, 1]);
editor.handle(Input::Ctrl('f'));
editor.handle(Input::Char(' '));
editor.handle(Input::Enter);
assert!(matches!(
editor.take_diff_request(),
Some((
_,
Operation::ResolvePullRequest(Revision {
target: RevisionTarget::PullRequest { number: 7, .. },
..
assert!(numbers(&editor).is_empty(), "no checkboxes means no rows");
editor.execute("diff.close");
editor.execute("pull_requests elsewhere");
assert_eq!(
editor.diff().unwrap().pull_request_preferences,
editor.pull_request_preferences
);
}
#[test]
fn pull_request_sort_ranks_pin_slots_shift_collisions_and_clear_without_changing_direction() {
use crate::pull_requests::{Direction, Preferences, SortField::*};
let mut state = DiffState::pull_requests("origin".into());
state.handle(Input::Ctrl('s'));
for input in [
Input::Char('2'),
Input::Home,
Input::Char('2'),
Input::Right,
] {
state.handle(input);
}
let draft = &state.pull_request_menu.as_ref().unwrap().draft;
assert_eq!(draft.sort[&Requested].precedence, Some(3));
assert_eq!(draft.sort[&UpdatedAt].precedence, Some(2));
assert_eq!(
draft.ordered_fields(),
[Approved, UpdatedAt, Requested, CreatedAt]
);
assert_eq!(state.pull_request_menu.as_ref().unwrap().selected, 1);
state.handle(Input::Backspace);
assert_eq!(state.pull_request_menu.as_ref().unwrap().selected, 0);
state.handle(Input::Enter);
assert_eq!(
state.pull_request_preferences.sort[&UpdatedAt].precedence,
None
);
assert_eq!(
state.pull_request_preferences.sort[&UpdatedAt].direction,
Direction::Asc
);
assert_eq!(
state.pull_request_preferences.ordered_fields(),
[UpdatedAt, Approved, Requested, CreatedAt]
);
let mut preferences = Preferences::default();
preferences.set_rank(Requested, Some(4));
preferences.set_rank(UpdatedAt, Some(4));
assert_eq!(preferences.sort[&Requested].precedence, Some(1));
for (field, rank) in [(Approved, 1), (CreatedAt, 2), (UpdatedAt, 1)] {
preferences.set_rank(field, Some(rank));
preferences.validate().unwrap();
}
assert_eq!(
preferences.ordered_fields(),
[UpdatedAt, Approved, CreatedAt, Requested]
);
}
#[test]
fn pull_request_sort_uses_independent_approval_dates_and_number_ties() {
use crate::pull_requests::SortField;
let mut state = DiffState::pull_requests("origin".into());
state.take_request();
let mut page = pull_page(
false,
&[(1, "A", "1"), (2, "B", "1"), (3, "C", "1"), (4, "D", "2")],
false,
);
if let Outcome::PullRequestPage {
result: Ok(page), ..
} = &mut page
{
for entry in &mut page.entries {
entry.1.updated_at = "same".into();
}
page.entries[0].1.requested = true;
page.entries[0].1.approved = true;
page.entries[1].1.requested = true;
}
state.apply_result(1, Ok(page));
let numbers = |state: &DiffState| {
state
.entries()
.map(|entry| {
let RevisionTarget::PullRequest { number, .. } = entry.target else {
unreachable!()
};
number
})
))
));
.collect::<Vec<_>>()
};
assert_eq!(numbers(&state), [1, 2, 4, 3]);
assert_eq!(
state.pull_request_review_state(state.entries().next().unwrap()),
PullRequestReviewState::Requested
);
let mut preferences = state.pull_request_preferences.clone();
preferences.set_rank(SortField::CreatedAt, Some(1));
state.set_pull_request_preferences(preferences);
assert_eq!(numbers(&state), [4, 1, 2, 3]);
state.handle(Input::Ctrl('s'));
state.handle(Input::Home);
state.handle(Input::Left);
state.handle(Input::Enter);
assert_eq!(
numbers(&state),
[1, 2, 3, 4],
"creation direction is independent of update direction"
);
let mut tied = pull_page(false, &[(5, "E", "1")], false);
if let Outcome::PullRequestPage {
result: Ok(page), ..
} = &mut tied
{
page.entries[0].1.updated_at = "same".into();
}
state.apply_result(1, Ok(tied));
assert_eq!(
numbers(&state),
[1, 2, 5, 3, 4],
"number descending breaks complete ties"
);
let mut preferences = state.pull_request_preferences.clone();
preferences
.sort
.entry(SortField::Requested)
.or_default()
.direction = crate::pull_requests::Direction::Asc;
state.set_pull_request_preferences(preferences);
assert_eq!(
numbers(&state),
[5, 3, 1, 2, 4],
"explicit config directions also apply to booleans"
);
}
#[test]
fn pull_request_menu_drafts_survive_streaming_and_checkbox_unions_exclude_requests_from_approved() {
let mut state = DiffState::pull_requests("origin".into());
state.take_request();
state.handle(Input::Ctrl('f'));
state.handle(Input::Char(' '));
let mut page = pull_page(
false,
&[(1, "Both", "1"), (2, "Approved", "2"), (3, "Other", "3")],
true,
);
if let Outcome::PullRequestPage {
result: Ok(page), ..
} = &mut page
{
page.entries[0].1.requested = true;
page.entries[0].1.approved = true;
page.entries[1].1.approved = true;
}
state.apply_result(1, Ok(page));
assert_eq!(state.entries().count(), 3);
assert_eq!(
state.pull_request_menu.as_ref().unwrap().draft.filters,
[ReviewFilter::Approved, ReviewFilter::Other]
);
state.handle(Input::Enter);
assert_eq!(
state
.entries()
.map(|entry| entry.label.as_str())
.collect::<Vec<_>>(),
["#3 Other", "#2 Approved"]
);
}
#[test]
fn pull_request_preferences_validate_ranks_filters_and_unknown_fields() {
for value in [
r#"{"sort":{"requested":{"precedence":0}}}"#,
r#"{"sort":{"requested":{"precedence":5}}}"#,
r#"{"sort":{"requested":{"precedence":1},"approved":{"precedence":1}}}"#,
r#"{"filters":["requested","requested"]}"#,
] {
let preferences: crate::pull_requests::Preferences = serde_json::from_str(value).unwrap();
assert!(preferences.validate().is_err(), "{value}");
}
for value in [
r#"{"extra":true}"#,
r#"{"sort":{"requested":{"extra":true}}}"#,
r#"{"filters":["unknown"]}"#,
] {
assert!(
serde_json::from_str::<crate::pull_requests::Preferences>(value).is_err(),
"{value}"
);
}
let preferences: crate::pull_requests::Preferences =
serde_json::from_str(r#"{"sort":{"updated_at":{}}}"#).unwrap();
preferences.validate().unwrap();
assert_eq!(preferences.filters, ReviewFilter::ALL);
assert_eq!(
preferences.sort[&SortField::UpdatedAt],
crate::pull_requests::SortRule::default()
);
}
#[test]
+18 -8
View File
@@ -498,6 +498,7 @@ fn summaries_keep_outstanding_requests_and_approvals_independent() {
});
}
Ok(json!([{"number":7,"iid":7,"title":"Request again","created_at":"2024-01-01T00:00:00Z",
"updated_at":"2024-01-02T01:00:00+01:00",
"state":if provider == Provider::Gitlab { "opened" } else { "open" },
"merged":false,"merged_at":null,"user":{"login":"author"},"author":{"username":"author"},
"requested_reviewers":[{"login":"viewer","id":42}],"reviewers":[{"username":"viewer"}],
@@ -506,6 +507,7 @@ fn summaries_keep_outstanding_requests_and_approvals_independent() {
assert_eq!(page.entries.len(), 1);
assert!(page.entries[0].approved && page.entries[0].review_requested);
assert!(page.entries[0].description.contains("+1 -2"));
assert_eq!(page.entries[0].updated_at, "2024-01-02T00:00:00.000000000Z");
}
}
@@ -524,7 +526,8 @@ fn draft_summaries_keep_open_closed_and_merged_states_distinct() {
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"
"user": null, "author": null, "created_at": "2024-01-01T00:00:00Z",
"updated_at": "2024-01-02T00:00:00Z"
});
let entry = summary(provider, &value, Some((1, 2))).unwrap();
assert!(entry.draft);
@@ -602,6 +605,7 @@ fn pull_request_sizes_include_zero_and_preserve_unknown_statistics() {
"state":if provider == Provider::Gitlab { "opened" } else { "open" },
"user":{"login":"author"}, "author":{"username":"author"},
"created_at":"2024-01-01T00:00:00Z",
"updated_at":"2024-01-02T00:00:00Z",
"base":{"ref":"main"}, "head":{"ref":"topic"},
"target_branch":"main", "source_branch":"topic",
"additions":match id { 7 => Some(123), 8 => Some(0), _ => None },
@@ -654,10 +658,10 @@ fn pages_are_bounded_newest_first_summaries_without_historical_metadata() {
assert_eq!(url, format!("{}?{query}", forge.endpoint));
Ok(json!([
{"number": 1, "iid": 1, "title": "Older", "state": if provider == Provider::Gitlab { "opened" } else { "open" },
"user": null, "author": null, "merged_at": null, "merged": false, "created_at": "2024-01-01T01:00:00+01:00"},
"user": null, "author": null, "merged_at": null, "merged": false, "created_at": "2024-01-01T01:00:00+01:00", "updated_at":"2024-02-01T01:00:00+01:00"},
{"number": 2, "iid": 2, "title": "Newer", "state": if provider == Provider::Gitlab { "opened" } else { "open" },
"user": {"login": "author"}, "author": {"username": "author"}, "merged_at": null, "merged": false,
"created_at": "2024-01-01T00:00:00.001Z", "diff_refs": null, "head": {"sha": "not-a-commit"}}
"created_at": "2024-01-01T00:00:00.001Z", "updated_at":"2024-02-01T00:00:00.001Z", "diff_refs": null, "head": {"sha": "not-a-commit"}}
]))
}).unwrap();
assert_eq!(
@@ -665,6 +669,8 @@ fn pages_are_bounded_newest_first_summaries_without_historical_metadata() {
[2, 1]
);
assert!(page.entries[1].description.contains("deleted user"));
assert_eq!(page.entries[1].updated_at, "2024-02-01T00:00:00.000000000Z");
assert_eq!(page.entries[0].updated_at, "2024-02-01T00:00:00.001000000Z");
assert!(
page.has_more,
"A short, server-capped page is not exhaustion"
@@ -708,7 +714,7 @@ fn gitlab_closed_combines_closed_and_merged_pages_without_open_requests() {
_ => return Ok(json!([])),
};
Ok(json!([{"iid": id, "title": state, "state": state,
"author": null, "created_at": date, "diff_refs": null}]))
"author": null, "created_at": date, "updated_at": date, "diff_refs": null}]))
})
.unwrap();
assert_eq!(states, ["closed", "merged"]);
@@ -733,7 +739,7 @@ fn gitlab_closed_combines_closed_and_merged_pages_without_open_requests() {
return Err("statistics unavailable".into());
}
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", "updated_at": "2024-01-02T00:00:00Z"}]))
}).unwrap();
assert_eq!(raced.entries.len(), 1);
assert!(raced.entries[0].description.contains("merged"));
@@ -755,7 +761,9 @@ fn timestamps_normalize_offsets_precision_and_calendar_boundaries() {
"2024-01-01T00:00:00.000000001Z",
),
] {
assert_eq!(created_at(&json!({"created_at": input})).unwrap(), expected);
for key in ["created_at", "updated_at"] {
assert_eq!(timestamp(&json!({(key): input}), key).unwrap(), expected);
}
}
for input in [
"",
@@ -770,12 +778,14 @@ fn timestamps_normalize_offsets_precision_and_calendar_boundaries() {
"0001-01-01T00:00:00+01:00",
"9999-12-31T23:00:00-02:00",
] {
for key in ["created_at", "updated_at"] {
assert!(
created_at(&json!({"created_at": input})).is_err(),
"{input}"
timestamp(&json!({(key): input}), key).is_err(),
"{key}: {input}"
);
}
}
}
#[test]
fn resolution_reads_only_selected_detail_and_exact_preserved_head() {
+125
View File
@@ -0,0 +1,125 @@
use super::*;
use crate::pull_requests::{Direction, ReviewFilter, SortField, SortRule};
struct Fixture(PathBuf);
impl Fixture {
fn new() -> Self {
let path = std::env::temp_dir().join(format!(
"vamp-pr-preferences-{}-{}",
std::process::id(),
NEXT_TEMP.fetch_add(1, Ordering::Relaxed)
));
fs::create_dir(&path).unwrap();
Self(path)
}
fn path(&self) -> PathBuf {
self.0.join("state/vamp/pull_requests.json")
}
}
impl Drop for Fixture {
fn drop(&mut self) {
let _ = fs::remove_dir_all(&self.0);
}
}
#[test]
fn paths_use_absolute_xdg_state_or_home_without_reading_environment() {
assert_eq!(
path_from(Some(OsStr::new("/state")), Some(OsStr::new("/home"))).unwrap(),
PathBuf::from("/state/vamp/pull_requests.json")
);
for xdg in [None, Some(OsStr::new("")), Some(OsStr::new("relative"))] {
assert_eq!(
path_from(xdg, Some(OsStr::new("/home"))).unwrap(),
PathBuf::from("/home/.local/state/vamp/pull_requests.json")
);
}
assert!(path_from(None, None).is_err());
assert!(path_from(None, Some(OsStr::new("relative"))).is_err());
}
#[test]
fn loading_is_lazy_and_applied_preferences_round_trip_atomically() {
let fixture = Fixture::new();
let path = fixture.path();
let (store, mut preferences, notice) = Store::load(Ok(path.clone()));
assert_eq!(preferences, Preferences::default());
assert!(notice.is_none());
assert!(!path.parent().unwrap().exists());
preferences.filters = vec![ReviewFilter::Approved];
preferences.sort.insert(
SortField::UpdatedAt,
SortRule {
precedence: Some(1),
direction: Direction::Asc,
},
);
store.save(&preferences).unwrap();
let (_, loaded, notice) = Store::load(Ok(path.clone()));
assert_eq!(loaded, preferences);
assert!(notice.is_none());
preferences.filters.clear();
preferences.sort.clear();
store.save(&preferences).unwrap();
assert_eq!(Store::load(Ok(path)).1, preferences);
assert_eq!(
fs::read_dir(fixture.0.join("state/vamp")).unwrap().count(),
1
);
}
#[test]
fn corrupt_incompatible_and_invalid_data_is_preserved_even_after_apply() {
let fixture = Fixture::new();
let path = fixture.path();
fs::create_dir_all(path.parent().unwrap()).unwrap();
for bytes in [
"not json",
r#"{"version":2,"preferences":{}}"#,
r#"{"version":1,"preferences":{"sort":{"unknown":{}}}}"#,
r#"{"version":1,"preferences":{"sort":{"updated_at":{"direction":"up"}}}}"#,
r#"{"version":1,"preferences":{"sort":{"updated_at":{"precedence":0}}}}"#,
r#"{"version":1,"preferences":{"sort":{"updated_at":{"precedence":1},"created_at":{"precedence":1}}}}"#,
r#"{"version":1,"preferences":{"filters":["unknown"]}}"#,
r#"{"version":1,"preferences":{"filters":["requested","requested"]}}"#,
] {
fs::write(&path, bytes).unwrap();
let (store, loaded, notice) = Store::load(Ok(path.clone()));
assert_eq!(loaded, Preferences::default());
assert!(notice.unwrap().contains("session-only"));
assert!(store.save(&loaded).unwrap_err().contains("preserved"));
assert_eq!(fs::read_to_string(&path).unwrap(), bytes);
}
}
#[test]
fn failed_writes_and_newly_incompatible_data_are_reported_without_replacement() {
let fixture = Fixture::new();
let path = fixture.path();
let (store, preferences, _) = Store::load(Ok(path.clone()));
fs::write(fixture.0.join("state"), "keep this file").unwrap();
assert!(
store
.save(&preferences)
.unwrap_err()
.contains("session-only")
);
assert_eq!(
fs::read_to_string(fixture.0.join("state")).unwrap(),
"keep this file"
);
fs::remove_file(fixture.0.join("state")).unwrap();
fs::create_dir_all(path.parent().unwrap()).unwrap();
let incompatible = r#"{"version":9,"preferences":{}}"#;
fs::write(&path, incompatible).unwrap();
assert!(
store
.save(&preferences)
.unwrap_err()
.contains("unsupported")
);
assert_eq!(fs::read_to_string(path).unwrap(), incompatible);
}
+77
View File
@@ -2356,3 +2356,80 @@ fn ruby_rejects_invalid_document_context_and_sync_async_dsl_usage() {
plugin.host.child.is_none() && plugin.host.reader.is_none() && plugin.host.writer.is_none()
);
}
#[test]
fn pull_request_preferences_distinguish_unset_nil_and_explicit_sections() {
use crate::pull_requests::{Direction, ReviewFilter, SortField};
let fixture = Fixture::new();
for assignment in [
"",
"config.pull_request_sort = nil; config.pull_request_filters = nil",
] {
let path = fixture.file(
"config.rb",
&format!("Vamp.configure {{ |config| {assignment} }}"),
);
let config = load_config(&path).unwrap();
assert!(config.pull_request_sort.is_none());
assert!(config.pull_request_filters.is_none());
}
let path = fixture.file(
"config.rb",
r#"
Vamp.configure do |config|
config.pull_request_sort = {
updated_at: {precedence: 1, direction: :desc},
created_at: {direction: :asc}
}
config.pull_request_filters = [:requested, :approved]
end
"#,
);
let config = load_config(&path).unwrap();
let sort = config.pull_request_sort.unwrap();
assert_eq!(sort[&SortField::UpdatedAt].precedence, Some(1));
assert_eq!(sort[&SortField::UpdatedAt].direction, Direction::Desc);
assert_eq!(sort[&SortField::CreatedAt].precedence, None);
assert_eq!(sort[&SortField::CreatedAt].direction, Direction::Asc);
assert_eq!(
config.pull_request_filters.unwrap(),
[ReviewFilter::Requested, ReviewFilter::Approved]
);
let path = fixture.file("config.rb", "Vamp.configure { |config| config.pull_request_sort = {}; config.pull_request_filters = [] }");
let config = load_config(&path).unwrap();
assert!(config.pull_request_sort.unwrap().is_empty());
assert!(config.pull_request_filters.unwrap().is_empty());
}
#[test]
fn pull_request_preferences_reject_unknown_values_and_invalid_ranks() {
let fixture = Fixture::new();
for (field, value) in [
("sort", "[]"),
("sort", "{unknown: {}}"),
("sort", "{updated_at: []}"),
("sort", "{updated_at: {unknown: 1}}"),
("sort", "{updated_at: {direction: :up}}"),
("sort", "{updated_at: {direction: nil}}"),
("sort", "{updated_at: {precedence: 0}}"),
("sort", "{updated_at: {precedence: 5}}"),
("sort", "{updated_at: {precedence: 1.0}}"),
(
"sort",
"{updated_at: {precedence: 1}, created_at: {precedence: 1}}",
),
("filters", "{}"),
("filters", "[:unknown]"),
("filters", "[:requested, :requested]"),
] {
let path = fixture.file(
"config.rb",
&format!("Vamp.configure {{ |config| config.pull_request_{field} = {value} }}"),
);
let error = load_config(&path).unwrap_err();
assert!(
error.contains(&format!("pull_request_{field}")),
"{value}: {error}"
);
}
}
+158
View File
@@ -3836,3 +3836,161 @@ fn async_responses_edit_only_current_bound_working_after_and_refresh_projection(
session.editor.execute("diff.close");
assert!(session.editor.dirty());
}
#[test]
fn applied_pull_request_preferences_survive_startup_without_config() {
use crate::pull_requests::ReviewFilter;
let fixture = Fixture::new();
let path = fixture.0.join("state/pull_requests.json");
let mut session =
EditorSession::open_with_preferences_path(None, vec![], None, Some(Ok(path.clone())))
.unwrap();
assert!(!path.exists());
session.editor.execute("pull_requests");
// Consume the queued listing so menu input cannot start a forge worker.
assert!(session.editor.take_diff_request().is_some());
session.handle(Input::Ctrl('f'));
session.handle(Input::Char(' '));
assert!(!path.exists());
session.handle(Input::Escape);
assert_eq!(
session.editor.pull_request_preferences,
crate::pull_requests::Preferences::default()
);
assert!(!path.exists());
session.handle(Input::Ctrl('f'));
session.handle(Input::Char(' '));
session.handle(Input::Down);
session.handle(Input::Char(' '));
assert!(!path.exists());
session.handle(Input::Enter);
assert!(path.exists());
assert!(!session.editor.pull_request_preferences_changed);
let reopened =
EditorSession::open_with_preferences_path(None, vec![], None, Some(Ok(path))).unwrap();
assert_eq!(
reopened.editor.pull_request_preferences.filters,
[ReviewFilter::Other]
);
assert_eq!(reopened.editor.pull_request_configured, [false, false]);
}
#[test]
fn configured_pull_request_sections_replace_only_their_persisted_section() {
use crate::pull_requests::{Direction, Preferences, ReviewFilter, SortField, SortRule};
let fixture = Fixture::new();
let path = fixture.0.join("state/pull_requests.json");
let (store, mut preferences, _) =
crate::pull_request_preferences::Store::load(Ok(path.clone()));
preferences.filters = vec![ReviewFilter::Other];
preferences.sort.insert(
SortField::UpdatedAt,
SortRule {
precedence: Some(1),
direction: Direction::Asc,
},
);
store.save(&preferences).unwrap();
let bytes = fs::read(&path).unwrap();
for (configuration, expected, configured) in [
(
"config.pull_request_sort = {}",
Preferences {
sort: Default::default(),
filters: preferences.filters.clone(),
},
[true, false],
),
(
"config.pull_request_filters = []",
Preferences {
sort: preferences.sort.clone(),
filters: vec![],
},
[false, true],
),
(
"config.pull_request_sort = {created_at: {direction: :asc}}; config.pull_request_filters = nil",
Preferences {
sort: [(
SortField::CreatedAt,
SortRule {
precedence: None,
direction: Direction::Asc,
},
)]
.into(),
filters: preferences.filters.clone(),
},
[true, false],
),
] {
let config = fixture.file(
"config.rb",
&format!("Vamp.configure {{ |config| {configuration} }}"),
);
let session = EditorSession::open_with_preferences_path(
None,
vec![],
Some(&config),
Some(Ok(path.clone())),
)
.unwrap();
assert_eq!(session.editor.pull_request_preferences, expected);
assert_eq!(session.editor.pull_request_configured, configured);
assert_eq!(fs::read(&path).unwrap(), bytes);
}
}
#[test]
fn corrupt_pull_request_storage_warns_and_apply_remains_session_only() {
let fixture = Fixture::new();
let path = fixture.file("pull_requests.json", "preserve invalid bytes");
let mut session =
EditorSession::open_with_preferences_path(None, vec![], None, Some(Ok(path.clone())))
.unwrap();
assert!(session.editor.message().contains("session-only"));
session.editor.execute("pull_requests");
assert!(session.editor.take_diff_request().is_some());
session.handle(Input::Ctrl('f'));
for input in [
Input::Char(' '),
Input::Down,
Input::Char(' '),
Input::Down,
Input::Char(' '),
Input::Enter,
] {
session.handle(input);
}
assert!(session.editor.message().contains("preserved"));
assert!(session.editor.pull_request_preferences.filters.is_empty());
assert_eq!(fs::read_to_string(path).unwrap(), "preserve invalid bytes");
}
#[test]
fn failed_pull_request_save_keeps_applied_session_preferences_and_reports_failure() {
let fixture = Fixture::new();
let parent = fixture.0.join("state");
let path = parent.join("pull_requests.json");
let mut session =
EditorSession::open_with_preferences_path(None, vec![], None, Some(Ok(path))).unwrap();
fs::write(&parent, "do not replace").unwrap();
session.editor.execute("pull_requests");
assert!(session.editor.take_diff_request().is_some());
session.handle(Input::Ctrl('f'));
for input in [
Input::Char(' '),
Input::Down,
Input::Char(' '),
Input::Down,
Input::Char(' '),
Input::Enter,
] {
session.handle(input);
}
assert!(session.editor.message().contains("session-only"));
assert!(session.editor.pull_request_preferences.filters.is_empty());
assert!(!session.editor.pull_request_preferences_changed);
assert_eq!(fs::read_to_string(parent).unwrap(), "do not replace");
}
+75 -4
View File
@@ -3514,8 +3514,11 @@ fn pull_request_picker_renders_all_rows_without_a_pinned_offset() {
Category::Open
},
},
format!("2026-01-{:02}T00:00:00Z", 20 - index),
crate::diff::PullRequestReviewState::None,
crate::diff::PullRequestMetadata {
created_at: format!("2026-01-{:02}T00:00:00Z", 20 - index),
updated_at: format!("2026-01-{:02}T00:00:00Z", 20 - index),
..Default::default()
},
)
})
.collect(),
@@ -3560,6 +3563,70 @@ fn pull_request_picker_renders_all_rows_without_a_pinned_offset() {
assert!(!text.contains("#00 Pull request") && !text.contains("#19 Pull request"));
}
}
#[test]
fn pull_request_menus_replace_picker_contents_and_show_explicit_choices() {
for (width, height) in [(100, 24), (44, 12)] {
let mut diff = DiffState::pull_requests("origin".into());
let mut terminal =
Terminal::new(ratatui::backend::TestBackend::new(width, height)).unwrap();
let mut panels = WheelPanels::default();
diff.handle(Input::Ctrl('s'));
render_diff(&mut terminal, &diff, &mut panels);
let text = screen_text(&terminal);
assert!(text.contains("Sort pull requests"));
assert!(text.contains("[–] Requested from me"));
assert!(text.contains("Updated at") && text.contains("Created at"));
assert!(text.contains("newest first"));
assert!(!text.contains("Loading pull requests") && !text.contains("[partial]"));
assert!(text.contains("Enter Apply") && text.contains("Esc Cancel"));
diff.handle(Input::Char('3'));
render_diff(&mut terminal, &diff, &mut panels);
let text = screen_text(&terminal);
let row = |label: &str| text.lines().position(|line| line.contains(label)).unwrap();
assert!(row("Updated at") < row("Approved by me"));
assert!(row("Approved by me") < row("Requested from me"));
assert!(row("Requested from me") < row("Created at"));
assert!(text.contains("> [3] Requested from me"));
diff.handle(Input::Backspace);
render_diff(&mut terminal, &diff, &mut panels);
assert!(screen_text(&terminal).contains("> [–] Requested from me"));
diff.handle(Input::Down);
diff.handle(Input::Char('4'));
diff.handle(Input::Left);
render_diff(&mut terminal, &diff, &mut panels);
let text = screen_text(&terminal);
assert!(text.contains("> [4] Updated at · oldest first"));
let row = |label: &str| text.lines().position(|line| line.contains(label)).unwrap();
assert!(row("Requested from me") < row("Approved by me"));
assert!(row("Approved by me") < row("Created at"));
assert!(row("Created at") < row("Updated at"));
diff.handle(Input::Backspace);
render_diff(&mut terminal, &diff, &mut panels);
let text = screen_text(&terminal);
assert!(text.contains("> [–] Updated at · oldest first"));
let row = |label: &str| text.lines().position(|line| line.contains(label)).unwrap();
assert!(row("Requested from me") < row("Updated at"));
assert!(row("Updated at") < row("Approved by me"));
assert!(row("Approved by me") < row("Created at"));
diff.handle(Input::Right);
render_diff(&mut terminal, &diff, &mut panels);
assert!(screen_text(&terminal).contains("> [–] Updated at · newest first"));
diff.handle(Input::Ctrl('f'));
diff.handle(Input::Char(' '));
render_diff(&mut terminal, &diff, &mut panels);
let text = screen_text(&terminal);
assert!(text.contains("Filter pull requests"));
assert!(text.contains("[ ] Requested from me"));
assert!(text.contains("[x] Approved by me"));
assert!(text.contains("[x] Other / unavailable"));
assert!(!text.contains("Updated at"));
diff.handle(Input::Escape);
render_diff(&mut terminal, &diff, &mut panels);
assert!(screen_text(&terminal).contains("Pull requests"));
assert!(screen_text(&terminal).contains("[partial]"));
}
}
#[test]
fn pull_request_review_symbols_use_terminal_cells_and_keep_the_selection_arrow_independent() {
use crate::diff::{
@@ -3594,8 +3661,12 @@ fn pull_request_review_symbols_use_terminal_cells_and_keep_the_selection_arrow_i
description: format!("metadata{index}"),
category: Category::Open,
},
(3 - index).to_string(),
state,
crate::diff::PullRequestMetadata {
created_at: (3 - index).to_string(),
updated_at: (3 - index).to_string(),
requested: state == PullRequestReviewState::Requested,
approved: state != PullRequestReviewState::None,
},
)
})
.collect(),