fix: preserve selector text input and close on escape
This commit is contained in:
+62
-66
@@ -164,12 +164,6 @@ pub enum DisplayRow {
|
||||
Fold { start: usize, end: usize },
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
struct SelectorPosition {
|
||||
category: usize,
|
||||
query: String,
|
||||
selected: RevisionTarget,
|
||||
}
|
||||
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
|
||||
enum RequestKind {
|
||||
Catalog,
|
||||
@@ -214,7 +208,6 @@ pub struct DiffState {
|
||||
request_kind: Option<RequestKind>,
|
||||
visible: Vec<usize>,
|
||||
search_labels: Vec<String>,
|
||||
before_position: Option<SelectorPosition>,
|
||||
max_before: usize,
|
||||
max_after: usize,
|
||||
after_widths: Vec<usize>,
|
||||
@@ -255,7 +248,6 @@ impl DiffState {
|
||||
request_kind: Some(RequestKind::Catalog),
|
||||
visible: Vec::new(),
|
||||
search_labels: Vec::new(),
|
||||
before_position: None,
|
||||
max_before: 0,
|
||||
max_after: 0,
|
||||
after_widths: Vec::new(),
|
||||
@@ -468,44 +460,20 @@ impl DiffState {
|
||||
}
|
||||
}
|
||||
|
||||
/// Handles modal input; closing is an explicit editor action.
|
||||
/// Handles selector filtering and comparison navigation; the editor owns selector closing.
|
||||
pub(crate) fn handle(&mut self, input: Input) {
|
||||
if input == Input::Escape {
|
||||
if self.phase == Phase::View
|
||||
&& self.focus == Focus::Files
|
||||
&& let Some(files) = &mut self.files
|
||||
&& files.searching
|
||||
&& files.view == FileView::Tree
|
||||
{
|
||||
files.finish_search();
|
||||
return;
|
||||
}
|
||||
if self.phase == Phase::View {
|
||||
if self.focus == Focus::Files
|
||||
&& let Some(files) = &mut self.files
|
||||
{
|
||||
if files.searching && files.view == FileView::Tree {
|
||||
files.finish_search();
|
||||
} else {
|
||||
files.focused = false;
|
||||
self.files_visible = files.presentation == FilePresentation::Docked;
|
||||
self.focus = self.first_text_focus();
|
||||
}
|
||||
return;
|
||||
}
|
||||
self.pending = None;
|
||||
self.request_kind = None;
|
||||
self.loading = false;
|
||||
self.error = None;
|
||||
if self.phase == Phase::Before {
|
||||
self.query.clear();
|
||||
self.refilter();
|
||||
return;
|
||||
}
|
||||
self.phase = Phase::Before;
|
||||
self.after = None;
|
||||
if let Some(position) = self.before_position.take() {
|
||||
self.category = position.category;
|
||||
self.query = position.query;
|
||||
self.refilter();
|
||||
self.select_target(&position.selected);
|
||||
}
|
||||
return;
|
||||
}
|
||||
@@ -562,11 +530,6 @@ impl DiffState {
|
||||
} else if let RevisionTarget::ListRemote(remote) = &revision.target {
|
||||
self.queue(Operation::ListRemote(remote.clone()));
|
||||
} else if self.phase == Phase::Before {
|
||||
self.before_position = Some(SelectorPosition {
|
||||
category: self.category,
|
||||
query: self.query.clone(),
|
||||
selected: revision.target.clone(),
|
||||
});
|
||||
self.before = Some(revision);
|
||||
self.after = None;
|
||||
self.phase = Phase::After;
|
||||
@@ -2106,32 +2069,66 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn after_escape_restores_before_query_and_selected_revision_and_cancels_request() {
|
||||
fn selector_escape_closes_each_stage_and_rejects_late_results() {
|
||||
for after in [false, true] {
|
||||
let mut editor = editor();
|
||||
editor
|
||||
.apply(vec![Effect::Replace("unsaved source".into())])
|
||||
.unwrap();
|
||||
let buffer = editor.buffer_id();
|
||||
open(&mut editor);
|
||||
let request = if after {
|
||||
Some(choose_pair(&mut editor))
|
||||
} else {
|
||||
text(&mut editor, "query");
|
||||
None
|
||||
};
|
||||
editor.handle(Input::Escape);
|
||||
assert!(editor.diff().is_none());
|
||||
assert_eq!(editor.buffer_id(), buffer);
|
||||
assert_eq!(editor.lines(), ["unsaved source"]);
|
||||
assert!(editor.dirty());
|
||||
if let Some(id) = request {
|
||||
assert!(!editor.apply_diff_result(id, Ok(Outcome::Comparison(comparison()))));
|
||||
assert!(editor.diff().is_none());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn selector_printable_input_precedes_normal_bindings_but_not_selection_overrides() {
|
||||
for after in [false, true] {
|
||||
let mut editor = editor();
|
||||
open(&mut editor);
|
||||
if after {
|
||||
editor.handle(Input::Enter);
|
||||
}
|
||||
editor
|
||||
.bind("normal", "x", Some("core.command".into()))
|
||||
.unwrap();
|
||||
for ch in ":_| x".chars() {
|
||||
editor.handle(Input::Char(ch));
|
||||
assert_eq!(editor.mode(), Mode::Normal);
|
||||
assert_eq!(editor.pane_layout().len(), 1);
|
||||
}
|
||||
assert_eq!(editor.diff().unwrap().query, ":_| x");
|
||||
editor.handle(Input::Ctrl('u'));
|
||||
editor
|
||||
.bind("selection", ":", Some("core.down".into()))
|
||||
.unwrap();
|
||||
editor.handle(Input::Char(':'));
|
||||
assert_eq!(editor.diff().unwrap().selected, 1);
|
||||
assert!(editor.diff().unwrap().query.is_empty());
|
||||
}
|
||||
let mut editor = editor();
|
||||
open(&mut editor);
|
||||
editor.handle(Input::Char('\t'));
|
||||
text(&mut editor, "topic");
|
||||
editor.handle(Input::Down);
|
||||
editor.handle(Input::Enter);
|
||||
assert!(editor.diff().unwrap().query.is_empty());
|
||||
editor.handle(Input::Enter);
|
||||
let (id, _) = editor.take_diff_request().unwrap();
|
||||
editor.handle(Input::Ctrl('p'));
|
||||
editor.handle(Input::Char(':'));
|
||||
assert_eq!(editor.palette().unwrap().query, ":");
|
||||
assert_eq!(editor.mode(), Mode::Normal);
|
||||
editor.handle(Input::Escape);
|
||||
let state = editor.diff().unwrap();
|
||||
assert_eq!(state.phase, Phase::Before);
|
||||
assert_eq!(state.category, 1);
|
||||
assert_eq!(state.query, "topic");
|
||||
assert_eq!(state.selected, 1);
|
||||
assert!(!state.loading);
|
||||
assert!(!editor.apply_diff_result(id, Ok(Outcome::Comparison(comparison()))));
|
||||
editor.handle(Input::Enter);
|
||||
assert_eq!(editor.diff().unwrap().phase, Phase::After);
|
||||
editor.handle(Input::Escape);
|
||||
editor.handle(Input::Escape);
|
||||
assert_eq!(editor.diff().unwrap().phase, Phase::Before);
|
||||
assert!(editor.diff().unwrap().query.is_empty());
|
||||
editor.execute("diff.close");
|
||||
assert!(editor.diff().is_none());
|
||||
assert!(editor.palette().is_none());
|
||||
assert!(editor.diff().is_some());
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -2560,8 +2557,7 @@ mod tests {
|
||||
buffers
|
||||
);
|
||||
editor.handle(Input::Escape);
|
||||
assert!(editor.diff().is_some());
|
||||
editor.execute("diff.close");
|
||||
assert!(editor.diff().is_none());
|
||||
assert_eq!(editor.mode(), Mode::Normal);
|
||||
open(&mut editor);
|
||||
let id = choose_pair(&mut editor);
|
||||
|
||||
+103
-42
@@ -2628,6 +2628,32 @@ impl Editor {
|
||||
}
|
||||
|
||||
fn handle_diff(&mut self, input: Input) -> Option<Invocation> {
|
||||
if self.palette.is_some()
|
||||
|| self.diff_prompt_mode.is_some()
|
||||
|| self.startup_diagnostic.is_some()
|
||||
|| self.hover.is_some()
|
||||
{
|
||||
return self.handle_input(input);
|
||||
}
|
||||
let phase = self.diff.as_ref().expect("active diff").phase;
|
||||
let files_focused =
|
||||
phase == Phase::View && self.diff.as_ref().unwrap().focus == Focus::Files;
|
||||
if phase != Phase::View && input == Input::Escape {
|
||||
return self.execute_action(Action::DiffClose.entry(), "");
|
||||
}
|
||||
let text_focused = phase != Phase::View
|
||||
|| (files_focused
|
||||
&& self
|
||||
.diff
|
||||
.as_ref()
|
||||
.and_then(|diff| diff.files.as_ref())
|
||||
.is_some_and(|files| files.searching || files.view == FileView::List));
|
||||
if text_focused && matches!(input, Input::Char(ch) if !ch.is_control()) {
|
||||
if !self.handle_selection_binding(&input) {
|
||||
self.diff.as_mut().unwrap().handle(input);
|
||||
}
|
||||
return None;
|
||||
}
|
||||
if self
|
||||
.bindings
|
||||
.get(&self.binding_mode())
|
||||
@@ -2655,9 +2681,7 @@ impl Editor {
|
||||
{
|
||||
return self.handle_input(input);
|
||||
}
|
||||
if self.palette.is_some()
|
||||
|| self.diff_prompt_mode.is_some()
|
||||
|| (self.diff_interacting() && self.active.view.mode != Mode::Normal)
|
||||
if (self.diff_interacting() && self.active.view.mode != Mode::Normal)
|
||||
|| self.count.is_some()
|
||||
|| self.sequence_pending().is_some()
|
||||
|| self.g_pending
|
||||
@@ -2666,9 +2690,6 @@ impl Editor {
|
||||
{
|
||||
return self.handle_input(input);
|
||||
}
|
||||
let phase = self.diff.as_ref().expect("active diff").phase;
|
||||
let files_focused =
|
||||
phase == Phase::View && self.diff.as_ref().unwrap().focus == Focus::Files;
|
||||
if self.diff_interacting() {
|
||||
if matches!(input, Input::Char('[' | ']')) {
|
||||
let focus = self.diff_buffer_focus().unwrap();
|
||||
@@ -2704,29 +2725,7 @@ impl Editor {
|
||||
if files_focused && self.handle_tree_binding(&input) {
|
||||
return None;
|
||||
}
|
||||
if !self.diff_interacting() {
|
||||
match self.selection_bindings.get(&input) {
|
||||
Some(Binding::Action(Action::Down)) => {
|
||||
self.diff.as_mut().unwrap().navigate(true);
|
||||
return None;
|
||||
}
|
||||
Some(Binding::Action(Action::Up)) => {
|
||||
self.diff.as_mut().unwrap().navigate(false);
|
||||
return None;
|
||||
}
|
||||
Some(Binding::Disabled) => return None,
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
if files_focused
|
||||
&& matches!(input, Input::Char(ch) if !ch.is_control())
|
||||
&& self
|
||||
.diff
|
||||
.as_ref()
|
||||
.and_then(|diff| diff.files.as_ref())
|
||||
.is_some_and(|files| files.searching || files.view == FileView::List)
|
||||
{
|
||||
self.diff.as_mut().unwrap().handle(input);
|
||||
if !self.diff_interacting() && self.handle_selection_binding(&input) {
|
||||
return None;
|
||||
}
|
||||
if self.sequence_pending().is_some()
|
||||
@@ -2758,7 +2757,6 @@ impl Editor {
|
||||
return self.handle_input(input);
|
||||
}
|
||||
let escape = input == Input::Escape;
|
||||
let back = input == Input::Escape && phase != Phase::View;
|
||||
let return_focus = files_focused
|
||||
.then(|| self.diff_bound.map(|bound| bound.2))
|
||||
.flatten();
|
||||
@@ -2772,9 +2770,6 @@ impl Editor {
|
||||
diff.focus = focus;
|
||||
diff.normalize_focus();
|
||||
}
|
||||
if back {
|
||||
self.outstanding_diff_request = None;
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
@@ -4286,6 +4281,10 @@ impl Editor {
|
||||
} else {
|
||||
palette.selected - 1
|
||||
};
|
||||
} else if self.palette.is_none()
|
||||
&& let Some(diff) = &mut self.diff
|
||||
{
|
||||
diff.navigate(next);
|
||||
}
|
||||
true
|
||||
}
|
||||
@@ -8721,6 +8720,70 @@ mod tests {
|
||||
assert_eq!(editor.active.view.targets, targets);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn focused_text_fields_keep_printable_normal_bindings_as_text() {
|
||||
let directory = Directory::new();
|
||||
fs::write(directory.path("sample.txt"), "text").unwrap();
|
||||
for field in [
|
||||
"floating",
|
||||
"docked",
|
||||
"diff_tree",
|
||||
"diff_list",
|
||||
"palette",
|
||||
"command",
|
||||
"search",
|
||||
] {
|
||||
let mut editor = editor();
|
||||
editor.project_root = directory.0.clone();
|
||||
editor
|
||||
.bind("normal", "x", Some("core.pane.split_vertical".into()))
|
||||
.unwrap();
|
||||
match field {
|
||||
"floating" => editor.open_file_picker(FilePresentation::Floating),
|
||||
"docked" => {
|
||||
editor.open_file_picker(FilePresentation::Docked);
|
||||
editor.handle(Input::Char('i'));
|
||||
}
|
||||
"diff_tree" | "diff_list" => {
|
||||
historical_diff(
|
||||
&mut editor,
|
||||
&directory.0,
|
||||
vec!["before".into()],
|
||||
vec!["after".into()],
|
||||
);
|
||||
if field == "diff_list" {
|
||||
editor.set_file_explorer_view("list").unwrap();
|
||||
}
|
||||
editor.open_file_picker(FilePresentation::Docked);
|
||||
if field == "diff_tree" {
|
||||
editor.handle(Input::Char('i'));
|
||||
}
|
||||
}
|
||||
"palette" => {
|
||||
editor.handle(Input::Ctrl('p'));
|
||||
}
|
||||
"command" => {
|
||||
editor.handle(Input::Char(':'));
|
||||
}
|
||||
"search" => {
|
||||
editor.handle(Input::Char('/'));
|
||||
}
|
||||
_ => unreachable!(),
|
||||
}
|
||||
let document = editor.lines().to_vec();
|
||||
editor.handle(Input::Char(':'));
|
||||
editor.handle(Input::Char('x'));
|
||||
let query = match field {
|
||||
"palette" => editor.palette().unwrap().query.as_str(),
|
||||
"command" | "search" => editor.command_line(),
|
||||
_ => editor.file_picker().unwrap().query.as_str(),
|
||||
};
|
||||
assert_eq!(query, ":x", "{field}");
|
||||
assert_eq!(editor.pane_layout().len(), 1, "{field}");
|
||||
assert_eq!(editor.lines(), document, "{field}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn empty_notification_does_not_swallow_escape_but_cursor_diagnostic_does() {
|
||||
let mut editor = editor();
|
||||
@@ -9637,12 +9700,12 @@ mod tests {
|
||||
] {
|
||||
editor.handle(input);
|
||||
}
|
||||
editor.handle(Input::Escape);
|
||||
assert!(editor.diff().is_some());
|
||||
type_text(&mut editor, ":unknown\n");
|
||||
editor.handle(Input::Ctrl('p'));
|
||||
editor.handle(Input::Escape);
|
||||
editor.execute("diff.close");
|
||||
assert!(editor.diff().is_some());
|
||||
editor.handle(Input::Escape);
|
||||
assert!(editor.diff().is_none());
|
||||
assert_eq!(editor.mode(), Mode::Visual);
|
||||
assert_eq!(editor.carets(), carets);
|
||||
assert_eq!(editor.selections(), selections);
|
||||
@@ -9665,14 +9728,12 @@ mod tests {
|
||||
let caret = editor.cursor();
|
||||
editor.execute("diff");
|
||||
editor.handle(Input::Paste("ignored".into()));
|
||||
editor.handle(Input::Escape);
|
||||
assert!(editor.diff().is_some());
|
||||
type_text(&mut editor, ":unfinished");
|
||||
editor.handle(Input::Escape);
|
||||
editor
|
||||
.apply(vec![Effect::Message("message without edits".into())])
|
||||
.unwrap();
|
||||
editor.execute("diff.close");
|
||||
editor.handle(Input::Escape);
|
||||
assert!(editor.diff().is_none());
|
||||
assert_eq!(editor.mode(), Mode::Insert);
|
||||
assert_eq!(editor.cursor(), caret);
|
||||
type_text(&mut editor, "finish");
|
||||
@@ -9822,7 +9883,7 @@ mod tests {
|
||||
editor.handle(Input::Char('q'));
|
||||
assert_eq!(editor.diff().unwrap().query, "q");
|
||||
editor.handle(Input::Escape);
|
||||
assert_eq!(editor.diff().unwrap().query, "");
|
||||
assert!(editor.diff().is_none());
|
||||
assert!(editor.notification_visible());
|
||||
}
|
||||
|
||||
@@ -10406,7 +10467,7 @@ mod tests {
|
||||
editor.execute("diff");
|
||||
type_text(&mut editor, ":diffclose\n");
|
||||
assert!(editor.diff().is_some());
|
||||
type_text(&mut editor, ":diff.close\n");
|
||||
editor.execute("diff.close");
|
||||
assert!(editor.diff().is_none());
|
||||
}
|
||||
|
||||
|
||||
+2
-2
@@ -51,9 +51,9 @@ Diff: :diff (Normal); choose Before then After; Working checkout After edits liv
|
||||
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
|
||||
Type to search IDs/refs; navigation skips omitted lines without expanding excerpts
|
||||
Selector queries own printable keys, including :; text navigation skips omitted lines
|
||||
Normal [/] explicitly expands context; Insert brackets remain literal text
|
||||
:diff.close exits explicitly; Esc cancels modes/prompts/filtering without closing the diff
|
||||
Esc closes Before/After selectors; :diff.close exits an opened comparison explicitly
|
||||
Remote refs/history load into temporary comparison repos without changing active refs/checkout
|
||||
Changed files: normal picker/explorer bindings, Ctrl+w close; A/M/D/R status and rename paths
|
||||
|
||||
|
||||
+4
-2
@@ -965,7 +965,7 @@ end
|
||||
config.bind "D", config.core.buffer_delete
|
||||
config.bind "Z", config.core.buffer_force_delete
|
||||
config.bind "R", config.core.diff
|
||||
config.bind "T", config.core.diff.close
|
||||
config.bind "ctrl+t", config.core.diff.close
|
||||
config.bind "H", config.core.word_wrap.toggle
|
||||
raise "mutable diff actions" unless config.core.frozen? && config.core.diff.frozen? &&
|
||||
config.core.diff.close.frozen? && config.core.diff.name.frozen? &&
|
||||
@@ -982,8 +982,10 @@ end"#,
|
||||
assert!(!review.editor().word_wrap());
|
||||
review.handle(Input::Char('R'));
|
||||
review.handle(Input::Escape);
|
||||
assert!(review.editor().diff().is_none());
|
||||
review.handle(Input::Char('R'));
|
||||
assert!(review.editor().diff().is_some());
|
||||
review.handle(Input::Char('T'));
|
||||
review.handle(Input::Ctrl('t'));
|
||||
assert!(review.editor().diff().is_none());
|
||||
for (key, dirty, quit, saved) in [
|
||||
('Q', false, true, false),
|
||||
|
||||
+8
-6
@@ -4833,10 +4833,6 @@ mod tests {
|
||||
diff.handle(Input::Enter);
|
||||
render_diff(&mut terminal, &diff, &mut panels);
|
||||
assert!(screen_text(&terminal).contains("Choose After · Before: commit-19"));
|
||||
diff.handle(Input::Escape);
|
||||
render_diff(&mut terminal, &diff, &mut panels);
|
||||
assert!(screen_text(&terminal).contains("> needle"));
|
||||
assert!(screen_text(&terminal).contains("> commit-19"));
|
||||
diff.handle(Input::Ctrl('u'));
|
||||
diff.handle(Input::Char('\t'));
|
||||
render_diff(&mut terminal, &diff, &mut panels);
|
||||
@@ -5649,7 +5645,6 @@ mod tests {
|
||||
fn floating_inputs_share_workspace_geometry_across_documents_splits_and_diffs() {
|
||||
for context in ["document", "split", "diff", "split-diff", "selector"] {
|
||||
for prefix in [':', '/'] {
|
||||
// A selector's slash belongs to its revision filter, not native search.
|
||||
if context == "selector" && prefix == '/' {
|
||||
continue;
|
||||
}
|
||||
@@ -5670,7 +5665,14 @@ mod tests {
|
||||
editor.apply_diff_result(id, Ok(crate::diff::Outcome::Catalog(diff_catalog())));
|
||||
}
|
||||
editor.set_message("saved notification");
|
||||
editor.handle(Input::Char(prefix));
|
||||
if context == "selector" {
|
||||
editor
|
||||
.bind("normal", "ctrl+t", Some("core.command".into()))
|
||||
.unwrap();
|
||||
editor.handle(Input::Ctrl('t'));
|
||||
} else {
|
||||
editor.handle(Input::Char(prefix));
|
||||
}
|
||||
let query = format!("{}TAIL", "界e\u{301}👩🚀".repeat(30));
|
||||
for ch in query.chars() {
|
||||
editor.handle(Input::Char(ch));
|
||||
|
||||
Reference in New Issue
Block a user