Lsp log selector fixes

Kirill Bulatov created

Change summary

crates/gpui2/src/elements/uniform_list.rs      |   4 
crates/language_tools2/src/lsp_log.rs          | 104 +++++++++++--------
crates/language_tools2/src/syntax_tree_view.rs |  22 +---
crates/ui2/src/components/context_menu.rs      |  23 ++-
4 files changed, 82 insertions(+), 71 deletions(-)

Detailed changes

crates/gpui2/src/elements/uniform_list.rs 🔗

@@ -127,7 +127,6 @@ impl Element for UniformList {
             .map(|s| s.item_size)
             .unwrap_or_else(|| self.measure_item(None, cx));
 
-        let element_id = self.interactivity.element_id.clone();
         let (layout_id, interactive) =
             self.interactivity
                 .layout(state.map(|s| s.interactive), cx, |style, cx| {
@@ -145,9 +144,6 @@ impl Element for UniformList {
                                         }
                                     });
 
-                            if element_id == Some(ElementId::Name("SyntaxTreeView".into())) {
-                                dbg!(known_dimensions, available_space.height);
-                            }
                             let height = match available_space.height {
                                 AvailableSpace::Definite(height) => desired_height.min(height),
                                 AvailableSpace::MinContent | AvailableSpace::MaxContent => {

crates/language_tools2/src/lsp_log.rs 🔗

@@ -520,6 +520,7 @@ impl LspLogView {
             self.editor_subscription = editor_subscription;
             cx.notify();
         }
+        cx.focus(&self.focus_handle);
     }
 
     fn show_rpc_trace_for_server(
@@ -560,6 +561,8 @@ impl LspLogView {
             self.editor_subscription = editor_subscription;
             cx.notify();
         }
+
+        cx.focus(&self.focus_handle);
     }
 
     fn toggle_rpc_trace_for_server(
@@ -753,6 +756,7 @@ impl Render for LspLogToolbarItemView {
                 let log_toolbar_view = log_toolbar_view.clone();
                 ContextMenu::build(cx, move |mut menu, cx| {
                     for (ix, row) in menu_rows.into_iter().enumerate() {
+                        let server_selected = Some(row.server_id) == current_server_id;
                         menu = menu
                             .header(format!(
                                 "{} ({})",
@@ -761,52 +765,65 @@ impl Render for LspLogToolbarItemView {
                             .entry(
                                 SERVER_LOGS,
                                 cx.handler_for(&log_view, move |view, cx| {
-                                    view.show_logs_for_server(row.server_id, cx)
+                                    view.show_logs_for_server(row.server_id, cx);
                                 }),
-                            )
-                            .custom_entry({
-                                let log_view = log_view.clone();
-                                let log_toolbar_view = log_toolbar_view.clone();
-                                move |cx| {
-                                    h_stack()
-                                        .w_full()
-                                        .justify_between()
-                                        .child(Label::new(RPC_MESSAGES))
-                                        .child(
-                                            Checkbox::new(
-                                                ix,
-                                                if row.rpc_trace_enabled {
-                                                    Selection::Selected
-                                                } else {
-                                                    Selection::Unselected
+                            );
+                        if server_selected && row.logs_selected {
+                            debug_assert_eq!(
+                                Some(ix * 3 + 1),
+                                menu.select_last(),
+                                "Could not scroll to a just added LSP menu item"
+                            );
+                        }
+
+                        menu = menu.custom_entry({
+                            let log_view = log_view.clone();
+                            let log_toolbar_view = log_toolbar_view.clone();
+                            move |cx| {
+                                h_stack()
+                                    .w_full()
+                                    .justify_between()
+                                    .child(Label::new(RPC_MESSAGES))
+                                    .child(
+                                        Checkbox::new(
+                                            ix,
+                                            if row.rpc_trace_enabled {
+                                                Selection::Selected
+                                            } else {
+                                                Selection::Unselected
+                                            },
+                                        )
+                                        .on_click(
+                                            cx.listener_for(
+                                                &log_toolbar_view,
+                                                move |view, selection, cx| {
+                                                    let enabled =
+                                                        matches!(selection, Selection::Selected);
+                                                    view.toggle_logging_for_server(
+                                                        row.server_id,
+                                                        enabled,
+                                                        cx,
+                                                    );
                                                 },
-                                            )
-                                            .on_click(
-                                                cx.listener_for(
-                                                    &log_toolbar_view,
-                                                    move |view, selection, cx| {
-                                                        let enabled = matches!(
-                                                            selection,
-                                                            Selection::Selected
-                                                        );
-                                                        view.toggle_logging_for_server(
-                                                            row.server_id,
-                                                            enabled,
-                                                            cx,
-                                                        );
-                                                    },
-                                                ),
                                             ),
-                                        )
-                                        .on_mouse_down(
-                                            MouseButton::Left,
-                                            cx.listener_for(&log_view, move |view, _, cx| {
-                                                view.show_rpc_trace_for_server(row.server_id, cx)
-                                            }),
-                                        )
-                                        .into_any_element()
-                                }
-                            })
+                                        ),
+                                    )
+                                    .on_mouse_down(
+                                        MouseButton::Left,
+                                        cx.listener_for(&log_view, move |view, _, cx| {
+                                            view.show_rpc_trace_for_server(row.server_id, cx);
+                                        }),
+                                    )
+                                    .into_any_element()
+                            }
+                        });
+                        if server_selected && row.rpc_trace_selected {
+                            debug_assert_eq!(
+                                Some(ix * 3 + 2),
+                                menu.select_last(),
+                                "Could not scroll to a just added LSP menu item"
+                            );
+                        }
                     }
                     menu
                 })
@@ -858,6 +875,7 @@ impl LspLogToolbarItemView {
                     log_view.show_logs_for_server(id, cx);
                     cx.notify();
                 }
+                cx.focus(&log_view.focus_handle);
             });
         }
         cx.notify();

crates/language_tools2/src/syntax_tree_view.rs 🔗

@@ -8,7 +8,7 @@ use gpui::{
 use language::{Buffer, OwnedSyntaxLayerInfo};
 use settings::Settings;
 use std::{mem, ops::Range};
-use theme::{Theme, ThemeSettings};
+use theme::{ActiveTheme, ThemeSettings};
 use tree_sitter::{Node, TreeCursor};
 use ui::{h_stack, popover_menu, ButtonLike, Color, ContextMenu, Label, LabelCommon, PopoverMenu};
 use workspace::{
@@ -275,14 +275,8 @@ impl SyntaxTreeView {
         Some(())
     }
 
-    fn render_node(
-        cursor: &TreeCursor,
-        depth: u32,
-        selected: bool,
-        editor_theme: &Theme,
-        _cx: &AppContext,
-    ) -> Div {
-        let editor_colors = editor_theme.colors();
+    fn render_node(cursor: &TreeCursor, depth: u32, selected: bool, cx: &AppContext) -> Div {
+        let colors = cx.theme().colors();
         let mut row = h_stack();
         if let Some(field_name) = cursor.field_name() {
             row = row.children([Label::new(field_name).color(Color::Info), Label::new(": ")]);
@@ -301,12 +295,12 @@ impl SyntaxTreeView {
                     .pl_1(),
             )
             .text_bg(if selected {
-                editor_colors.element_selected
+                colors.element_selected
             } else {
                 Hsla::default()
             })
             .pl(rems(depth as f32))
-            .hover(|style| style.bg(editor_colors.element_active));
+            .hover(|style| style.bg(colors.element_hover));
     }
 }
 
@@ -315,8 +309,6 @@ impl Render for SyntaxTreeView {
 
     fn render(&mut self, cx: &mut gpui::ViewContext<'_, Self>) -> Self::Element {
         let settings = ThemeSettings::get_global(cx);
-        let editor_theme = settings.active_theme.clone();
-        let editor_colors = editor_theme.colors();
         let line_height = cx
             .text_style()
             .line_height_in_pixels(settings.buffer_font_size(cx));
@@ -334,7 +326,6 @@ impl Render for SyntaxTreeView {
             .and_then(|buffer| buffer.active_layer.as_ref())
         {
             let layer = layer.clone();
-            let theme = editor_theme.clone();
             let list = uniform_list(
                 cx.view().clone(),
                 "SyntaxTreeView",
@@ -360,7 +351,6 @@ impl Render for SyntaxTreeView {
                                 &cursor,
                                 depth,
                                 Some(descendant_ix) == this.selected_descendant_ix,
-                                &theme,
                                 cx,
                             ));
                             descendant_ix += 1;
@@ -386,7 +376,7 @@ impl Render for SyntaxTreeView {
                     tree_view.handle_click(event.position.y, cx);
                 }),
             )
-            .text_bg(editor_colors.background);
+            .text_bg(cx.theme().colors().background);
 
             rendered = rendered.child(
                 canvas(move |bounds, cx| {

crates/ui2/src/components/context_menu.rs 🔗

@@ -122,6 +122,7 @@ impl ContextMenu {
         {
             (handler)(cx)
         }
+
         cx.emit(DismissEvent);
     }
 
@@ -135,14 +136,20 @@ impl ContextMenu {
         cx.notify();
     }
 
-    fn select_last(&mut self, _: &SelectLast, cx: &mut ViewContext<Self>) {
+    pub fn select_last(&mut self) -> Option<usize> {
         for (ix, item) in self.items.iter().enumerate().rev() {
             if item.is_selectable() {
                 self.selected_index = Some(ix);
-                cx.notify();
-                break;
+                return Some(ix);
             }
         }
+        None
+    }
+
+    fn handle_select_last(&mut self, _: &SelectLast, cx: &mut ViewContext<Self>) {
+        if self.select_last().is_some() {
+            cx.notify();
+        }
     }
 
     fn select_next(&mut self, _: &SelectNext, cx: &mut ViewContext<Self>) {
@@ -169,7 +176,7 @@ impl ContextMenu {
                 }
             }
         } else {
-            self.select_last(&Default::default(), cx);
+            self.handle_select_last(&Default::default(), cx);
         }
     }
 
@@ -195,7 +202,7 @@ impl ContextMenu {
                     .await;
                 this.update(&mut cx, |this, cx| {
                     cx.dispatch_action(action);
-                    this.cancel(&Default::default(), cx)
+                    this.cancel(&menu::Cancel, cx)
                 })
             })
             .detach_and_log_err(cx);
@@ -207,7 +214,7 @@ impl ContextMenu {
 
 impl ContextMenuItem {
     fn is_selectable(&self) -> bool {
-        matches!(self, Self::Entry { .. })
+        matches!(self, Self::Entry { .. } | Self::CustomEntry { .. })
     }
 }
 
@@ -219,10 +226,10 @@ impl Render for ContextMenu {
             v_stack()
                 .min_w(px(200.))
                 .track_focus(&self.focus_handle)
-                .on_mouse_down_out(cx.listener(|this, _, cx| this.cancel(&Default::default(), cx)))
+                .on_mouse_down_out(cx.listener(|this, _, cx| this.cancel(&menu::Cancel, cx)))
                 .key_context("menu")
                 .on_action(cx.listener(ContextMenu::select_first))
-                .on_action(cx.listener(ContextMenu::select_last))
+                .on_action(cx.listener(ContextMenu::handle_select_last))
                 .on_action(cx.listener(ContextMenu::select_next))
                 .on_action(cx.listener(ContextMenu::select_prev))
                 .on_action(cx.listener(ContextMenu::confirm))