hl-valdemar/vamp
1
0
Fork 0
Code Issues Pull Requests Actions Packages Releases Wiki Activity

fix: retain selections and float local review editors

Browse Source
This commit is contained in:
Valdemar Lorenzen
2026-10-16 14:16:29 +02:00
parent d67b874b87
commit 4cc0453344
9 changed files with 336 additions and 68 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/editor.rs
+48 -6
View File
@@ -1773,6 +1773,7 @@ struct Pane {
struct Tab {
pane_id: usize,
pane_layout: PaneTree,
review_pane: Option<usize>,
file_picker: Option<FilePicker>,
diff: Option<DiffState>,
parked_diff: Option<DiffState>,
@@ -2060,6 +2061,7 @@ pub struct Editor {
next_pane_id: usize,
panes: BTreeMap<usize, Pane>,
pane_layout: PaneTree,
review_pane: Option<usize>,
tab_id: usize,
next_tab_id: usize,
tab_order: Vec<usize>,
@@ -2160,6 +2162,7 @@ impl Editor {
},
)]),
pane_layout: PaneTree::new(1),
review_pane: None,
tab_id: 1,
next_tab_id: 2,
tab_order: vec![1],
@@ -3485,6 +3488,7 @@ impl Editor {
let previous = Tab {
pane_id: std::mem::replace(&mut self.pane_id, next.pane_id),
pane_layout: std::mem::replace(&mut self.pane_layout, next.pane_layout),
review_pane: std::mem::replace(&mut self.review_pane, next.review_pane),
file_picker: std::mem::replace(&mut self.file_picker, next.file_picker),
diff: std::mem::replace(&mut self.diff, next.diff),
parked_diff: std::mem::replace(&mut self.parked_diff, next.parked_diff),
@@ -3533,6 +3537,7 @@ impl Editor {
Tab {
pane_id,
pane_layout: PaneTree::new(pane_id),
review_pane: None,
file_picker: None,
diff: None,
parked_diff: None,
@@ -3635,6 +3640,11 @@ impl Editor {
// No input dispatch or document copies: source documents stay unique in active/parked.
fn select_pane(&mut self, id: usize) {
let id = if self.review_pane == Some(id) && self.workspace_diff().is_none() {
self.panes[&id].origin.unwrap_or(id)
} else {
id
};
if id == self.pane_id {
return;
}
@@ -3660,13 +3670,13 @@ impl Editor {
assert!(self.panes.contains_key(&id), "rendered pane exists");
let focused = self.pane_id;
let tab = self.tab_id;
let target_tab = if self.pane_layout.contains(id) {
let target_tab = if self.pane_layout.contains(id) || self.review_pane == Some(id) {
tab
} else {
*self
.tabs
.iter()
.find(|(_, tab)| tab.pane_layout.contains(id))
.find(|(_, tab)| tab.pane_layout.contains(id) || tab.review_pane == Some(id))
.map(|(id, _)| id)
.expect("pane belongs to a tab")
};
@@ -3943,7 +3953,7 @@ impl Editor {
.panes
.keys()
.copied()
.filter(|id| self.pane_layout.contains(*id))
.filter(|id| self.pane_layout.contains(*id) || self.review_pane == Some(*id))
.collect();
let mut buffers = self.closing_buffers(&panes);
if let Some(resolver) = self.resolver() {
@@ -3992,6 +4002,9 @@ impl Editor {
}
fn close_pane(&mut self, force: bool) {
if self.dismiss_review_panel() {
return;
}
if self.resolver().is_some() {
self.close_tab(force);
return;
@@ -6685,7 +6698,9 @@ impl Editor {
}
/// Whole-row shape belongs to the active buffer, not the Visual binding context.
pub fn visual_linewise(&self) -> bool {
self.active.view.mode == Mode::Visual && self.active.view.linewise
(self.active.view.mode == Mode::Visual
|| (self.active.view.mode == Mode::Command && self.active.view.command_from_visual))
&& self.active.view.linewise
}
/// Inclusive selected rows, including an empty final row with an empty text range.
@@ -6700,9 +6715,11 @@ impl Editor {
})
}
/// Ordered, end-exclusive Visual ranges in document order; empty in other modes.
/// Ordered Visual ranges, including selections retained by command input.
pub fn selections(&self) -> Vec<Selection> {
if self.active.view.mode == Mode::Visual {
if self.active.view.mode == Mode::Visual
|| (self.active.view.mode == Mode::Command && self.active.view.command_from_visual)
{
self.active
.view
.targets
@@ -9351,6 +9368,31 @@ impl Editor {
}
return None;
}
if self.review_panel().is_some()
&& matches!(
entry.action,
Action::PaneSplitHorizontal
| Action::PaneSplitVertical
| Action::PaneFocusLeft
| Action::PaneFocusRight
| Action::PaneFocusUp
| Action::PaneFocusDown
| Action::TabNew
| Action::Edit
| Action::Buffers
| Action::Buffer
| Action::BufferNext
| Action::BufferPrevious
| Action::BufferDelete
| Action::BufferForceDelete
| Action::Diff
| Action::DiffClose
| Action::PullRequests
| Action::Resolve
)
{
self.dismiss_review_panel();
}
if matches!(
entry.action,
Action::Resolve
src/editor_review.rs
+44 -15
View File
@@ -22,6 +22,24 @@ impl Editor {
.map(|buffer| buffer.title.as_str())
}
pub fn review_panel(&self) -> Option<usize> {
self.review_pane
.filter(|pane| *pane == self.pane_id && self.workspace_diff().is_some())
}
pub(super) fn dismiss_review_panel(&mut self) -> bool {
let Some(panel) = self.review_panel() else {
return false;
};
let source = self.panes[&panel]
.origin
.expect("review panel has a source pane");
self.finish_pane_interaction();
self.select_pane(source);
self.bind_diff_buffer();
true
}
fn review_diff_mut(&mut self) -> Option<&mut DiffState> {
self.diff.as_mut().or(self.parked_diff.as_mut())
}
@@ -546,22 +564,24 @@ impl Editor {
.is_some_and(|binding| binding.owner == self.tab_id && binding.kind == kind)
})
.map(|buffer| buffer.id);
let panel = self.panes.iter().find_map(|(id, pane)| {
(self.pane_layout.contains(*id)
&& self.buffer(pane.buffer_id).is_some_and(|buffer| {
buffer
.review_buffer
.as_ref()
.is_some_and(|binding| binding.owner == self.tab_id)
}))
.then_some(*id)
});
if let Some(panel) = panel {
self.select_pane(panel);
let panel = if let Some(panel) = self.review_pane {
panel
} else {
self.select_pane(self.diff_owner.unwrap());
self.split_pane(SplitAxis::Vertical);
}
let id = self.next_pane_id;
self.next_pane_id += 1;
self.panes.insert(
id,
Pane {
buffer_id: self.active.id,
origin: self.diff_owner,
views: BTreeMap::from([(self.active.id, self.active.view.inherit())]),
},
);
self.review_pane = Some(id);
id
};
self.panes.get_mut(&panel).unwrap().origin = self.diff_owner;
self.select_pane(panel);
if let Some(id) = existing {
self.switch_buffer(id);
if kind == ReviewText::Overview {
@@ -586,6 +606,15 @@ impl Editor {
}
pub(super) fn handle_review_overview(&mut self, input: &Input) -> bool {
if *input == Input::Escape
&& self.active.view.mode == Mode::Normal
&& self.palette.is_none()
&& self.sequence_pending().is_none()
&& self.count.is_none()
&& self.dismiss_review_panel()
{
return true;
}
if *input == Input::Enter
&& self.active.view.mode == Mode::Normal
&& self.palette.is_none()
src/main.rs
+2 -1
View File
@@ -86,7 +86,8 @@ Diff: :diff (Normal); choose Before then After; Working checkout After edits liv
Local reviews: :diff.comment at caret or on a Visual/Visual Line selection
:diff.review.edit/delete [ID], :diff.review.next/previous, :diff.review.overview
:diff.review.summary; :diff.review.verdict looks_good|changes_requested|none
:diff.review.save PATH / :diff.review.open PATH; :w in review panes saves local JSON
:diff.review.save PATH / :diff.review.open PATH; :w in floating review editors saves local JSON
Normal Esc or :pane.close dismisses the floating editor and retains drafts
Snapshot anchors stay immutable; stale comments lose gutter markers; no publishing
Pull requests: :pull_requests [REMOTE]; GitHub/GitLab/Gitea Open/Closed/All tabs, type to search
Enter opens the immutable PR head against its merge base; API auth uses provider tokens or gh/glab
src/tui.rs
+229 -39
View File
@@ -678,18 +678,23 @@ impl BufferViewports {
.resolver()
.is_some_and(|resolver| resolver.view == ResolverView::History);
let multiple = !resolver_resolution && editor.pane_layout().len() > 1;
let (workspace, dock) = match editor.file_picker() {
Some(picker) if picker.presentation == FilePresentation::Docked => dock_layout(
full_workspace,
if editor.picker_diff().is_some() {
DockSide::Right
} else {
editor.file_explorer_side()
},
picker.focused,
),
_ => (full_workspace, None),
};
let layout_pane = editor
.review_panel()
.and_then(|pane| editor.pane_origin(pane))
.unwrap_or(focused);
let (workspace, dock) =
editor.with_render_pane(layout_pane, |view| match view.file_picker() {
Some(picker) if picker.presentation == FilePresentation::Docked => dock_layout(
full_workspace,
if view.picker_diff().is_some() {
DockSide::Right
} else {
view.file_explorer_side()
},
picker.focused,
),
_ => (full_workspace, None),
});
editor.set_pane_bounds(pane_rect(workspace));
self.rects.clear();
if resolver_history {
@@ -737,9 +742,6 @@ impl BufferViewports {
.retain(|id, _| view.has_buffer(*id));
let (top, left) = self.offsets.entry(view.buffer_id()).or_default();
let syntax = self.syntax.entry(view.buffer_id()).or_default();
let leaf = view.review_panel_title().map_or(leaf, |title| {
resolver_document_panel(frame, leaf, title, id == focused)
});
draw_document(
frame,
view,
@@ -767,22 +769,25 @@ impl BufferViewports {
self.panels.begin_frame();
self.document.geometry = None;
}
if let Some(panel) = dock
&& let Some(picker) = editor.file_picker()
{
self.panels.picker_scroll.distance = editor.scroll_distance();
draw_file_list(
frame,
picker,
panel,
editor.file_navigation_hint(),
&mut self.panels,
editor.picker_diff().and_then(|diff| {
diff.comparison
.as_ref()
.map(|comparison| (comparison.files.as_slice(), &diff.reviewed_files))
}),
);
if let Some(panel) = dock {
self.activate(layout_pane, editor);
editor.with_render_pane(layout_pane, |view| {
if let Some(picker) = view.file_picker() {
self.panels.picker_scroll.distance = view.scroll_distance();
draw_file_list(
frame,
picker,
panel,
view.file_navigation_hint(),
&mut self.panels,
view.picker_diff().and_then(|diff| {
diff.comparison.as_ref().map(|comparison| {
(comparison.files.as_slice(), &diff.reviewed_files)
})
}),
);
}
});
if let Some(geometry) = &mut self.panels.diff {
geometry.regions[0] = (panel, Focus::Files);
editor.set_diff_pane_bounds(
@@ -827,6 +832,44 @@ impl BufferViewports {
}
}
}
if let Some(panel) = editor.review_panel() {
let source = editor.pane_origin(panel);
let area = self
.rects
.iter()
.find(|(id, _)| Some(*id) == source)
.map_or(full_workspace, |(_, area)| *area);
let area = floating_panel(area);
self.rects.insert(0, (panel, area));
self.activate(panel, editor);
self.panels.workspace_managed = true;
editor.with_render_pane(panel, |view| {
frame.render_widget(Clear, area);
let block = Block::default()
.borders(Borders::ALL)
.border_style(Style::default().fg(Color::Cyan))
.title(display_text(
view.review_panel_title().unwrap_or("Local review"),
));
let content = block.inner(area);
frame.render_widget(block, area);
self.activate_document(view.buffer_id());
let (top, left) = self.offsets.entry(view.buffer_id()).or_default();
let syntax = self.syntax.entry(view.buffer_id()).or_default();
draw_document(
frame,
view,
content,
full_workspace,
top,
left,
syntax,
&mut self.document,
&mut self.panels,
);
});
self.panels.workspace_managed = false;
}
if let Some(diff) = editor.diff().filter(|diff| diff.phase != Phase::View) {
draw_diff(
frame,
@@ -1097,10 +1140,18 @@ impl BufferViewports {
return true;
}
let focused = session.editor().pane_id();
let Some(target) = self.rects.iter().find_map(|(id, area)| {
let target = self.rects.iter().find_map(|(id, area)| {
area.contains(Position::new(mouse.column, mouse.row))
.then_some(*id)
}) else {
});
let target = if let Some(panel) = session.editor().review_panel() {
if target != Some(panel) {
return true;
}
panel
} else if let Some(target) = target {
target
} else {
return false;
};
if let Some(resolver) = session
@@ -1729,11 +1780,13 @@ fn chrome_items<'a>(
continue;
}
let mut text = match name.as_str() {
"mode" => Cow::Owned(if editor.visual_linewise() {
"VISUAL LINE".into()
} else {
editor.mode().to_string()
}),
"mode" => Cow::Owned(
if editor.mode() == Mode::Visual && editor.visual_linewise() {
"VISUAL LINE".into()
} else {
editor.mode().to_string()
},
),
"path" => Cow::Owned(chrome_title(
"{path}{modified}",
editor.tab_index() + 1,
@@ -13962,7 +14015,7 @@ end
editor.apply_diff_result(
id,
Ok(crate::diff::Outcome::Comparison(crate::diff::Comparison {
root: std::path::PathBuf::new(),
root: editor.project_root().to_path_buf(),
editable_paths: Default::default(),
before: "baseline".into(),
after: "historic".into(),
@@ -13983,6 +14036,143 @@ end
terminal.get_cursor_position().unwrap()
}
#[test]
fn command_input_keeps_characterwise_and_linewise_selection_highlights() {
for context in ["document", "split", "unified"] {
for selector in ['v', 'V'] {
let mut editor = if context == "document" {
let mut editor = Editor::open(None, vec![]).unwrap();
editor
.apply(vec![crate::editor::Effect::Replace(
"alpha beta\nsecond line".into(),
)])
.unwrap();
editor
} else {
let mut editor = readonly_diff_editor(syntax_diff_file(
"file.txt",
"alpha beta\nold line",
"alpha beta\nnew line",
));
if context == "unified" {
editor.execute("diff.view.toggle");
}
editor.handle(Input::Char('\t'));
editor
};
let mut terminal =
Terminal::new(ratatui::backend::TestBackend::new(140, 24)).unwrap();
let mut views = BufferViewports::default();
terminal
.draw(|frame| views.draw(frame, &mut editor))
.unwrap();
let source = terminal.get_cursor_position().unwrap();
editor.handle(Input::Char(selector));
editor.handle(Input::Right);
let ranges = editor.selections();
editor.handle(Input::Char(':'));
for ch in "diff.comment".chars() {
editor.handle(Input::Char(ch));
}
terminal
.draw(|frame| views.draw(frame, &mut editor))
.unwrap();
assert_eq!(editor.mode(), Mode::Command);
assert_eq!(editor.selections(), ranges);
let buffer = terminal.backend().buffer();
assert_eq!(
buffer[(source.x, source.y)].bg,
Color::Blue,
"{context} {selector}"
);
assert_eq!(
buffer[(source.x + if selector == 'V' { 9 } else { 1 }, source.y)].bg,
Color::Blue,
"{context} {selector}"
);
editor.handle(Input::Escape);
assert_eq!(editor.selections(), ranges);
}
}
}
#[test]
fn review_editor_floats_over_the_diff_without_changing_workspace_layout() {
for unified in [false, true] {
let mut editor = readonly_diff_editor(syntax_diff_file(
"file.txt",
"old first\nold last",
"new first\nnew last",
));
if unified {
editor.execute("diff.view.toggle");
}
editor.handle(Input::Char('\t'));
let owner = editor.pane_id();
let source_text = editor.context().text;
let mut terminal = Terminal::new(ratatui::backend::TestBackend::new(160, 30)).unwrap();
let mut views = BufferViewports::default();
terminal
.draw(|frame| views.draw(frame, &mut editor))
.unwrap();
let source_area = views
.rects
.iter()
.find(|(pane, _)| *pane == owner)
.unwrap()
.1;
editor.execute("diff.comment");
let panel = editor.review_panel().expect("focused floating review");
assert_eq!(editor.pane_layout().len(), 1);
assert!(!editor.pane_layout().contains(panel));
terminal
.draw(|frame| views.draw(frame, &mut editor))
.unwrap();
assert_eq!(views.rects[0], (panel, floating_panel(source_area)));
assert_eq!(
views
.rects
.iter()
.find(|(pane, _)| *pane == owner)
.unwrap()
.1,
source_area
);
let area = views.rects[0].1;
let cursor = terminal.get_cursor_position().unwrap();
assert!(area.contains(cursor));
for input in [
Input::Char('i'),
Input::Paste("Retained draft".into()),
Input::Escape,
] {
editor.handle(input);
}
assert_eq!(editor.context().text, "Retained draft");
assert_eq!(editor.review_panel(), Some(panel));
editor.handle(Input::Escape);
assert!(editor.review_panel().is_none());
assert_eq!(editor.pane_id(), owner);
assert_eq!(editor.context().text, source_text);
editor.execute("diff.review.edit 1");
assert_eq!(
editor.context().text,
"Retained draft",
"{}",
editor.message()
);
assert_eq!(editor.review_panel(), Some(panel));
editor.execute("tab.new");
editor.execute("tab.previous");
editor.execute("diff.review.edit 1");
assert_eq!(editor.review_panel(), Some(panel));
assert_eq!(editor.context().text, "Retained draft");
editor.execute("pane.close");
assert!(editor.review_panel().is_none());
assert_eq!(editor.pane_layout().len(), 1);
}
}
#[test]
fn scope_projection_keeps_unified_wrapped_caret_and_selection_source_only() {
let before = format!(