diff --git a/crates/settings_ui/src/keybindings.rs b/crates/settings_ui/src/keybindings.rs index af096f4ce1..d5ac253fb8 100644 --- a/crates/settings_ui/src/keybindings.rs +++ b/crates/settings_ui/src/keybindings.rs @@ -22,9 +22,9 @@ use settings::{BaseKeymap, KeybindSource, KeymapFile, SettingsAssets}; use util::ResultExt; use ui::{ - ActiveTheme as _, App, Banner, BorrowAppContext, ContextMenu, IconButtonShape, Modal, - ModalFooter, ModalHeader, ParentElement as _, Render, Section, SharedString, Styled as _, - Tooltip, Window, prelude::*, + ActiveTheme as _, App, Banner, BorrowAppContext, ContextMenu, IconButtonShape, Indicator, + Modal, ModalFooter, ModalHeader, ParentElement as _, Render, Section, SharedString, + Styled as _, Tooltip, Window, prelude::*, }; use ui_input::SingleLineInput; use workspace::{ @@ -179,14 +179,29 @@ impl FilterState { #[derive(Debug, Default, PartialEq, Eq, Clone, Hash)] struct ActionMapping { - keystroke_text: SharedString, + keystrokes: Vec, context: Option, } +#[derive(Debug)] +struct KeybindConflict { + first_conflict_index: usize, + remaining_conflict_amount: usize, +} + +impl KeybindConflict { + fn from_iter<'a>(mut indices: impl Iterator) -> Option { + indices.next().map(|index| Self { + first_conflict_index: *index, + remaining_conflict_amount: indices.count(), + }) + } +} + #[derive(Default)] struct ConflictState { conflicts: Vec, - action_keybind_mapping: HashMap>, + keybind_mapping: HashMap>, } impl ConflictState { @@ -197,7 +212,7 @@ impl ConflictState { .iter() .enumerate() .filter(|(_, binding)| { - !binding.keystroke_text.is_empty() + binding.keystrokes().is_some() && binding .source .as_ref() @@ -217,27 +232,26 @@ impl ConflictState { .flatten() .copied() .collect(), - action_keybind_mapping, + keybind_mapping: action_keybind_mapping, } } fn conflicting_indices_for_mapping( &self, - action_mapping: ActionMapping, + action_mapping: &ActionMapping, keybind_idx: usize, - ) -> Option> { - self.action_keybind_mapping - .get(&action_mapping) + ) -> Option { + self.keybind_mapping + .get(action_mapping) .and_then(|indices| { - let mut indices = indices.iter().filter(|&idx| *idx != keybind_idx).peekable(); - indices.peek().is_some().then(|| indices.copied().collect()) + KeybindConflict::from_iter(indices.iter().filter(|&idx| *idx != keybind_idx)) }) } - fn will_conflict(&self, action_mapping: ActionMapping) -> Option> { - self.action_keybind_mapping - .get(&action_mapping) - .and_then(|indices| indices.is_empty().not().then_some(indices.clone())) + fn will_conflict(&self, action_mapping: &ActionMapping) -> Option { + self.keybind_mapping + .get(action_mapping) + .and_then(|indices| KeybindConflict::from_iter(indices.iter())) } fn has_conflict(&self, candidate_idx: &usize) -> bool { @@ -267,7 +281,7 @@ struct KeymapEditor { selected_index: Option, context_menu: Option<(Entity, Point, Subscription)>, previous_edit: Option, - humanized_action_names: HashMap<&'static str, SharedString>, + humanized_action_names: HumanizedActionNameCache, show_hover_menus: bool, } @@ -332,14 +346,6 @@ impl KeymapEditor { }) .detach(); - let humanized_action_names = - HashMap::from_iter(cx.all_action_names().into_iter().map(|&action_name| { - ( - action_name, - command_palette::humanize_action_name(action_name).into(), - ) - })); - let mut this = Self { workspace, keybindings: vec![], @@ -356,8 +362,8 @@ impl KeymapEditor { selected_index: None, context_menu: None, previous_edit: None, - humanized_action_names, search_query_debounce: None, + humanized_action_names: HumanizedActionNameCache::new(cx), show_hover_menus: true, }; @@ -383,6 +389,24 @@ impl KeymapEditor { } } + fn filter_on_selected_binding_keystrokes(&mut self, cx: &mut Context) { + let Some(selected_binding) = self.selected_binding() else { + return; + }; + + let keystrokes = selected_binding + .keystrokes() + .map(Vec::from) + .unwrap_or_default(); + + self.filter_state = FilterState::All; + self.search_mode = SearchMode::KeyStroke { exact_match: true }; + + self.keystroke_editor.update(cx, |editor, cx| { + editor.set_keystrokes(keystrokes, cx); + }); + } + fn on_query_changed(&mut self, cx: &mut Context) { let action_query = self.current_action_query(cx); let keystroke_query = self.current_keystroke_query(cx); @@ -523,6 +547,7 @@ impl KeymapEditor { fn process_bindings( json_language: Arc, zed_keybind_context_language: Arc, + humanized_action_names: &HumanizedActionNameCache, cx: &mut App, ) -> (Vec, Vec) { let key_bindings_ptr = cx.key_bindings(); @@ -570,12 +595,14 @@ impl KeymapEditor { let action_docs = action_documentation.get(action_name).copied(); let index = processed_bindings.len(); - let string_match_candidate = StringMatchCandidate::new(index, &action_name); + let humanized_action_name = humanized_action_names.get(action_name); + let string_match_candidate = StringMatchCandidate::new(index, &humanized_action_name); processed_bindings.push(ProcessedKeybinding { keystroke_text: keystroke_text.into(), ui_key_binding, action_name, action_arguments, + humanized_action_name, action_docs, action_schema: action_schema.get(action_name).cloned(), context: Some(context), @@ -587,12 +614,14 @@ impl KeymapEditor { let empty = SharedString::new_static(""); for action_name in unmapped_action_names.into_iter() { let index = processed_bindings.len(); - let string_match_candidate = StringMatchCandidate::new(index, &action_name); + let humanized_action_name = humanized_action_names.get(action_name); + let string_match_candidate = StringMatchCandidate::new(index, &humanized_action_name); processed_bindings.push(ProcessedKeybinding { keystroke_text: empty.clone(), ui_key_binding: None, action_name, action_arguments: None, + humanized_action_name, action_docs: action_documentation.get(action_name).copied(), action_schema: action_schema.get(action_name).cloned(), context: None, @@ -612,15 +641,15 @@ impl KeymapEditor { load_keybind_context_language(workspace.clone(), cx).await; let (action_query, keystroke_query) = this.update(cx, |this, cx| { - let (key_bindings, string_match_candidates) = - Self::process_bindings(json_language, zed_keybind_context_language, cx); + let (key_bindings, string_match_candidates) = Self::process_bindings( + json_language, + zed_keybind_context_language, + &this.humanized_action_names, + cx, + ); this.keybinding_conflict_state = ConflictState::new(&key_bindings); - if !this.keybinding_conflict_state.any_conflicts() { - this.filter_state = FilterState::All; - } - this.keybindings = key_bindings; this.string_match_candidates = Arc::new(string_match_candidates); this.matches = this @@ -751,10 +780,6 @@ impl KeymapEditor { ) { let weak = cx.weak_entity(); self.context_menu = self.selected_binding().map(|selected_binding| { - let key_strokes = selected_binding - .keystrokes() - .map(Vec::from) - .unwrap_or_default(); let selected_binding_has_no_context = selected_binding .context .as_ref() @@ -784,17 +809,9 @@ impl KeymapEditor { Box::new(CopyContext), ) .entry("Show matching keybindings", None, { - let weak = weak.clone(); - let key_strokes = key_strokes.clone(); - move |_, cx| { weak.update(cx, |this, cx| { - this.filter_state = FilterState::All; - this.search_mode = SearchMode::KeyStroke { exact_match: true }; - - this.keystroke_editor.update(cx, |editor, cx| { - editor.set_keystrokes(key_strokes.clone(), cx); - }); + this.filter_on_selected_binding_keystrokes(cx); }) .ok(); } @@ -826,6 +843,24 @@ impl KeymapEditor { self.context_menu.is_some() } + fn render_no_matches_hint(&self, _window: &mut Window, _cx: &App) -> AnyElement { + let hint = match (self.filter_state, &self.search_mode) { + (FilterState::Conflicts, _) => { + if self.keybinding_conflict_state.any_conflicts() { + "No conflicting keybinds found that match the provided query" + } else { + "No conflicting keybinds found" + } + } + (FilterState::All, SearchMode::KeyStroke { .. }) => { + "No keybinds found matching the entered keystrokes" + } + (FilterState::All, SearchMode::Normal) => "No matches found for the provided query", + }; + + Label::new(hint).color(Color::Muted).into_any_element() + } + fn select_next(&mut self, _: &menu::SelectNext, window: &mut Window, cx: &mut Context) { self.show_hover_menus = false; if let Some(selected) = self.selected_index { @@ -1064,11 +1099,35 @@ impl KeymapEditor { } } +struct HumanizedActionNameCache { + cache: HashMap<&'static str, SharedString>, +} + +impl HumanizedActionNameCache { + fn new(cx: &App) -> Self { + let cache = HashMap::from_iter(cx.all_action_names().into_iter().map(|&action_name| { + ( + action_name, + command_palette::humanize_action_name(action_name).into(), + ) + })); + Self { cache } + } + + fn get(&self, action_name: &'static str) -> SharedString { + match self.cache.get(action_name) { + Some(name) => name.clone(), + None => action_name.into(), + } + } +} + #[derive(Clone)] struct ProcessedKeybinding { keystroke_text: SharedString, ui_key_binding: Option, action_name: &'static str, + humanized_action_name: SharedString, action_arguments: Option, action_docs: Option<&'static str>, action_schema: Option, @@ -1079,7 +1138,7 @@ struct ProcessedKeybinding { impl ProcessedKeybinding { fn get_action_mapping(&self) -> ActionMapping { ActionMapping { - keystroke_text: self.keystroke_text.clone(), + keystrokes: self.keystrokes().map(Vec::from).unwrap_or_default(), context: self .context .as_ref() @@ -1223,38 +1282,39 @@ impl Render for KeymapEditor { window.dispatch_action(ToggleKeystrokeSearch.boxed_clone(), cx); }), ) - .when(self.keybinding_conflict_state.any_conflicts(), |this| { - this.child( - IconButton::new("KeymapEditorConflictIcon", IconName::Warning) - .shape(ui::IconButtonShape::Square) - .tooltip({ - let filter_state = self.filter_state; + .child( + IconButton::new("KeymapEditorConflictIcon", IconName::Warning) + .shape(ui::IconButtonShape::Square) + .when(self.keybinding_conflict_state.any_conflicts(), |this| { + this.indicator(Indicator::dot().color(Color::Warning)) + }) + .tooltip({ + let filter_state = self.filter_state; - move |window, cx| { - Tooltip::for_action( - match filter_state { - FilterState::All => "Show Conflicts", - FilterState::Conflicts => "Hide Conflicts", - }, - &ToggleConflictFilter, - window, - cx, - ) - } - }) - .selected_icon_color(Color::Warning) - .toggle_state(matches!( - self.filter_state, - FilterState::Conflicts - )) - .on_click(|_, window, cx| { - window.dispatch_action( - ToggleConflictFilter.boxed_clone(), + move |window, cx| { + Tooltip::for_action( + match filter_state { + FilterState::All => "Show Conflicts", + FilterState::Conflicts => "Hide Conflicts", + }, + &ToggleConflictFilter, + window, cx, - ); - }), - ) - }), + ) + } + }) + .selected_icon_color(Color::Warning) + .toggle_state(matches!( + self.filter_state, + FilterState::Conflicts + )) + .on_click(|_, window, cx| { + window.dispatch_action( + ToggleConflictFilter.boxed_clone(), + cx, + ); + }), + ), ) .when_some( match self.search_mode { @@ -1310,13 +1370,17 @@ impl Render for KeymapEditor { Table::new() .interactable(&self.table_interaction_state) .striped() + .empty_table_callback({ + let this = cx.entity(); + move |window, cx| this.read(cx).render_no_matches_hint(window, cx) + }) .column_widths([ - rems(2.5), - rems(16.), - rems(16.), - rems(16.), - rems(32.), - rems(8.), + DefiniteLength::Absolute(AbsoluteLength::Pixels(px(40.))), + DefiniteLength::Fraction(0.25), + DefiniteLength::Fraction(0.20), + DefiniteLength::Fraction(0.14), + DefiniteLength::Fraction(0.45), + DefiniteLength::Fraction(0.08), ]) .header(["", "Action", "Arguments", "Keystrokes", "Context", "Source"]) .uniform_list( @@ -1393,10 +1457,9 @@ impl Render for KeymapEditor { .id(("keymap action", index)) .child({ if action_name != gpui::NoAction.name() { - this.humanized_action_names - .get(action_name) - .cloned() - .unwrap_or(action_name.into()) + binding + .humanized_action_name + .clone() .into_any_element() } else { const NULL: SharedString = @@ -1606,7 +1669,7 @@ impl RenderOnce for SyntaxHighlightedText { runs.push(text_style.to_run(text.len() - offset)); } - return StyledText::new(text).with_runs(runs); + StyledText::new(text).with_runs(runs) } } @@ -1621,8 +1684,8 @@ impl InputError { Self::Warning(message.into()) } - fn error(message: impl Into) -> Self { - Self::Error(message.into()) + fn error(error: anyhow::Error) -> Self { + Self::Error(error.to_string().into()) } fn content(&self) -> &SharedString { @@ -1630,10 +1693,6 @@ impl InputError { InputError::Warning(content) | InputError::Error(content) => content, } } - - fn is_warning(&self) -> bool { - matches!(self, InputError::Warning(_)) - } } struct KeybindingEditorModal { @@ -1766,17 +1825,14 @@ impl KeybindingEditorModal { } } - fn set_error(&mut self, error: InputError, cx: &mut Context) -> bool { + fn set_error(&mut self, error: InputError, cx: &mut Context) { if self .error .as_ref() - .is_some_and(|old_error| old_error.is_warning() && *old_error == error) + .is_none_or(|old_error| *old_error != error) { - false - } else { self.error = Some(error); cx.notify(); - true } } @@ -1818,66 +1874,62 @@ impl KeybindingEditorModal { Ok(Some(context)) } - fn save(&mut self, cx: &mut Context) { + fn save_or_display_error(&mut self, cx: &mut Context) { + self.save(cx).map_err(|err| self.set_error(err, cx)).ok(); + } + + fn save(&mut self, cx: &mut Context) -> Result<(), InputError> { let existing_keybind = self.editing_keybind.clone(); let fs = self.fs.clone(); let tab_size = cx.global::().json_tab_size(); - let new_keystrokes = match self.validate_keystrokes(cx) { - Err(err) => { - self.set_error(InputError::error(err.to_string()), cx); - return; - } - Ok(keystrokes) => keystrokes, - }; - let new_context = match self.validate_context(cx) { - Err(err) => { - self.set_error(InputError::error(err.to_string()), cx); - return; - } - Ok(context) => context, - }; + let new_keystrokes = self + .validate_keystrokes(cx) + .map_err(InputError::error)? + .into_iter() + .map(remove_key_char) + .collect::>(); - let new_action_args = match self.validate_action_arguments(cx) { - Err(input_err) => { - self.set_error(InputError::error(input_err.to_string()), cx); - return; - } - Ok(input) => input, - }; + let new_context = self.validate_context(cx).map_err(InputError::error)?; + let new_action_args = self + .validate_action_arguments(cx) + .map_err(InputError::error)?; let action_mapping = ActionMapping { - keystroke_text: ui::text_for_keystrokes(&new_keystrokes, cx).into(), - context: new_context.as_ref().map(Into::into), + keystrokes: new_keystrokes, + context: new_context.map(SharedString::from), }; let conflicting_indices = if self.creating { self.keymap_editor .read(cx) .keybinding_conflict_state - .will_conflict(action_mapping) + .will_conflict(&action_mapping) } else { self.keymap_editor .read(cx) .keybinding_conflict_state - .conflicting_indices_for_mapping(action_mapping, self.editing_keybind_idx) + .conflicting_indices_for_mapping(&action_mapping, self.editing_keybind_idx) }; - if let Some(conflicting_indices) = conflicting_indices { - let first_conflicting_index = conflicting_indices[0]; + + conflicting_indices.map(|KeybindConflict { + first_conflict_index, + remaining_conflict_amount, + }| + { let conflicting_action_name = self .keymap_editor .read(cx) .keybindings - .get(first_conflicting_index) + .get(first_conflict_index) .map(|keybind| keybind.action_name); let warning_message = match conflicting_action_name { Some(name) => { - let confliction_action_amount = conflicting_indices.len() - 1; - if confliction_action_amount > 0 { + if remaining_conflict_amount > 0 { format!( "Your keybind would conflict with the \"{}\" action and {} other bindings", - name, confliction_action_amount + name, remaining_conflict_amount ) } else { format!("Your keybind would conflict with the \"{}\" action", name) @@ -1886,23 +1938,26 @@ impl KeybindingEditorModal { None => { log::info!( "Could not find action in keybindings with index {}", - first_conflicting_index + first_conflict_index ); "Your keybind would conflict with other actions".to_string() } }; - if self.set_error(InputError::warning(warning_message), cx) { - return; + let warning = InputError::warning(warning_message); + if self.error.as_ref().is_some_and(|old_error| *old_error == warning) { + Ok(()) + } else { + Err(warning) } - } + }).unwrap_or(Ok(()))?; let create = self.creating; let status_toast = StatusToast::new( format!( "Saved edits to the {} action.", - command_palette::humanize_action_name(&self.editing_keybind.action_name) + &self.editing_keybind.humanized_action_name ), cx, move |this, _cx| { @@ -1924,8 +1979,7 @@ impl KeybindingEditorModal { if let Err(err) = save_keybinding_update( create, existing_keybind, - &new_keystrokes, - new_context.as_deref(), + &action_mapping, new_action_args.as_deref(), &fs, tab_size, @@ -1933,17 +1987,11 @@ impl KeybindingEditorModal { .await { this.update(cx, |this, cx| { - this.set_error(InputError::error(err.to_string()), cx); + this.set_error(InputError::error(err), cx); }) .log_err(); } else { this.update(cx, |this, cx| { - let action_mapping = ActionMapping { - keystroke_text: ui::text_for_keystrokes(new_keystrokes.as_slice(), cx) - .into(), - context: new_context.map(SharedString::from), - }; - this.keymap_editor.update(cx, |keymap, cx| { keymap.previous_edit = Some(PreviousEdit::Keybinding { action_mapping, @@ -1960,6 +2008,8 @@ impl KeybindingEditorModal { } }) .detach(); + + Ok(()) } fn key_context(&self) -> KeyContext { @@ -1982,7 +2032,7 @@ impl KeybindingEditorModal { } fn confirm(&mut self, _: &menu::Confirm, _window: &mut Window, cx: &mut Context) { - self.save(cx); + self.save_or_display_error(cx); } fn cancel(&mut self, _: &menu::Cancel, _window: &mut Window, cx: &mut Context) { @@ -1990,11 +2040,17 @@ impl KeybindingEditorModal { } } +fn remove_key_char(Keystroke { modifiers, key, .. }: Keystroke) -> Keystroke { + Keystroke { + modifiers, + key, + ..Default::default() + } +} + impl Render for KeybindingEditorModal { fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { let theme = cx.theme().colors(); - let action_name = - command_palette::humanize_action_name(&self.editing_keybind.action_name).to_string(); v_flex() .w(rems(34.)) @@ -2014,7 +2070,9 @@ impl Render for KeybindingEditorModal { .gap_0p5() .border_b_1() .border_color(theme.border_variant) - .child(Label::new(action_name)) + .child(Label::new( + self.editing_keybind.humanized_action_name.clone(), + )) .when_some(self.editing_keybind.action_docs, |this, docs| { this.child( Label::new(docs).size(LabelSize::Small).color(Color::Muted), @@ -2085,7 +2143,7 @@ impl Render for KeybindingEditorModal { ) .child(Button::new("save-btn", "Save").on_click(cx.listener( |this, _event, _window, cx| { - this.save(cx); + this.save_or_display_error(cx); }, ))), ), @@ -2273,8 +2331,7 @@ async fn load_keybind_context_language( async fn save_keybinding_update( create: bool, existing: ProcessedKeybinding, - new_keystrokes: &[Keystroke], - new_context: Option<&str>, + action_mapping: &ActionMapping, new_args: Option<&str>, fs: &Arc, tab_size: usize, @@ -2301,8 +2358,8 @@ async fn save_keybinding_update( }; let source = settings::KeybindUpdateTarget { - context: new_context, - keystrokes: new_keystrokes, + context: action_mapping.context.as_ref().map(|a| &***a), + keystrokes: &action_mapping.keystrokes, action_name: &existing.action_name, action_arguments: new_args, }; @@ -2772,7 +2829,7 @@ impl Render for KeystrokeInput { IconName::PlayFilled }; - return h_flex() + h_flex() .id("keystroke-input") .track_focus(&self.outer_focus_handle) .py_2() @@ -2895,7 +2952,7 @@ impl Render for KeystrokeInput { this.clear_keystrokes(&ClearKeystrokes, window, cx); })), ), - ); + ) } } diff --git a/crates/settings_ui/src/ui_components/table.rs b/crates/settings_ui/src/ui_components/table.rs index 98dd738765..6ea59cd2f4 100644 --- a/crates/settings_ui/src/ui_components/table.rs +++ b/crates/settings_ui/src/ui_components/table.rs @@ -40,6 +40,10 @@ impl TableContents { TableContents::UniformList(data) => data.row_count, } } + + fn is_empty(&self) -> bool { + self.len() == 0 + } } pub struct TableInteractionState { @@ -375,6 +379,7 @@ pub struct Table { interaction_state: Option>, column_widths: Option<[Length; COLS]>, map_row: Option AnyElement>>, + empty_table_callback: Option AnyElement>>, } impl Table { @@ -388,6 +393,7 @@ impl Table { interaction_state: None, column_widths: None, map_row: None, + empty_table_callback: None, } } @@ -460,6 +466,15 @@ impl Table { self.map_row = Some(Rc::new(callback)); self } + + /// Provide a callback that is invoked when the table is rendered without any rows + pub fn empty_table_callback( + mut self, + callback: impl Fn(&mut Window, &mut App) -> AnyElement + 'static, + ) -> Self { + self.empty_table_callback = Some(Rc::new(callback)); + self + } } fn base_cell_style(width: Option, cx: &App) -> Div { @@ -582,6 +597,7 @@ impl RenderOnce for Table { }; let width = self.width; + let no_rows_rendered = self.rows.is_empty(); let table = div() .when_some(width, |this, width| this.w(width)) @@ -662,6 +678,21 @@ impl RenderOnce for Table { }) }), ) + .when_some( + no_rows_rendered + .then_some(self.empty_table_callback) + .flatten(), + |this, callback| { + this.child( + h_flex() + .size_full() + .p_3() + .items_start() + .justify_center() + .child(callback(window, cx)), + ) + }, + ) .when_some( width.and(interaction_state.as_ref()), |this, interaction_state| {