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.
This commit is contained in:
Valdemar Lorenzen
2026-11-05 14:42:04 +01:00
parent e3d44f24c0
commit 1afc896cab
6 changed files with 93 additions and 32 deletions
+1 -1
View File
@@ -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. 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. 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.
+4
View File
@@ -1605,6 +1605,10 @@ impl DiffState {
} }
fn rebuild_picker_graph(&mut self) { 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 mut result = BTreeMap::new();
let Some(catalog) = &self.catalog else { let Some(catalog) = &self.catalog else {
self.picker_graph = result; self.picker_graph = result;
+1 -1
View File
@@ -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: :tab.new, :tab.previous, :tab.next, :tab.close, :tab.force_close
Tabs retain pane layouts and independent views; close protects exclusively displayed unsaved edits Tabs retain pane layouts and independent views; close protects exclusively displayed unsaved edits
Markdown links: :follow_link (primary caret; local files only) 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 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 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 Switching Before/After preserves the aligned cursor row and display column
+10 -10
View File
@@ -3905,20 +3905,15 @@ fn draw_diff(
let width = usize::from(results.width); let width = usize::from(results.width);
let change = metadata.change_id.as_str(); let change = metadata.change_id.as_str();
let change = change.get(..12).unwrap_or(change); 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!( let label = format!(
"{} {}{bookmark}{}{} {} {} {} {}", "{} {}{}{} {} {} {} {}",
if index == diff.selected { ">" } else { " " }, if index == diff.selected { ">" } else { " " },
prefix[0], prefix[0],
display_text(change), display_text(change),
before_badge, before_badge,
display_text(&metadata.author), display_text(&metadata.author),
display_text(&metadata.timestamp), display_text(&metadata.timestamp),
if metadata.bookmarks.is_empty() || !bookmark.is_empty() { if metadata.bookmarks.is_empty() {
String::new() String::new()
} else { } else {
format!("[{}]", display_text(&metadata.bookmarks)) format!("[{}]", display_text(&metadata.bookmarks))
@@ -3939,7 +3934,7 @@ fn draw_diff(
lines.push(Line::styled( lines.push(Line::styled(
visible_line( visible_line(
&format!( &format!(
"{} {}{}{}{bookmark}{} · {}", "{} {}{}{}{} · {}",
if index == diff.selected { ">" } else { " " }, if index == diff.selected { ">" } else { " " },
prefix[0], prefix[0],
if prefix[1].contains('~') { "~ " } else { "" }, if prefix[1].contains('~') { "~ " } else { "" },
@@ -3975,6 +3970,11 @@ fn draw_diff(
display_text(&revision.label), display_text(&revision.label),
padding = decoration_width.saturating_sub(symbol.width()), 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); let width = usize::from(results.width);
if row_height == 2 { if row_height == 2 {
lines.push(Line::styled(visible_line(&label, 0, width), style)); lines.push(Line::styled(visible_line(&label, 0, width), style));
@@ -3983,7 +3983,7 @@ fn draw_diff(
&format!( &format!(
"{:indent$}{}", "{:indent$}{}",
"", "",
display_text(&revision.description), display_text(description),
indent = 2 + decoration_width, indent = 2 + decoration_width,
), ),
0, 0,
@@ -3994,7 +3994,7 @@ fn draw_diff(
} else { } else {
lines.push(Line::styled( lines.push(Line::styled(
visible_line( visible_line(
&format!("{label} · {}", display_text(&revision.description)), &format!("{label} · {}", display_text(description)),
0, 0,
width, width,
), ),
-6
View File
@@ -1699,12 +1699,6 @@ fn picker_graph_preserves_forks_merges_and_marks_filtered_ancestry() {
state.category = 1; state.category = 1;
state.refilter(); state.refilter();
assert_eq!(state.entries().len(), 3); assert_eq!(state.entries().len(), 3);
let graph = state.picker_graph();
assert_eq!(
graph.keys().map(String::as_str).collect::<Vec<_>>(),
["merge", "root"]
);
assert!(graph["merge"][1].contains('~'));
assert_eq!( assert_eq!(
state state
.graph_revision(state.entries().nth(1).unwrap()) .graph_revision(state.entries().nth(1).unwrap())
+77 -14
View File
@@ -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(); let mut diff = DiffState::new();
diff.take_request(); diff.take_request();
diff.apply_result(1, Ok(crate::diff::Outcome::Catalog(catalog))); 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 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::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); render_diff(&mut terminal, &diff, &mut panels);
let text = screen_text(&terminal); let text = screen_text(&terminal);
assert!(text.contains("[main] change-a")); assert!(text.contains("topic-19@origin [Before]"), "{text}");
assert!(text.contains("[topic] change-a")); assert!(
assert_eq!(text.matches("change-a description").count(), 2); 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] #[test]