From 09b6e3f2a6a3b422d04934c5c0fc88ad8dc72741 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 15 Aug 2024 17:45:23 -0700 Subject: [PATCH] Improve workflow step view (#16329) * Improve the tab title: give it an icon, and indicate the step index. * Display the line number ranges that the symbols resolve to. * Don't open duplicate tabs for the same step Release Notes: - N/A --- crates/assistant/src/assistant_panel.rs | 21 +++++++---- crates/assistant/src/context.rs | 25 +++++++------ crates/assistant/src/workflow/step_view.rs | 42 +++++++++++++++++----- 3 files changed, 63 insertions(+), 25 deletions(-) diff --git a/crates/assistant/src/assistant_panel.rs b/crates/assistant/src/assistant_panel.rs index fb2fc805a3..ac2e54ab12 100644 --- a/crates/assistant/src/assistant_panel.rs +++ b/crates/assistant/src/assistant_panel.rs @@ -2407,7 +2407,7 @@ impl ContextEditor { let Some(step) = self .context .read(cx) - .workflow_step_for_range(step_range.clone()) + .workflow_step_for_range(step_range.clone(), cx) else { return; }; @@ -2630,13 +2630,22 @@ impl ContextEditor { .ok()??; let context = self.context.read(cx); let language_registry = context.language_registry(); - let step = context.workflow_step_for_range(step_range)?; - let view = cx.new_view(|cx| { - WorkflowStepView::new(self.context.clone(), step, language_registry, cx) - }); + let step = context.workflow_step_for_range(step_range, cx)?; + let context = self.context.clone(); cx.deref_mut().defer(move |cx| { pane.update(cx, |pane, cx| { - pane.add_item(Box::new(view), true, true, None, cx); + let existing_item = pane + .items_of_type::() + .find(|item| *item.read(cx).step() == step.downgrade()); + if let Some(item) = existing_item { + if let Some(index) = pane.index_for_item(&item) { + pane.activate_item(index, true, true, cx); + } + } else { + let view = cx + .new_view(|cx| WorkflowStepView::new(context, step, language_registry, cx)); + pane.add_item(Box::new(view), true, true, None, cx); + } }); }); None diff --git a/crates/assistant/src/context.rs b/crates/assistant/src/context.rs index 5fa816a817..a49a624c95 100644 --- a/crates/assistant/src/context.rs +++ b/crates/assistant/src/context.rs @@ -872,14 +872,20 @@ impl Context { pub fn workflow_step_for_range( &self, range: Range, + cx: &AppContext, ) -> Option> { - Some( - self.workflow_steps - .iter() - .find(|step| step.range == range)? - .step - .clone(), - ) + let buffer = self.buffer.read(cx); + let index = self.workflow_step_index_for_range(&range, buffer).ok()?; + Some(self.workflow_steps[index].step.clone()) + } + + pub fn workflow_step_index_for_range( + &self, + tagged_range: &Range, + buffer: &text::BufferSnapshot, + ) -> Result { + self.workflow_steps + .binary_search_by(|probe| probe.range.cmp(&tagged_range, buffer)) } pub fn pending_slash_commands(&self) -> &[PendingSlashCommand] { @@ -1126,9 +1132,8 @@ impl Context { ..buffer.anchor_before(step_end_tag_start_ix); // Check if a step with the same range already exists - let existing_step_index = self - .workflow_steps - .binary_search_by(|probe| probe.range.cmp(&tagged_range, &buffer)); + let existing_step_index = + self.workflow_step_index_for_range(&tagged_range, &buffer); if let Err(ix) = existing_step_index { new_edit_steps.push(( diff --git a/crates/assistant/src/workflow/step_view.rs b/crates/assistant/src/workflow/step_view.rs index 96d2ab710b..8c07526fd4 100644 --- a/crates/assistant/src/workflow/step_view.rs +++ b/crates/assistant/src/workflow/step_view.rs @@ -11,10 +11,11 @@ use gpui::{ }; use language::{language_settings::SoftWrap, Anchor, Buffer, LanguageRegistry}; use std::{ops::DerefMut, sync::Arc}; +use text::OffsetRangeExt; use theme::ActiveTheme as _; use ui::{ - h_flex, v_flex, ButtonCommon as _, ButtonLike, ButtonStyle, Color, InteractiveElement as _, - Label, LabelCommon as _, + h_flex, v_flex, ButtonCommon as _, ButtonLike, ButtonStyle, Color, Icon, IconName, + InteractiveElement as _, Label, LabelCommon as _, }; use workspace::{ item::{self, Item}, @@ -145,6 +146,10 @@ impl WorkflowStepView { } } + pub fn step(&self) -> &WeakModel { + &self.step + } + fn render_result(&mut self, cx: &mut ViewContext) -> Option { let step = self.step.upgrade()?; let result = step.read(cx).resolution.as_ref()?; @@ -155,14 +160,17 @@ impl WorkflowStepView { .child(result.title.clone()) .children(result.suggestion_groups.iter().filter_map( |(buffer, suggestion_groups)| { - let path = buffer.read(cx).file().map(|f| f.path()); + let buffer = buffer.read(cx); + let path = buffer.file().map(|f| f.path()); + let snapshot = buffer.snapshot(); v_flex() .mb_2() .border_b_1() .children(path.map(|path| format!("path: {}", path.display()))) .children(suggestion_groups.iter().map(|group| { - v_flex().pl_2().children(group.suggestions.iter().map( - |suggestion| { + v_flex().pt_2().pl_2().children( + group.suggestions.iter().map(|suggestion| { + let range = suggestion.range().to_point(&snapshot); v_flex() .children( suggestion.description().map(|desc| { @@ -173,8 +181,13 @@ impl WorkflowStepView { .children(suggestion.symbol_path().map( |path| format!("symbol path: {}", path.0), )) - }, - )) + .child(format!( + "lines: {} - {}", + range.start.row + 1, + range.end.row + 1 + )) + }), + ) })) .into() }, @@ -247,8 +260,19 @@ impl FocusableView for WorkflowStepView { impl Item for WorkflowStepView { type Event = EditorEvent; - fn tab_content_text(&self, _cx: &WindowContext) -> Option { - Some("workflow step".into()) + fn tab_content_text(&self, cx: &WindowContext) -> Option { + let step = self.step.upgrade()?.read(cx); + let context = step.context.upgrade()?.read(cx); + let buffer = context.buffer().read(cx); + let index = context + .workflow_step_index_for_range(&step.context_buffer_range, buffer) + .ok()? + + 1; + Some(format!("Step {index}").into()) + } + + fn tab_icon(&self, _cx: &WindowContext) -> Option { + Some(Icon::new(IconName::Pencil)) } fn to_item_events(event: &Self::Event, mut f: impl FnMut(item::ItemEvent)) {