From 7876b30445fafbe206dee4828e6c9ecc3abc2535 Mon Sep 17 00:00:00 2001 From: sashml Date: Wed, 1 Jul 2026 12:15:17 +0200 Subject: [PATCH] feat(pr): filter by merge state + help overlay with glyph legend Rebased onto current main and reconciled with the git-log/diff overlay rework (#9): the filter + help are additive and independent of the per-PR Overlay state. - `f` cycles the merge filter: all -> conflicting -> clean; active filter shown in the table title. Composes with `/` search (applied in filtered_prs). - `?` opens a help overlay documenting all keybindings (incl. l: log, d: diff) and the State/CI glyph legends (accessibility: glyphs are now documented). - state: MergeFilter enum + merge_filter/help_open; Esc/h close help first. - tests: cycle order, help toggle, back-closes-help, filter + search composition. Refs: #10, zombo-sash-eco-pp0u --- src/app/actions.rs | 2 ++ src/app/event_loop.rs | 2 ++ src/app/state.rs | 49 ++++++++++++++++++++++--- src/app/update.rs | 14 +++++++- src/app/view.rs | 1 + src/ui/widgets.rs | 84 +++++++++++++++++++++++++++++++++++++++++-- tests/state_tests.rs | 75 ++++++++++++++++++++++++++++++++++++++ 7 files changed, 220 insertions(+), 7 deletions(-) diff --git a/src/app/actions.rs b/src/app/actions.rs index 2016300..050d989 100644 --- a/src/app/actions.rs +++ b/src/app/actions.rs @@ -14,6 +14,8 @@ pub enum Action { ToggleGitLog, ToggleDiff, CloseOverlay, + ToggleHelp, + CycleMergeFilter, SearchInput(char), SearchBackspace, SearchClear, diff --git a/src/app/event_loop.rs b/src/app/event_loop.rs index 50287e7..5d665af 100644 --- a/src/app/event_loop.rs +++ b/src/app/event_loop.rs @@ -291,6 +291,8 @@ fn map_event_to_action(event: &Event, state: &AppState) -> Option { KeyCode::BackTab => Some(Action::SwitchPane), KeyCode::Char('r') => Some(Action::Refresh), KeyCode::Char('o') => Some(Action::OpenInBrowser), + KeyCode::Char('f') => Some(Action::CycleMergeFilter), + KeyCode::Char('?') => Some(Action::ToggleHelp), KeyCode::Char('/') => Some(Action::ToggleSearch), _ => None, } diff --git a/src/app/state.rs b/src/app/state.rs index 46de286..df6c434 100644 --- a/src/app/state.rs +++ b/src/app/state.rs @@ -28,6 +28,42 @@ pub enum Overlay { Diff, } +/// Merge-state filter applied to PR lists. Cycled with the filter key. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum MergeFilter { + All, + Conflicting, + Clean, +} + +impl MergeFilter { + /// Cycle All -> Conflicting -> Clean -> All. + pub fn next(self) -> Self { + match self { + MergeFilter::All => MergeFilter::Conflicting, + MergeFilter::Conflicting => MergeFilter::Clean, + MergeFilter::Clean => MergeFilter::All, + } + } + + /// Short label for the table title; `None` when inactive (All). + pub fn label(self) -> Option<&'static str> { + match self { + MergeFilter::All => None, + MergeFilter::Conflicting => Some("conflicting"), + MergeFilter::Clean => Some("clean"), + } + } + + fn matches(self, pr: &PullRequest) -> bool { + match self { + MergeFilter::All => true, + MergeFilter::Conflicting => pr.mergeable.as_deref() == Some("CONFLICTING"), + MergeFilter::Clean => pr.mergeable.as_deref() == Some("MERGEABLE"), + } + } +} + #[derive(Debug, Clone, PartialEq, Eq)] pub enum FocusedPane { Navigation, @@ -91,6 +127,10 @@ pub struct AppState { /// Vertical scroll offset (in lines) for the diff overlay. pub diff_scroll: u16, + // Merge-state filter + help overlay + pub merge_filter: MergeFilter, + pub help_open: bool, + // UI flags pub loading: bool, pub loading_orgs: HashSet, @@ -133,6 +173,8 @@ impl AppState { pr_details: HashMap::new(), pr_diffs: HashMap::new(), diff_scroll: 0, + merge_filter: MergeFilter::All, + help_open: false, loading: true, loading_orgs: HashSet::new(), error_message: None, @@ -186,13 +228,12 @@ impl AppState { } pub fn filtered_prs(&self, prs: &[PullRequest]) -> Vec { - if self.search_query.is_empty() { - return prs.to_vec(); - } let query = self.search_query.to_lowercase(); prs.iter() + .filter(|pr| self.merge_filter.matches(pr)) .filter(|pr| { - pr.title.to_lowercase().contains(&query) + query.is_empty() + || pr.title.to_lowercase().contains(&query) || pr.author.to_lowercase().contains(&query) || pr.repo_name.to_lowercase().contains(&query) || pr.repo_full_name().to_lowercase().contains(&query) diff --git a/src/app/update.rs b/src/app/update.rs index cecd055..20f7463 100644 --- a/src/app/update.rs +++ b/src/app/update.rs @@ -96,7 +96,9 @@ pub fn update(state: &mut AppState, action: Action) -> Vec { vec![] } Action::Back => { - if state.search_active { + if state.help_open { + state.help_open = false; + } else if state.search_active { state.search_active = false; state.search_query.clear(); } else if state.error_message.is_some() { @@ -167,6 +169,16 @@ pub fn update(state: &mut AppState, action: Action) -> Vec { state.overlay = Overlay::None; vec![] } + Action::ToggleHelp => { + state.help_open = !state.help_open; + vec![] + } + Action::CycleMergeFilter => { + state.merge_filter = state.merge_filter.next(); + // Row set changed; reset the cursor so it stays in range. + state.content_cursor = 0; + vec![] + } Action::SearchInput(ch) => { if state.search_active { state.search_query.push(ch); diff --git a/src/app/view.rs b/src/app/view.rs index 29c298b..96cac28 100644 --- a/src/app/view.rs +++ b/src/app/view.rs @@ -31,6 +31,7 @@ pub fn render(f: &mut Frame, state: &AppState) { // Overlays widgets::render_pr_overlay(f, state); + widgets::render_help_overlay(f, state); widgets::render_search_overlay(f, state); if state.error_message.is_some() { widgets::render_error_modal(f, f.area(), state); diff --git a/src/ui/widgets.rs b/src/ui/widgets.rs index 3c8bab7..6c065ce 100644 --- a/src/ui/widgets.rs +++ b/src/ui/widgets.rs @@ -153,8 +153,18 @@ fn render_pr_table( } else { String::new() }; + let merge_suffix = match state.merge_filter.label() { + Some(l) => format!(" [state: {}]", l), + None => String::new(), + }; - let title = format!(" {} ({}) {} ", title, prs.len(), search_suffix); + let title = format!( + " {} ({}){}{} ", + title, + prs.len(), + merge_suffix, + search_suffix + ); let block = Block::default() .title(title) @@ -330,7 +340,7 @@ pub fn render_status_bar(f: &mut Frame, area: Rect, state: &AppState) { let key_hints = if state.search_active { "Esc: close search | Enter: filter" } else { - "j/k: nav | Tab: pane | Enter: select | l: log | d: diff | /: search | r: refresh | o: open | q: quit" + "j/k: nav | Enter: select | l: log | d: diff | f: filter | /: search | r: refresh | o: open | ?: help | q: quit" }; let status = if state.loading { @@ -635,3 +645,73 @@ fn render_diff_overlay(f: &mut Frame, state: &AppState) { let para = Paragraph::new(lines).block(block).scroll((scroll, 0)); f.render_widget(para, modal_area); } + +/// Help overlay: keybindings plus the State/CI glyph legends (accessibility — glyphs +/// are otherwise undocumented). Independent of the per-PR `Overlay` state. +pub fn render_help_overlay(f: &mut Frame, state: &AppState) { + if !state.help_open { + return; + } + + let area = f.area(); + let modal_width = 66u16.clamp(40, area.width.saturating_sub(4)); + let modal_height = 18u16.min(area.height.saturating_sub(2)); + let x = (area.width.saturating_sub(modal_width)) / 2; + let y = (area.height.saturating_sub(modal_height)) / 2; + let modal_area = Rect { + x, + y, + width: modal_width, + height: modal_height, + }; + + let block = Block::default() + .title(" Help ") + .borders(Borders::ALL) + .border_style(theme::BORDER_FOCUSED); + + let key = |k: &'static str, desc: &'static str| { + Line::from(vec![ + Span::styled(format!(" {:<12}", k), theme::PR_NUMBER), + Span::raw(desc), + ]) + }; + + let lines = vec![ + Line::from(Span::styled("Keys", theme::HEADER)), + key("j / k", "move up / down (scroll in diff)"), + key("Enter", "select / expand"), + key("l", "git-log overlay (content pane)"), + key("d", "diff overlay (content pane)"), + key("f", "cycle merge filter: all -> conflicting -> clean"), + key("/", "search r refresh o open in browser"), + key("Tab", "switch pane h / Esc back / close q quit"), + Line::from(""), + Line::from(Span::styled("State column", theme::HEADER)), + Line::from(vec![ + Span::styled(" ✓ ok", theme::MERGE_CLEAN), + Span::raw(" mergeable "), + Span::styled("✗ cf", theme::MERGE_CONFLICT), + Span::raw(" conflicting "), + Span::styled("?", theme::DIM), + Span::raw(" unknown (not yet computed)"), + ]), + Line::from(Span::styled("CI column", theme::HEADER)), + Line::from(vec![ + Span::styled(" ✓", theme::MERGE_CLEAN), + Span::raw(" passing "), + Span::styled("✗", theme::MERGE_CONFLICT), + Span::raw(" failing "), + Span::styled("…", theme::WARNING), + Span::raw(" pending "), + Span::styled("·", theme::DIM), + Span::raw(" no checks"), + ]), + Line::from(""), + Line::from(Span::styled("Press ? or Esc to close", theme::DIM)), + ]; + + f.render_widget(Clear, modal_area); + let para = Paragraph::new(lines).block(block); + f.render_widget(para, modal_area); +} diff --git a/tests/state_tests.rs b/tests/state_tests.rs index 69c6295..76aae30 100644 --- a/tests/state_tests.rs +++ b/tests/state_tests.rs @@ -595,3 +595,78 @@ fn test_pr_detail_failed_records_error() { Some(PrDetailEntry::Failed(_)) )); } + +// --- Merge-state filter + help (task pp0u) --- + +#[test] +fn test_cycle_merge_filter_order() { + use ghdash::app::state::MergeFilter; + let mut state = make_state(); + assert_eq!(state.merge_filter, MergeFilter::All); + update(&mut state, Action::CycleMergeFilter); + assert_eq!(state.merge_filter, MergeFilter::Conflicting); + update(&mut state, Action::CycleMergeFilter); + assert_eq!(state.merge_filter, MergeFilter::Clean); + update(&mut state, Action::CycleMergeFilter); + assert_eq!(state.merge_filter, MergeFilter::All); +} + +#[test] +fn test_toggle_help_flips_flag() { + let mut state = make_state(); + assert!(!state.help_open); + update(&mut state, Action::ToggleHelp); + assert!(state.help_open); + update(&mut state, Action::ToggleHelp); + assert!(!state.help_open); +} + +#[test] +fn test_back_closes_help_before_overlay() { + let mut state = make_state(); + state.overlay = Overlay::Diff; + update(&mut state, Action::ToggleHelp); + assert!(state.help_open); + + // Back closes help first, leaving the PR overlay untouched. + update(&mut state, Action::Back); + assert!(!state.help_open); + assert_eq!(state.overlay, Overlay::Diff); +} + +#[test] +fn test_merge_filter_selects_and_composes_with_search() { + use ghdash::app::state::MergeFilter; + let mut state = make_state(); + let mut clean = make_pr("org-a", "repo1", 1, "clean one"); + clean.mergeable = Some("MERGEABLE".into()); + let mut conflict = make_pr("org-a", "repo1", 2, "conflict two"); + conflict.mergeable = Some("CONFLICTING".into()); + let mut unknown = make_pr("org-a", "repo1", 3, "unknown three"); + unknown.mergeable = Some("UNKNOWN".into()); + + update( + &mut state, + Action::DataLoaded(DataPayload::AllOpenPrs { + prs: vec![clean, conflict, unknown], + rate_limit: RateLimit::default(), + }), + ); + state.content_view = ContentView::AllOpenPrs; + + assert_eq!(state.current_pr_list().len(), 3); + + state.merge_filter = MergeFilter::Clean; + let clean_list = state.current_pr_list(); + assert_eq!(clean_list.len(), 1); + assert_eq!(clean_list[0].number, 1); + + state.merge_filter = MergeFilter::Conflicting; + assert_eq!(state.current_pr_list().len(), 1); + assert_eq!(state.current_pr_list()[0].number, 2); + + // Composes with search: Clean + a query excluding the clean PR -> empty. + state.merge_filter = MergeFilter::Clean; + state.search_query = "conflict".into(); + assert_eq!(state.current_pr_list().len(), 0); +}