From 1afc896cabf317999f54cc6b0f353f38612f7b75 Mon Sep 17 00:00:00 2001 From: Valdemar Lorenzen Date: Thu, 5 Nov 2026 14:42:04 +0100 Subject: [PATCH] fix: make bookmark diff selection a name-first list Bookmarks shared revision-graph rendering, allowing ancestry lanes and revision metadata to crowd out reference names. Restrict graph projection to Revisions and reuse flat bookmark rows with commit summaries. --- docs/editing.md | 2 +- src/diff.rs | 4 ++ src/main.rs | 2 +- src/tui.rs | 20 +++++----- tests/unit/diff.rs | 6 --- tests/unit/tui.rs | 91 +++++++++++++++++++++++++++++++++++++++------- 6 files changed, 93 insertions(+), 32 deletions(-) diff --git a/docs/editing.md b/docs/editing.md index 46ddd90..8f21d21 100644 --- a/docs/editing.md +++ b/docs/editing.md @@ -390,7 +390,7 @@ The project root is the working directory captured at startup. Scanning uses the Run `:diff` in Normal mode, choose **Before**, then **After**, and press Enter to compare. The command palette also exposes **Diff**; there is no default keybinding. Repository discovery starts at the active file's directory, or the startup project root for an unnamed buffer, and prefers jj over Git. -The floating selector has **Revisions / Bookmarks** tabs for jj, or **Commits / Branches / Tags** for Git. jj results use a log graph: `@` marks the recorded checkout revision, `◆` immutable revisions, and `○` other revisions. Each revision shows its short change ID, author, date, bookmarks, commit ID, and description. Bookmarks uses the same graph with unbookmarked revisions omitted; `~` marks skipped ancestry. Multiple bookmarks on one revision remain individually selectable, preserving their symbolic follow behavior. Narrow windows use compact single-line entries. +The floating selector has **Revisions / Bookmarks** tabs for jj, or **Commits / Branches / Tags** for Git. jj **Revisions** uses a log graph: `@` marks the recorded checkout revision, `◆` immutable revisions, and `○` other revisions; `~` marks skipped ancestry. Each revision shows its short change ID, author, date, bookmarks, commit ID, and description. **Bookmarks** is a flat, name-first list with the commit description underneath, without graph lanes or revision metadata crowding out names. Local and remote-tracking bookmarks remain individually selectable, including multiple names on one revision, preserving their symbolic follow behavior. Narrow windows use compact single-line entries. Both stages support case-insensitive fuzzy search by revision/change ID, commit ID, reference name, or description: `f1205pas` can find `sm/fin-1205-pilot-assignment-schema`. Space-separated tokens must all match, but can match different fields; a token cannot span unrelated fields. jj keeps graph order while filtering. Git ranks exact, prefix, and contiguous matches ahead of scattered subsequences, with names/IDs ahead of descriptions and catalogue order breaking ties. An empty query restores catalogue order. After choosing Before, its revision carries a **[Before]** badge instead of a separate summary row. It remains included through search and category changes, but scrolls normally. diff --git a/src/diff.rs b/src/diff.rs index 6aa2141..87fc160 100644 --- a/src/diff.rs +++ b/src/diff.rs @@ -1605,6 +1605,10 @@ impl DiffState { } fn rebuild_picker_graph(&mut self) { + if self.categories().get(self.category) != Some(&Category::Revisions) { + self.picker_graph.clear(); + return; + } let mut result = BTreeMap::new(); let Some(catalog) = &self.catalog else { self.picker_graph = result; diff --git a/src/main.rs b/src/main.rs index 40ac769..a5fb346 100644 --- a/src/main.rs +++ b/src/main.rs @@ -85,7 +85,7 @@ Restart: :restart closes clean buffers; unsaved buffers remain open and block re Tabs: :tab.new, :tab.previous, :tab.next, :tab.close, :tab.force_close Tabs retain pane layouts and independent views; close protects exclusively displayed unsaved edits Markdown links: :follow_link (primary caret; local files only) -Diff: :diff (Normal); floating jj log graph; choose Before then After; selected Before is marked inline +Diff: :diff (Normal); floating jj revision graph / bookmark list; choose Before then After; Before marked inline Before and historical/remote After stay read-only; Tab/Shift+Tab categories/Normal focus All text panes use native motions, selections and copy; read-only panes reject edits/saves Switching Before/After preserves the aligned cursor row and display column diff --git a/src/tui.rs b/src/tui.rs index 7fddf33..af71feb 100644 --- a/src/tui.rs +++ b/src/tui.rs @@ -3905,20 +3905,15 @@ fn draw_diff( let width = usize::from(results.width); let change = metadata.change_id.as_str(); let change = change.get(..12).unwrap_or(change); - let bookmark = if revision.category == crate::diff::Category::Bookmarks { - format!("[{}] ", display_text(&revision.label)) - } else { - String::new() - }; let label = format!( - "{} {}{bookmark}{}{} {} {} {} {}", + "{} {}{}{} {} {} {} {}", if index == diff.selected { ">" } else { " " }, prefix[0], display_text(change), before_badge, display_text(&metadata.author), display_text(&metadata.timestamp), - if metadata.bookmarks.is_empty() || !bookmark.is_empty() { + if metadata.bookmarks.is_empty() { String::new() } else { format!("[{}]", display_text(&metadata.bookmarks)) @@ -3939,7 +3934,7 @@ fn draw_diff( lines.push(Line::styled( visible_line( &format!( - "{} {}{}{}{bookmark}{} · {}", + "{} {}{}{}{} · {}", if index == diff.selected { ">" } else { " " }, prefix[0], if prefix[1].contains('~') { "~ " } else { "" }, @@ -3975,6 +3970,11 @@ fn draw_diff( display_text(&revision.label), padding = decoration_width.saturating_sub(symbol.width()), ); + let description = diff + .graph_revision(revision) + .map_or(revision.description.as_str(), |(_, metadata)| { + metadata.description.as_str() + }); let width = usize::from(results.width); if row_height == 2 { lines.push(Line::styled(visible_line(&label, 0, width), style)); @@ -3983,7 +3983,7 @@ fn draw_diff( &format!( "{:indent$}{}", "", - display_text(&revision.description), + display_text(description), indent = 2 + decoration_width, ), 0, @@ -3994,7 +3994,7 @@ fn draw_diff( } else { lines.push(Line::styled( visible_line( - &format!("{label} · {}", display_text(&revision.description)), + &format!("{label} · {}", display_text(description)), 0, width, ), diff --git a/tests/unit/diff.rs b/tests/unit/diff.rs index f6bc278..4a9349b 100644 --- a/tests/unit/diff.rs +++ b/tests/unit/diff.rs @@ -1699,12 +1699,6 @@ fn picker_graph_preserves_forks_merges_and_marks_filtered_ancestry() { state.category = 1; state.refilter(); assert_eq!(state.entries().len(), 3); - let graph = state.picker_graph(); - assert_eq!( - graph.keys().map(String::as_str).collect::>(), - ["merge", "root"] - ); - assert!(graph["merge"][1].contains('~')); assert_eq!( state .graph_revision(state.entries().nth(1).unwrap()) diff --git a/tests/unit/tui.rs b/tests/unit/tui.rs index d2edb84..c089d85 100644 --- a/tests/unit/tui.rs +++ b/tests/unit/tui.rs @@ -3594,17 +3594,6 @@ fn jj_picker_renders_graph_before_badge_and_compacts_without_losing_controls() { }, ); } - for name in ["main", "topic"] { - catalog.entries.push(Revision { - target: RevisionTarget::Bookmark { - name: name.into(), - remote: None, - }, - label: name.into(), - description: "change-a aaaa1111 change-a description".into(), - category: Category::Bookmarks, - }); - } let mut diff = DiffState::new(); diff.take_request(); diff.apply_result(1, Ok(crate::diff::Outcome::Catalog(catalog))); @@ -3652,12 +3641,86 @@ fn jj_picker_renders_graph_before_badge_and_compacts_without_losing_controls() { 1 ); } +} + +#[test] +fn jj_bookmark_picker_keeps_names_readable_through_search_scroll_and_selection() { + use crate::diff::{Category, RepoKind, Revision, RevisionGraph, RevisionTarget}; + let mut catalog = diff_catalog(); + catalog.kind = RepoKind::Jujutsu; + catalog.entries.truncate(1); + catalog.graph.insert( + "aaaa1111".into(), + RevisionGraph { + change_id: "change-a".into(), + description: "Fix recovery".into(), + author: "Revision author".into(), + current: true, + ..Default::default() + }, + ); + for index in 0..20 { + for remote in [None, Some("origin")] { + let name = format!("topic-{index:02}"); + catalog.entries.push(Revision { + target: RevisionTarget::Bookmark { + name: name.clone(), + remote: remote.map(str::to_owned), + }, + label: remote.map_or_else(|| name.clone(), |remote| format!("{name}@{remote}")), + description: "change-a aaaa1111 Fix recovery".into(), + category: Category::Bookmarks, + }); + } + } + let mut diff = DiffState::new(); + diff.take_request(); + diff.apply_result(1, Ok(crate::diff::Outcome::Catalog(catalog))); diff.handle(Input::Char('\t')); + diff.handle(Input::End); + let mut panels = WheelPanels::default(); + for (width, height) in [(100, 16), (44, 12)] { + let mut terminal = + Terminal::new(ratatui::backend::TestBackend::new(width, height)).unwrap(); + render_diff(&mut terminal, &diff, &mut panels); + let text = screen_text(&terminal); + assert!(text.contains("> topic-19@origin"), "{text}"); + assert!(!text.contains("Revision author"), "{text}"); + assert!(!text.contains("topic-00"), "{text}"); + } + for ch in "topic-19".chars() { + diff.handle(Input::Char(ch)); + } + assert_eq!(diff.entries().len(), 3); + diff.handle(Input::End); + diff.handle(Input::Enter); + assert_eq!( + diff.before.as_ref().unwrap().target, + RevisionTarget::Bookmark { + name: "topic-19".into(), + remote: Some("origin".into()), + } + ); + diff.handle(Input::End); + let mut terminal = Terminal::new(ratatui::backend::TestBackend::new(100, 16)).unwrap(); render_diff(&mut terminal, &diff, &mut panels); let text = screen_text(&terminal); - assert!(text.contains("[main] change-a")); - assert!(text.contains("[topic] change-a")); - assert_eq!(text.matches("change-a description").count(), 2); + assert!(text.contains("topic-19@origin [Before]"), "{text}"); + assert!( + text.lines().any(|line| line + .trim_start_matches([' ', '│']) + .starts_with("Fix recovery")), + "{text}" + ); + assert!(!text.contains("change-a aaaa1111"), "{text}"); + diff.handle(Input::Char('\t')); + assert_eq!(diff.entries().len(), 2); + assert_eq!( + diff.graph_revision(diff.entries().nth(1).unwrap()) + .unwrap() + .0, + "aaaa1111" + ); } #[test]